Re: [PATCH V2] mkfs: Emit a clearer message if too-small log is due to AG geometry

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Aug 27, 2026 at 08:19:52AM -0500, Eric Sandeen wrote:
> On 8/27/26 12:01 AM, Darrick J. Wong wrote:
> > On Wed, Aug 26, 2026 at 10:49:38PM -0500, Eric Sandeen wrote:
> >> There is a constraint in mkfs.xfs that logs must not be smaller than
> >> 64MiB. We can arrive at a too-small log without specifying its size,
> >> though, if we specify so many AGs (or such small AGs) that a single
> >> AG cannot hold the minimum-sized log. This leads to confusing output:
> >>
> >> $ truncate --size=500m testfile
> >> $ mkfs.xfs -dagcount=10 testfile
> >> Log size must be at least 64MiB.
> >> Usage: mkfs.xfs
> >> ...
> >>
> >> because the user did not specify the log size at all! If this error
> >> occurs due to the AG geometry specification, make that clear with a
> >> better error message.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> ---
> >>  mkfs/xfs_mkfs.c | 19 ++++++++++++++++++-
> >>  1 file changed, 18 insertions(+), 1 deletion(-)
> >>
> >> V2: Ugh as soon as I sent this I realized I forgot to restrict this to internal
> >> logs. An external log may be too small all on its own, unrelated to AG size.
> >>
> >> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> >> index a4864c37..4f11c9de 100644
> >> --- a/mkfs/xfs_mkfs.c
> >> +++ b/mkfs/xfs_mkfs.c
> >> @@ -3456,8 +3456,25 @@ validate_supported(
> >>  	 */
> >>  	if (mp->m_sb.sb_logblocks <
> >>  			XFS_MIN_REALISTIC_LOG_BLOCKS(mp->m_sb.sb_blocklog)) {
> >> -		fprintf(stderr,
> >> +		/*
> >> +		 * An internal log must fit within a single allocation group.
> >> +		 * If the user specified the AG geometry but didn't ask for a
> >> +		 * specific log size, the undersized log is due to allocation
> >> +		 * groups that are too small, not a log size the user chose.
> >> +		 */
> >> +		if (cli->loginternal &&
> >> +		    !cli_opt_set(&lopts, L_SIZE) &&
> >> +		    (cli_opt_set(&dopts, D_AGCOUNT) ||
> >> +		     cli_opt_set(&dopts, D_AGSIZE))) {
> >> +			fprintf(stderr,
> >> + _("Allocation group size (%lld MiB) is too small to hold the minimum 64MiB log.\n"
> >> +   "Specify fewer or larger allocation groups, or use a larger data device.\n"),
> > 
> > I like this!  With a couple of comments:
> > 
> > Can the warning message be more targeted to the option?  e.g.
> > 
> > 	if (loginternal && !set(L_SIZE)) {
> > 		if (set(D_AGCOUNT))
> > 			fprintf("Specify fewer AGs or a larger data device...");
> > 		else
> > 			fprintf("Specify larger allocation groups...");
> > 	}
> 
> And if the user specified both size and count? I'm not really convinced
> that we need to go that far, I guess. All the logic in that block is
> kind of eye-bleeding already IMHO.

Eh, ok.  I forgot that maybe the user did both, so they get both
messages. :)

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D


> > Also, what do you think about not hardcoding 64MiB?
> > 
> > printf("Log size must be at least %uMiB\n",
> > 		XFS_MIN_REALISTIC_LOG_BLOCKS(20));
> 
> That give blocks, not MiB I think, right? 
> > (separate change obviously)
> 
> Yeah I noticed that but was just following djwong's lead from the existing
> printf. ;) 

Yeah but djwong is laaaaazy.

> #define XFS_MIN_REALISTIC_LOG_BLOCKS(blog)      (MEGABLOCKS(64, (blog)))
> 
> Maybe we'd need:
> 
> #define XFS_MIN_REALISTIC_LOG_MB	64
> 
> and then
> 
> #define XFS_MIN_REALISTIC_LOG_BLOCKS(blog)      (MEGABLOCKS(XFS_MIN_REALISTIC_LOG_MB, (blog)))
> 
> and then
> 
> printf("Log size must be at least %uMiB\n", XFS_MIN_REALISTIC_LOG_MB);
> 
> or something?

MEGABLOCKS(xx, 20) gets you megabytes:

#define MEGABLOCKS(count, blog)      ((uint64_t)(count) << (20 - (blog)))

so XFS_MIN_REALISTIC_LOG_BLOCKS(20) gets you the min realistic log size
in megabytes.

#define XFS_MIN_REALISTIC_LOG_MB \
	(XFS_MIN_REALISTIC_LOG_BLOCKS(20))

Your way also works. :)

--D

> -Eric
> 
> > --D
> > 
> >> +				(long long)XFS_FSB_TO_B(mp,
> >> +					mp->m_sb.sb_agblocks) >> 20);
> >> +		} else {
> >> +			fprintf(stderr,
> >>   _("Log size must be at least 64MiB.\n"));
> >> +		}
> >>  		usage();
> >>  	}
> >>  
> >> -- 
> >> 2.55.0
> >>
> >>
> > 
>