Re: [PATCH] mkfs.xfs fix sunit size on 512e and 4kN disks.

[Christoph Hellwig <hch@infradead.org>]

On Fri, Sep 26, 2025 at 02:38:30PM +0200, Lukas Herbolt wrote:
> Creating of XFS on 4kN or 512e disk result in suboptimal LSU/LSUNIT.
> As of now we check if the sectorsize is bigger than XLOG_HEADER_SIZE
> and so we set lsu to blocksize. But we do not check the the size if
> lsunit can be bigger to fit the disk geometry.

As I had to walk the code to understand (again for the nth time :))
what the lsunit actually does:  it pads every log write up to that
size.  I.e. if you set a log stripe unit, that effectively becomes the
minimum I/O size for the log.  So yes, setting it to the minimum I/O
size of the device makes sense.  But maybe the commit log should be
a bit more clear about that?  (and of course our terminology should
be as well, ast least outside the user interface that we can't touch).

> Before:

You Before/after also contain changes for metadir/zoned, looks like you
upgraded to a new xfsprogs for your patch, but not the baseline.

> index 8cd4ccd7..05268cd9 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3643,6 +3643,10 @@ check_lsunit:
>  		lsu = getnum(cli->lsu, &lopts, L_SU);
>  	else if (cfg->lsectorsize > XLOG_HEADER_SIZE)
>  		lsu = cfg->blocksize; /* lsunit matches filesystem block size */
> +		if (cfg->dsunit){
> +			cfg->lsunit = cfg->dsunit;
> +			lsu = 0;
> +		}

I don't think just picking the data stripe unit is correct here, given
that the log can also be external and on a separate device.  Instead
we'll need to duplicate the calculation based on ft.log, preferably by
factoring it into a helper.

The lsu = 0 also drop the multiple of block size check.  If that is not
a hard requirement (and I'd have to do some research where it is coming
from) we should relax the check instead of silently disabling it like
this.