Re: [PATCH 04/21] fs: Replace CURRENT_TIME with current_fs_time() for inode timestamps

On Wed, Jun 8, 2016 at 10:04 PM, Deepa Dinamani <[email protected]> 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

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e