XFS_AG_MIN_BLOCKS vs XFS_MIN_AG_BLOCKS

XFS_AG_MIN_BLOCKS vs XFS_MIN_AG_BLOCKS

[Eric Sandeen]

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.

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?

Thanks,
-Eric

Re: XFS_AG_MIN_BLOCKS vs XFS_MIN_AG_BLOCKS

[Dave Chinner]

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@fromorbit.com