On Thu, Jul 27, 2023 at 03:30:16PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > 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@xxxxxxxxxx> > --- > 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@xxxxxxxxxx> -- Dave Chinner david@xxxxxxxxxxxxx