xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: fix race in inode cluster freeing failing to stale inodes
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 26 May 2010 08:58:29 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20100525164052.GA18666@xxxxxxxxxxxxx>
References: <1274581478-19260-1-git-send-email-david@xxxxxxxxxxxxx> <20100525164052.GA18666@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
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@xxxxxx>
> 
> >             /*
> > +            * 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@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>