Re: [PATCH 2/2] xfs: fix agf_fllast when repairing an empty AGFL

Dave Chinner
2023-08-08T20:36:02Z

On Thu, Jul 27, 2023 at 03:30:16PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> xfs/139 with parent pointers enabled occasionally pops up a corruption
> message when online fsck force-rebuild repairs an AGFL:
>
> XFS (sde): Metadata corruption detected at xfs_agf_verify+0x11e/0x220 [xfs], xfs_agf block 0x9e0001
> XFS (sde): Unmount and run xfs_repair
> XFS (sde): First 128 bytes of corrupted metadata buffer:
> 00000000: 58 41 47 46 00 00 00 01 00 00 00 4f 00 00 40 00 XAGF.......O..@.
> 00000010: 00 00 00 01 00 00 00 02 00 00 00 05 00 00 00 01 ................
> 00000020: 00 00 00 01 00 00 00 01 00 00 00 00 ff ff ff ff ................
> 00000030: 00 00 00 00 00 00 00 05 00 00 00 05 00 00 00 00 ................
> 00000040: 91 2e 6f b1 ed 61 4b 4d 8c 9b 6e 87 08 bb f6 36 ..o..aKM..n....6
> 00000050: 00 00 00 01 00 00 00 01 00 00 00 06 00 00 00 01 ................
> 00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00000070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>
> The root cause of this failure is that prior to the repair, there were
> zero blocks in the AGFL. This scenario is set up by the test case, since
> it formats with 64MB AGs and tries to ENOSPC the whole filesystem. In
> this case of flcount==0, we reset fllast to -1U, which then trips the
> write verifier's check that fllast is less than xfs_agfl_size().
>
> Correct this code to set fllast to the last possible slot in the AGFL
> when flcount is zero, which mirrors the behavior of xfs_repair phase5
> when it has to create a totally empty AGFL.
>
> Fixes: 0e93d3f43ec7 ("xfs: repair the AGFL")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/scrub/agheader_repair.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>
> diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> index 4e99e19b2490d..36c511f96b004 100644
> --- a/fs/xfs/scrub/agheader_repair.c
> +++ b/fs/xfs/scrub/agheader_repair.c
> @@ -628,7 +628,10 @@ xrep_agfl_update_agf(
> }
> agf->agf_flfirst = cpu_to_be32(0);
> agf->agf_flcount = cpu_to_be32(flcount);
> - agf->agf_fllast = cpu_to_be32(flcount - 1);
> + if (flcount)
> + agf->agf_fllast = cpu_to_be32(flcount - 1);
> + else
> + agf->agf_fllast = cpu_to_be32(xfs_agfl_size(sc->mp) - 1);
>
> xfs_alloc_log_agf(sc->tp, agf_bp,
> XFS_AGF_FLFIRST | XFS_AGF_FLLAST | XFS_AGF_FLCOUNT);

Looks good.

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