xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: fix race in inode cluster freeing failing to stale inodes
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 25 May 2010 12:40:52 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1274581478-19260-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1274581478-19260-1-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.19 (2009-01-05)
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.
                 */

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