Re: [PATCH] xfs: test mkfs.xfs sizing of internal logs that

Dave Chinner
2022-05-26T00:12:00Z

On Wed, May 25, 2022 at 04:21:36PM -0700, Darrick J. Wong wrote:
> On Wed, May 25, 2022 at 10:24:13AM +1000, Dave Chinner wrote:
> > On Tue, May 24, 2022 at 04:49:30PM -0700, Darrick J. Wong wrote:
> > > On Wed, May 25, 2022 at 09:44:26AM +1000, Dave Chinner wrote:
> > > > On Tue, May 24, 2022 at 12:52:57PM -0700, Darrick J. Wong wrote:
> > > > > +# First we try various small filesystems and stripe sizes.
> > > > > +for M in `seq 298 302` `seq 490 520`; do
> > > > > + for S in `seq 32 4 64`; do
> > > > > + test_format "M=$M S=$S" -dsu=${S}k,sw=1,size=${M}m
> > > > > + done
> > > > > +done
> > > > > +
> > > > > +# log so large it pushes the root dir into AG 1
> > > > > +test_format "log pushes rootdir into AG 1" -d agcount=3200,size=6366g -lagnum=0
> > >
> > > ...this particular check in mkfs only happens after we allocate the root
> > > directory, which an -N invocation doesn't do.
> >
> > Ok, so for this test can we drop the -N? We don't need to do 30 IOs
> > and write 64MB logs for every config we test - I think there's ~35 *
> > 8 invocations of test_format in the loop above before we get here...
> >
> > Also, why do we need a 6.3TB filesystem with 2.1GB AGs and a 2GB log
> > to trigger this? That means we have to write 2GB to disk, plus
> > ~20,000 write IOs for the AG headers and btree root blocks before we
> > get to the failure case, yes?
> >
> > Why not just exercise the failure case with something like this:
> >
> > # mkfs.xfs -d agcount=2,size=64M -l size=8180b,agnum=0 -d file,name=test.img
> > meta-data=test.img isize=512 agcount=2, agsize=8192 blks
> > = sectsz=512 attr=2, projid32bit=1
> > = crc=1 finobt=1, sparse=1, rmapbt=0
> > = reflink=1 bigtime=0 inobtcount=0
> > data = bsize=4096 blocks=16384, imaxpct=25
> > = sunit=0 swidth=0 blks
> > naming =version 2 bsize=4096 ascii-ci=0, ftype=1
> > log =internal log bsize=4096 blocks=8180, version=2
> > = sectsz=512 sunit=0 blks, lazy-count=1
> > realtime =none extsz=4096 blocks=0, rtextents=0
> > mkfs.xfs: root inode created in AG 1, not AG 0
>
> Welll... a better reason for why this test can't do that -- one of my
> fixes also made mkfs reject -l size=XXX when XXX is not possible.

So you make other changes to mkfs that aren't yet upstream that
check whether the log causes the root inode to change position?
I mean, '-l size=8180b' results in a log that fits perfectly fine in
an empty AG, and so you must have added more checks to detect this
moves the root inode somehow?

> That said... -d agcount=3200,size=6366g -lagnum=0 -N seems to work?

On a new mkfs binary with those checks that you mention above? If
so, it seems to me that those new checks trigger for this case, too,
before we write anything?

IOWs, this won't fail on older mkfs.xfs binaries that don't have
these new log size checks because we never get around to allocating
the root inode and checking it's location, right? Yet what we
really want here is for it to fail on an old mkfs binary?