Re: [PATCH] always set a/c/mtime through ->setattr

[Christoph Hellwig <hch@lst.de>]

On Tue, May 20, 2008 at 09:40:56AM +0200, Miklos Szeredi wrote:
> > Currently touch_atime and file_update_time directly update a/c/mtime
> > in the inode and just mark the inode dirty afterwards.  This is pretty
> > bad for some more complex filesystems that have various different types
> > of dirtying an inode and/or need to store the data in another place
> > for example for a buffer to be logged.
> > 
> > This patch changes touch_atime and file_update_time to not update the
> > inode directly but rather call through ->setattr into the filessystem.
> 
> Do we know what effect this will have on read/write performance?  I
> can imagine that some ->setattr() implementations are orders of
> magnitude slower than just dirtying the inode.

All major disk or in-memory filesystems except for XFS just pass down
ATTR_*TIME requests to inode_setattr which is not more than just
dirtying the inode.  NFS and CIFS set S_NOCMTIME so they're not affected
by this at all.

> This optimization is fishy.  Remember, inode->i_*time are just cached
> values, and the actual times on the (remote) filesystem itself can
> differ.  Which means that we will now optimize out a "touch" because
> we happened to have the current time cached in the inode.  Not that
> this would be a likely event, but still...
> 
> So at least this check should be made dependent on ATTR_UPDTIMES, and
> explicit time updates left alone.

Good catch.  I'll fix by either/or moving the check into ->setattr and
making it conditional on ATTR_UPDTIMES.