Re: [PATCH] xfs: fix the xfs_iflush_done callback search

[Dave Chinner <david@fromorbit.com>]

On Wed, Oct 01, 2014 at 04:18:02PM -0500, Mark Tinguely wrote:
> Commit "xfs: remove all the inodes on a buffer from the AIL in bulk"
> made the xfs inode flush callback more efficient by combining all
> the inode writes on the buffer and the deletions of the inode log
> item from AIL.
> 
> The initial loop in this patch should be looping through all
> the log items on the buffer to see which items have
> xfs_iflush_done as their callback function. But currently,
> only the log item passed to the function has its callback
> compared to xfs_iflush_done. If the log item pointer passed to
> the function does have the xfs_iflush_done callback function,
> then all the log items on the buffer are removed from the
> li_bio_list on the buffer b_fspriv and could be removed from
> the AIL eventhough they may have not been written yet.

Looks like a bug, but what I don't know from this description is
the symptoms and impact of this bug being hit? Is there a risk of
filesystem corruption on crash or power loss? Perhaps it's a data
loss issue? I can't tell, and for anyone scanning the commit logs to
determine if they need to backport the fix will be asking the same
questions.

Also, is there a reproducable test case for it?

> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
> ---
>  fs/xfs/xfs_inode_item.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: b/fs/xfs/xfs_inode_item.c
> ===================================================================
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -615,7 +615,7 @@ xfs_iflush_done(
>  	blip = bp->b_fspriv;
>  	prev = NULL;
>  	while (blip != NULL) {
> -		if (lip->li_cb != xfs_iflush_done) {
> +		if (blip->li_cb != xfs_iflush_done) {

Looks correct, but more info needed...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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