On Tue, May 30, 2023 at 12:02:04PM -0500, Eric Sandeen wrote: > I got a bug report that REAR was trying to recreate an xfs filesystem > geometry by looking at the xfs_info from the original filesystem. > > In this case, the original fs was: > > meta-data=/dev/mapper/vg-lv_srv isize=512 agcount=400, agsize=6144 blks > = sectsz=512 attr=2, projid32bit=1 > = crc=1 finobt=1, sparse=1, rmapbt=0 > = reflink=1 bigtime=1 inobtcount=1 > data = bsize=4096 blocks=2453504, imaxpct=25 > = sunit=16 swidth=16 blks > naming =version 2 bsize=4096 ascii-ci=0, ftype=1 > log =internal log bsize=4096 blocks=1872, version=2 > = sectsz=512 sunit=16 blks, lazy-count=1 > realtime =none extsz=4096 blocks=0, rtextents=0 > > (horribly pessimal, almost certainly the result of xfs_growfs) > > But the point is, the last AG is only 8MB. However, mkfs.xfs refuses to make > an AG less than 16MB. So, this fails, because agcount was specified and mkfs > won't reduce it to fix the too-small AG: > > # truncate --size=10049552384 fsfile > # mkfs.xfs -f -m uuid=23ce7347-fce3-48b4-9854-60a6db155b16 -i size=512 -d > agcount=400 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b > size=4096 -i maxpct=25 -d sunit=128 -d swidth=128 -l version=2 -l sunit=128 > -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 fsfile > mkfs.xfs: xfs_mkfs.c:3016: align_ag_geometry: Assertion > `!cli_opt_set(&dopts, D_AGCOUNT)' failed. /me finally catches up and reads bug report and Oh My What In The World.... :) > I think this is the result of mkfs.xfs using 16MB as a limit on last AG > size: > > #define XFS_AG_MIN_BYTES ((XFS_AG_BYTES(15))) /* 16 MB */ > #define XFS_AG_MIN_BLOCKS(blog) (XFS_AG_MIN_BYTES >> (blog)) > > But growfs uses this: > > #define XFS_MIN_AG_BLOCKS 64 > > (which is much smaller than 16MB). > > This should almost certainly be consistent between mkfs and growfs, and my > guess is that growfs should start using the larger XFS_AG_MIN_BLOCKS > requirement that mkfs.xfs uses? Yeah, that seems reasonable, but we can't get rid of XFS_MIN_AG_BLOCKS unfortunately. There are clearly filesystems out there that depend on AGs this small existing, so we can't just replace one with the other. e.g. XFS_MIN_DBLOCKS() uses it and that's part of the superblock size verification checks.... That said, the same superblock verifier asserts this is a failure: XFS_FSB_TO_B(mp, sbp->sb_agblocks) < XFS_MIN_AG_BYTES So now I'm guessing that the AGF verifier doesn't have the same agf->agf_length verification against either XFS_MIN_AG_BYTES or sbp->sb_agblocks. Yup, the agf verifier does not check miniumum AGF length, it does not even check that the agf length is the same as (or smaller than for the last ag) sbp->sb_agblocks, either. Worse is that we use agf->agf_length as if it was fully validated and correct for other runtime block and extent range corruption checks. Ugh. I guess that's what the rest of my day is going to be spent unravelling, starting with trying to work out why I never validated this AGF field in the first place more than a decade ago.... -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
