Re: [PATCH] xfs: fix race in inode cluster freeing failing to stale inodes

[Dave Chinner <david@fromorbit.com>]

On Tue, May 25, 2010 at 12:40:52PM -0400, Christoph Hellwig wrote:
> Looks good, but some minor nits on the comments below:
> 
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> >  		/*
> > +		 * Now we've locked out tail pushing and flushing by locking
> > +		 * the buffer, look for each inode in memory and attempt to
> > +		 * lock it. Any inode we get the locks on add it to the inode
> > +		 * buffer and set it up for being staled on buffer IO
> > +		 * completion.
> 
> This comment reads a bit odd.  The first thing we do in the loop is
> locking the buffer, so the "Now" at the beginning of the comment feels
> rather out of place.  What about:
> 
> 		/*
> 		 * For each inode in memory attempt to add it to the inode
> 		 * buffer and set it up for being staled on buffer IO
> 		 * completion.  This is safe as we've locked out tail
> 		 * pushing and flushing by locking the buffer.
> 		 *
> 		 * We have already marked every inode that was part of
> 		 * a transaction stale above, which means there is no
> 		 * point in even trying to lock them.
> 		 */

That reads better. I'll update it to match.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs