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.
|