Re: [PATCH 3/4] mkfs: complain about impossible log size constraints

From: "Darrick J. Wong" <djwong@kernel.org>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 3/4] mkfs: complain about impossible log size constraints
Date: Wed, 13 Jul 2022 19:03:20 -0700	[thread overview]
Message-ID: <Ys95aIUPgQpryAeL@magnolia> (raw)
In-Reply-To: <ce55b1b4-53a2-a620-a2f8-d601fd48bfa9@sandeen.net>

On Wed, Jul 13, 2022 at 08:17:22PM -0500, Eric Sandeen wrote:
> On 7/12/22 8:09 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > xfs/042 trips over an impossible fs geometry when nrext64 is enabled.
> > The minimum log size calculation comes out to 4287 blocks, but the mkfs
> > parameters specify an AG size of 4096 blocks.  This eventually causes
> > mkfs to complain that the autoselected log size doesn't meet the minimum
> > size, but we could be a little more explicit in pointing out that the
> > two size constraints make for an impossible geometry.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  mkfs/xfs_mkfs.c |    7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > 
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index db322b3a..61ac1a4a 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -3401,6 +3401,13 @@ _("external log device size %lld blocks too small, must be at least %lld blocks\
> >  	 * an AG.
> >  	 */
> >  	max_logblocks = libxfs_alloc_ag_max_usable(mp) - 1;
> > +	if (max_logblocks < min_logblocks) {
> > +		fprintf(stderr,
> > +_("max log size %d smaller than min log size %d\n"),
> 
> And when the user sees this, they will know that they should ___________ ?

I dunno, ask for creating a bigger filesystem?

It's better than "log size 4083 blocks too small, minimum size is 4287
blocks", which hides the part where we autoselected 4083 blocks because
that's max_logblocks.

/me would suggest pulling in the no tiny fs patch, which will at least
fail the cases where the user wants tiny AGs but the featureset wants a
big log with "your fs is too small".

I haven't gotten around to playing with raid stripe size variations yet
though, so I don't know if this problem will come back with (say) a 301M
filesystem and a giant RAID stripe.

--D

> > +				max_logblocks,
> > +				min_logblocks);
> > +		usage();
> > +	}
> >  
> >  	/* internal log - if no size specified, calculate automatically */
> >  	if (!cfg->logblocks) {
> > 