On Wed, Jun 8, 2016 at 10:04 PM, Deepa Dinamani deepa.kernel@gmail.com wrote:
CURRENT_TIME macro is not appropriate for filesystems as it doesn't use the right granularity for filesystem timestamps. Use current_fs_time() instead.
Again - using the inode instead fo the syuperblock in tghis patch would have made the patch much more obvious (it could have been 99% generated with the sed-script I sent out a week or two ago), and it would have made it unnecessary to add these kinds of things:
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index e9f5043..85c12f0 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -2359,6 +2359,7 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, { struct usb_dev_state *ps = file->private_data; struct inode *inode = file_inode(file);
struct super_block *sb = inode->i_sb; struct usb_device *dev = ps->dev; int ret = -ENOTTY;
where we add a new variable just because the calling convention was wrong.
It's not even 100% obvious that a filesystem has to have one single time representation, so making the time function about the entity whose time is set is also conceptually a much better model, never mind that it is just what every single user seems to want anyway.
So I'd *much* rather see
+ inode->i_atime = inode->i_mtime = inode->i_ctime = current_fs_time(inode);
over seeing either of these two variants::
+ inode->i_atime = inode->i_mtime = inode->i_ctime = current_fs_time(inode->i_sb); + ret->i_atime = ret->i_mtime = ret->i_ctime = current_fs_time(sb);
because the first of those variants (grep for current_fs_time() in the current git tree, and notice that it's the common one) we have the pointless "let's chase a pointer in every caller"
And while it's true that the second variant is natural for *some* situations, I've yet to find one where it wasn't equally sane to just pass in the inode instead.
Linus