Re: [patch v2] fix truncate inode time modification breakage

[Nick Piggin <npiggin@suse.de>]

On Thu, Jun 03, 2010 at 10:18:24AM +0200, Miklos Szeredi wrote:
> On Thu, 3 Jun 2010, Nick Piggin wrote:
> > Index: linux-2.6/fs/fuse/dir.c
> > ===================================================================
> > --- linux-2.6.orig/fs/fuse/dir.c
> > +++ linux-2.6/fs/fuse/dir.c
> > @@ -1161,20 +1161,6 @@ static int fuse_dir_fsync(struct file *f
> >  	return fuse_fsync_common(file, datasync, 1);
> >  }
> >  
> > -static bool update_mtime(unsigned ivalid)
> > -{
> > -	/* Always update if mtime is explicitly set  */
> > -	if (ivalid & ATTR_MTIME_SET)
> > -		return true;
> > -
> > -	/* If it's an open(O_TRUNC) or an ftruncate(), don't update */
> > -	if ((ivalid & ATTR_SIZE) && (ivalid & (ATTR_OPEN | ATTR_FILE)))
> > -		return false;
> > -
> > -	/* In all other cases update */
> > -	return true;
> > -}
> > -
> 
> Fuse philosophy is: each operation itself has to update times on files
> if necessary.  So it basically moves the responsibility to update
> [amc]time from the VFS into the filesystem.
> 
> This means the only place fuse is interested in ATTR_ATIME or
> ATTR_MTIME is for the utime* syscalls.

OK, makes sense. I wonder why you do ATTR_ATIME changes, though,
rather than just getting those too back from the filesystem?

Do you ever have a problem with inode's atime going backwards due
to differences between touch_atime and reading the atime from the
server?


> It also means that fuse always ignores ATTR_CTIME which is never set
> explicitly.
> 
> So I believe the current fuse code is correct.

After my patch to pass ATTR_MTIME|ATTR_CTIME from truncate(2), it is
not (because above won't return false, ie. it will change the mtime
for truncate).

Why not avoid all mtime updates except MTIME_SET?