Re: [PATCH] xfs: flush inode gc workqueue before clearing agi bucket

[Dave Chinner <david@fromorbit.com>]

On Mon, Jul 11, 2022 at 10:41:34PM +0800, Zhang Yi wrote:
> In the procedure of recover AGI unlinked lists, if something bad
> happenes on one of the unlinked inode in the bucket list, we would call
> xlog_recover_clear_agi_bucket() to clear the whole unlinked bucket list,
> not the unlinked inodes after the bad one. If we have already added some
> inodes to the gc workqueue before the bad inode in the list, we could
> get below error when freeing those inodes, and finaly fail to complete
> the log recover procedure.
> 
>  XFS (ram0): Internal error xfs_iunlink_remove at line 2456 of file
>  fs/xfs/xfs_inode.c.  Caller xfs_ifree+0xb0/0x360 [xfs]
> 
> The problem is xlog_recover_clear_agi_bucket() clear the bucket list, so
> the gc worker fail to check the agino in xfs_verify_agino(). Fix this by
> flush workqueue before clearing the bucket.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
>  fs/xfs/xfs_log_recover.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 5f7e4e6e33ce..2f655ef4364e 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2714,6 +2714,7 @@ xlog_recover_process_one_iunlink(
>  	 * Call xlog_recover_clear_agi_bucket() to perform a transaction to
>  	 * clear the inode pointer in the bucket.
>  	 */
> +	xfs_inodegc_flush(mp);
>  	xlog_recover_clear_agi_bucket(mp, agno, bucket);
>  	return NULLAGINO;
>  }

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

Darrick, FYI, I actually tripped over this and fixed it in the inode
iunlink series as part of double linking the unlinked inode list in
this patch:

https://lore.kernel.org/linux-xfs/20220707234345.1097095-6-david@fromorbit.com/

I didn't realise at the time I was forward porting this code that it
was a pre-existing bug.....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com