On Wed, Sep 22, 2010 at 01:24:01PM -0400, Christoph Hellwig wrote: > > However, the timstamp changes are slightly more complex than this - > > there are a couple of places that do unlogged updates of the > > timestamps, and the VFS need to be informed of these. Hence add a > > new function xfs_trans_inode_chgtime() for transactional changes, > > and leave xfs_ichgtime() for the non-transactional changes. > > The only user of xfs_ichgtime after this patch is a special case in > truncate for the case of a zero-sized file that's also truncated to size > zero. I think we should just remove this special case and not require > xfs_ichgtime at all. I'll prepare patches to clean up xfs_setattr > and remove this non-transaction update and once this patch is rebased > ontop of that it can be simplied again. > > That leaves the timestamp updates from the data I/O path special as > they still get updated via direct writes to inode->i_*time and > mark_inode_dirty. I guess we'll have to live with that for now. > > > > + * Transactional inode timestamp update. requires inod to be locked and joined > > + * to the transaction supplied. Relies on the transaction subsystem to track > > + * dirty state and update/writeback the inode accordingly. > > s/inod/the inode/ > > Also I wonder if xfs_trans_ichgtime should be in xfs_trans_inode.c with > a prototype in xfs_trans.h, just like all the other xfs_trans* > functions. If we get rid of the special setattr case, then yes, it should be moved to a transaction specific file. > > > /* > > + * Hit the inode change time. > > + */ > > All these comments are utterly pointless. I'd suggest removing them > when touching the surrounding areas. Ok, will do. > > > +++ b/fs/xfs/xfs_inode_item.c > > @@ -223,15 +223,6 @@ xfs_inode_item_format( > > nvecs = 1; > > > > /* > > - * Make sure the linux inode is dirty. We do this before > > - * clearing i_update_core as the VFS will call back into > > - * XFS here and set i_update_core, so we need to dirty the > > - * inode first so that the ordering of i_update_core and > > - * unlogged modifications still works as described below. > > - */ > > - xfs_mark_inode_dirty_sync(ip); > > - > > With this gone the comment above xfs_fs_dirty_inode will need an update. OK. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
