Re: [RFC] don't update atime on file write...

[Dave Chinner <david@fromorbit.com>]

On Mon, Oct 30, 2017 at 03:02:06PM -0400, Mike Marshall wrote:
> I've been working with xfstests generic/003. It contains
> numerous tests.
> 
> For example, we were updating atime on a directory when a
> new file was created in the directory. It "makes sense",
> but it was easy to google "open posix" and see that it was
> wrong:
> 
>   If O_CREAT is set and the file did not previously exist, upon
>   successful completion, open() shall mark for update the st_atime,
>   st_ctime, and st_mtime fields of the file and the st_ctime and
>   st_mtime fields of the parent directory.
> 
> We fixed the above in the server.
> 
> Another thing we've been doing wrong is to update atime
> when a file is modified.
> 
> Google "write posix" to the rescue again:
> 
>   Upon successful completion, where nbyte is greater than 0, write()
>   shall mark for update the st_ctime and st_mtime fields of the file,
>   and if the file is a regular file, the S_ISUID and S_ISGID bits
>   of the file mode may be cleared.
> 
> This makes the test pass, and I don't see that it has any bad
> side effects, does it seem OK to you all?
> 
> diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
> index 336ecbf..009fce0 100644
> --- a/fs/orangefs/file.c
> +++ b/fs/orangefs/file.c
> @@ -384,7 +384,9 @@ static ssize_t do_readv_writev(enum
> ORANGEFS_io_type type, struct file *file,
>                 } else {
>                         SetMtimeFlag(orangefs_inode);
>                         inode->i_mtime = current_time(inode);
> +/*
>                         mark_inode_dirty_sync(inode);
> +*/

That'll break dirty inode tracking. Your problem here is an
orangefs issue:

mark_inode_dirty_sync
  __mark_inode_dirty
    ->dirty_inode
      orangefs_dirty_inode
        SetAtimeFlag(orangefs_inode);

i.e. every time the inode is dirtied for data, metadata or
timestamps, orangefs is updating atime.

If you need to do something whenever the kernel updates atime (or
c/mtime), then implement ->update_time....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com