On Thu, Apr 27, 2023 at 03:49:05PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> Through generic/300, I discovered that mkfs.xfs creates corrupt
> filesystems when given these parameters:
>
> # mkfs.xfs -d size=512M /dev/sda -f -d su=128k,sw=4 --unsupported
> Filesystems formatted with --unsupported are not supported!!
> meta-data=/dev/sda isize=512 agcount=8, agsize=16352 blks
> = sectsz=512 attr=2, projid32bit=1
> = crc=1 finobt=1, sparse=1, rmapbt=1
> = reflink=1 bigtime=1 inobtcount=1 nrext64=1
> data = bsize=4096 blocks=130816, imaxpct=25
> = sunit=32 swidth=128 blks
> naming =version 2 bsize=4096 ascii-ci=0, ftype=1
> log =internal log bsize=4096 blocks=8192, version=2
> = sectsz=512 sunit=32 blks, lazy-count=1
> realtime =none extsz=4096 blocks=0, rtextents=0
> = rgcount=0 rgsize=0 blks
> Discarding blocks...Done.
> # xfs_repair -n /dev/sda
> Phase 1 - find and verify superblock...
> - reporting progress in intervals of 15 minutes
> Phase 2 - using internal log
> - zero log...
> - 16:30:50: zeroing log - 16320 of 16320 blocks done
> - scan filesystem freespace and inode maps...
> agf_freeblks 25, counted 0 in ag 4
> sb_fdblocks 8823, counted 8798
>
> The root cause of this problem is the numrecs handling in
> xfs_freesp_init_recs, which is used to initialize a new AG. Prior to
> calling the function, we set up the new bnobt block with numrecs == 1
> and rely on _freesp_init_recs to format that new record. If the last
> record created has a blockcount of zero, then it sets numrecs = 0.
>
> That last bit isn't correct if the AG contains the log, the start of the
> log is not immediately after the initial blocks due to stripe alignment,
> and the end of the log is perfectly aligned with the end of the AG. For
> this case, we actually formatted a single bnobt record to handle the
> free space before the start of the (stripe aligned) log, and incremented
> arec to try to format a second record. That second record turned out to
> be unnecessary, so what we really want is to leave numrecs at 1.
>
> The numrecs handling itself is overly complicated because a different
> function sets numrecs == 1. Change the bnobt creation code to start
> with numrecs set to zero and only increment it after successfully
> formatting a free space extent into the btree block.
>
> Fixes: f327a00745ff ("xfs: account for log space when formatting new AGs")
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_ag.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 1b078bbbf225..4481ce8ead9d 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -499,6 +499,7 @@ xfs_freesp_init_recs(
> */
> arec->ar_blockcount = cpu_to_be32(start -
> mp->m_ag_prealloc_blocks);
> + be16_add_cpu(&block->bb_numrecs, 1);
> nrec = arec + 1;
>
> /*
> @@ -509,7 +510,6 @@ xfs_freesp_init_recs(
> be32_to_cpu(arec->ar_startblock) +
> be32_to_cpu(arec->ar_blockcount));
> arec = nrec;
> - be16_add_cpu(&block->bb_numrecs, 1);
> }
> /*
> * Change record start to after the internal log
> @@ -525,8 +525,8 @@ xfs_freesp_init_recs(
> */
> arec->ar_blockcount = cpu_to_be32(id->agsize -
> be32_to_cpu(arec->ar_startblock));
> - if (!arec->ar_blockcount)
> - block->bb_numrecs = 0;
> + if (arec->ar_blockcount)
> + be16_add_cpu(&block->bb_numrecs, 1);
Ok, but I think the comment above this about resetting the count
back to zero needs to be updated as we aren't resetting anything
back to zero anymore.
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
