Linux XFS filesystem development
 help / color / mirror / Atom feed
* [PATCH 0/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
@ 2026-02-19 11:44 Lukas Herbolt
  2026-02-19 11:44 ` [PATCH 1/1] " Lukas Herbolt
  0 siblings, 1 reply; 8+ messages in thread
From: Lukas Herbolt @ 2026-02-19 11:44 UTC (permalink / raw)
  To: hch, aalbersh, cem; +Cc: linux-xfs, Lukas Herbolt

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.

It was laready discussed here:
 - https://lore.kernel.org/linux-xfs/aOX_TzJIxJWWC63x@infradead.org/

but it somehow fell of the plate.

Before:
modprobe scsi_debug inq_vendor=XFS_TEST physblk_exp=3 sector_size=512 \
opt_xferlen_exp=9 opt_blks=512 dev_size_mb=100 virtual_gb=1000; \
lsblk -tQ 'VENDOR == "XFS_TEST"'; \
mkfs.xfs -f $(lsblk -Q 'VENDOR == "XFS_TEST"' -no path) 2>/dev/null; sleep 1; \
modprobe -r scsi_debug
NAME ALIGNMENT MIN-IO OPT-IO PHY-SEC LOG-SEC ROTA SCHED RQ-SIZE  RA WSAME
sda          0 262144 262144    4096     512    0 bfq       256 512    0B
meta-data=/dev/sda               isize=512    agcount=32, agsize=8192000 blks
         =                       sectsz=4096  attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=1, rmapbt=1
         =                       reflink=1    bigtime=1 inobtcount=1 nrext64=1
         =                       exchange=0
data     =                       bsize=4096   blocks=262144000, imaxpct=25
         =                       sunit=64     swidth=64 blks
naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
log      =internal log           bsize=4096   blocks=128000, version=2
         =                       sectsz=4096  sunit=1 blks, lazy-count=1
                                              ^^^^^^^^^^^^^^
realtime =none                   extsz=4096   blocks=0, rtextents=0

After:
modprobe scsi_debug inq_vendor=XFS_TEST physblk_exp=3 sector_size=512 \
opt_xferlen_exp=9 opt_blks=512 dev_size_mb=100 virtual_gb=1000; \
lsblk -tQ 'VENDOR == "XFS_TEST"'; \
mkfs.xfs -f $(lsblk -Q 'VENDOR == "XFS_TEST"' -no path) 2>/dev/null; sleep 1; \
modprobe -r scsi_debug
NAME ALIGNMENT MIN-IO OPT-IO PHY-SEC LOG-SEC ROTA SCHED RQ-SIZE  RA WSAME
sda          0 262144 262144    4096     512    0 bfq       256 512    0B
meta-data=/dev/sda               isize=512    agcount=32, agsize=8192000 blks
         =                       sectsz=4096  attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=1, rmapbt=1
         =                       reflink=1    bigtime=1 inobtcount=1 nrext64=1
         =                       exchange=0   metadir=0
data     =                       bsize=4096   blocks=262144000, imaxpct=25
         =                       sunit=64     swidth=64 blks
naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
log      =internal log           bsize=4096   blocks=128000, version=2
         =                       sectsz=4096  sunit=64 blks, lazy-count=1
                                              ^^^^^^^^^^^^^^
realtime =none                   extsz=4096   blocks=0, rtextents=0
         =                       rgcount=0    rgsize=0 extents
         =                       zoned=0      start=0 reserved=0

Lukas Herbolt (1):
  mkfs.xfs fix sunit size on 512e and 4kN disks.

 mkfs/xfs_mkfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
  2026-02-19 11:44 [PATCH 0/1] mkfs.xfs fix sunit size on 512e and 4kN disks Lukas Herbolt
@ 2026-02-19 11:44 ` Lukas Herbolt
  2026-02-19 13:32   ` Christoph Hellwig
                     ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Lukas Herbolt @ 2026-02-19 11:44 UTC (permalink / raw)
  To: hch, aalbersh, cem; +Cc: linux-xfs, Lukas Herbolt

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.

Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
---
 mkfs/xfs_mkfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index b34407725f76..1b6334e9adce 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3647,7 +3647,7 @@ check_lsunit:
 	else if (cfg->lsectorsize > XLOG_HEADER_SIZE)
 		lsu = cfg->blocksize; /* lsunit matches filesystem block size */
 
-	if (lsu) {
+	if (cli->lsu) {
 		/* verify if lsu is a multiple block size */
 		if (lsu % cfg->blocksize != 0) {
 			fprintf(stderr,
-- 
2.53.0

From 2771375662c9edce25d7268bc71cc6db35a0d5c7 Mon Sep 17 00:00:00 2001
From: Lukas Herbolt <lukas@herbolt.com>
Date: Fri, 26 Sep 2025 12:48:39 +0200
Subject: [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.

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.

Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
---
 mkfs/xfs_mkfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index b34407725f76..1b6334e9adce 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3647,7 +3647,7 @@ check_lsunit:
 	else if (cfg->lsectorsize > XLOG_HEADER_SIZE)
 		lsu = cfg->blocksize; /* lsunit matches filesystem block size */
 
-	if (lsu) {
+	if (cli->lsu) {
 		/* verify if lsu is a multiple block size */
 		if (lsu % cfg->blocksize != 0) {
 			fprintf(stderr,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
  2026-02-19 11:44 ` [PATCH 1/1] " Lukas Herbolt
@ 2026-02-19 13:32   ` Christoph Hellwig
  2026-03-04 19:49   ` Darrick J. Wong
  2026-06-15 13:17   ` Jan Kara
  2 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2026-02-19 13:32 UTC (permalink / raw)
  To: Lukas Herbolt; +Cc: hch, aalbersh, cem, linux-xfs

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
  2026-02-19 11:44 ` [PATCH 1/1] " Lukas Herbolt
  2026-02-19 13:32   ` Christoph Hellwig
@ 2026-03-04 19:49   ` Darrick J. Wong
  2026-06-15 13:17   ` Jan Kara
  2 siblings, 0 replies; 8+ messages in thread
From: Darrick J. Wong @ 2026-03-04 19:49 UTC (permalink / raw)
  To: Lukas Herbolt, Andrey Albershteyn; +Cc: hch, aalbersh, cem, linux-xfs

On Thu, Feb 19, 2026 at 12:44:09PM +0100, 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.
> 
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
>  mkfs/xfs_mkfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index b34407725f76..1b6334e9adce 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3647,7 +3647,7 @@ check_lsunit:
>  	else if (cfg->lsectorsize > XLOG_HEADER_SIZE)
>  		lsu = cfg->blocksize; /* lsunit matches filesystem block size */
>  
> -	if (lsu) {
> +	if (cli->lsu) {

This patch causes ~96% failure rates on fstests on my test fleet, some
of which now have 4k LBA disks with unexciting min/opt io geometry:

# lsblk -t /dev/sda
NAME   ALIGNMENT MIN-IO  OPT-IO PHY-SEC LOG-SEC ROTA SCHED RQ-SIZE   RA WSAME
sda            0   4096 1048576    4096     512    1 bfq       256 2048    0B
# mkfs.xfs -f -N /dev/sda3
meta-data=/dev/sda3              isize=512    agcount=4, agsize=2183680 blks
         =                       sectsz=4096  attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=1, rmapbt=1
         =                       reflink=1    bigtime=1 inobtcount=1 nrext64=1
         =                       exchange=1   metadir=0
data     =                       bsize=4096   blocks=8734720, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=1
log      =internal log           bsize=4096   blocks=16384, version=2
         =                       sectsz=4096  sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
         =                       rgcount=0    rgsize=0 extents
         =                       zoned=0      start=0 reserved=0

Note that MIN-IO == PHY-SEC, so dsunit/dswidth are zero.  With this
change, we no longer set the lsunit to the fsblock size if the log
sector size is greater than 512.  Unfortunately, dsunit is also not set,
so mkfs never sets the log sunit and it remains zero.  I think
this causes problems with the log roundoff computation in the kernel:

	if (xfs_has_logv2(mp) && mp->m_sb.sb_logsunit > 1)
		log->l_iclog_roundoff = mp->m_sb.sb_logsunit;
	else
		log->l_iclog_roundoff = BBSIZE;

because now the roundoff factor is less than the log sector size.  After
a while, the filesystem cannot be mounted anymore because:

XFS (sda3): Mounting V5 Filesystem 81b8ffa8-383b-4574-a68c-9b8202707a26
XFS (sda3): Corruption warning: Metadata has LSN (4:2729) ahead of current LSN (4:2727). Please unmount and run xfs_repair (>= v4.3) to resolve.
XFS (sda3): log mount/recovery failed: error -22
XFS (sda3): log mount failed

Reverting this patch makes the problem go away, but I think you're
trying to make it so that mkfs will set lsunit = dsunit if dsunit>0 and
the caller didn't specify any -lsunit= parameter, right?

But there's something that just seems off with this whole function.  If
the user provided a -lsunit/-lsu option then we need to validate the
value and either use it if it makes sense, or complain if not.  If the
user didn't specify any option, then we should figure it out
automatically from the other data device geometry options (internal) or
the external log device probing.

But that's not what this function does.  Why would you do this:

	else if (cfg->lsectorsize > XLOG_HEADER_SIZE)
		lsu = cfg->blocksize; /* lsunit matches filesystem block size */

and then loudly validate that lsu (bytes) is congruent with the fsblock
size?  This is trivially true, but then it disables the "make lsunit use
dsunit if set" logic below:

	} else if (cfg->sb_feat.log_version == 2 &&
		   cfg->loginternal && cfg->dsunit) {
		/* lsunit and dsunit now in fs blocks */
		cfg->lsunit = cfg->dsunit;
	}

AFAICT, the "lsunit matches fs block size" logic is buggy.  This code
was added with no justification as part of a "reworking" commit
2f44b1b0e5adc4 ("mkfs: rework stripe calculations") back in 2017.  I
think the correct logic is:

	if (cli_opt_set(&lopts, L_SUNIT))
		lsunit = cli->lsunit;
	else if (cli_opt_set(&lopts, L_SU))
		lsu = getnum(cli->lsu, &lopts, L_SU);

	if (lsu) {
		/* verify if lsu is a multiple block size */
		if (lsu % cfg->blocksize != 0) {
			fprintf(stderr,
	_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
				lsu, cfg->blocksize);
			usage();
		}
		lsunit = (int)BTOBBT(lsu);
	}
	if (BBTOB(lsunit) % cfg->blocksize != 0) {
		fprintf(stderr,
_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
			BBTOB(lsunit), cfg->blocksize);
		usage();
	}

and then we move the "lsunit matches fs block size" logic to the
no-lsunit-option code below:

	if (lsunit) {
		/* convert from 512 byte blocks to fs blocks */
		cfg->lsunit = DTOBT(lsunit, cfg->blocklog);
	} else if (cfg->sb_feat.log_version == 2 && cfg->loginternal) {
		if (cfg->dsunit) {
			/* lsunit and dsunit now in fs blocks */
			cfg->lsunit = cfg->dsunit;
		} else if (cfg->lsectorsize > XLOG_HEADER_SIZE) {
			/* lsunit matches filesystem block size */
			cfg->lsunit = 1;
		}
	} else if (cfg->sb_feat.log_version == 2 &&
		   !cfg->loginternal) {
		/* use the external log device properties */
		cfg->lsunit = DTOBT(ft->log.sunit, cfg->blocklog);
	}

This seems to set sb_logsunit to 4096 on my test VM, to 262144 with
the scsi_debug device that you created in [1], and to 0 on the even more
boring VMs with 512 physical sectors.

--D

[1] https://lore.kernel.org/linux-xfs/20250926123829.2101207-2-lukas@herbolt.com/

>  		/* verify if lsu is a multiple block size */
>  		if (lsu % cfg->blocksize != 0) {
>  			fprintf(stderr,
> -- 
> 2.53.0
> 
> From 2771375662c9edce25d7268bc71cc6db35a0d5c7 Mon Sep 17 00:00:00 2001
> From: Lukas Herbolt <lukas@herbolt.com>
> Date: Fri, 26 Sep 2025 12:48:39 +0200
> Subject: [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
> 
> 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.
> 
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
>  mkfs/xfs_mkfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index b34407725f76..1b6334e9adce 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3647,7 +3647,7 @@ check_lsunit:
>  	else if (cfg->lsectorsize > XLOG_HEADER_SIZE)
>  		lsu = cfg->blocksize; /* lsunit matches filesystem block size */
>  
> -	if (lsu) {
> +	if (cli->lsu) {
>  		/* verify if lsu is a multiple block size */
>  		if (lsu % cfg->blocksize != 0) {
>  			fprintf(stderr,
> -- 
> 2.53.0
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
  2026-02-19 11:44 ` [PATCH 1/1] " Lukas Herbolt
  2026-02-19 13:32   ` Christoph Hellwig
  2026-03-04 19:49   ` Darrick J. Wong
@ 2026-06-15 13:17   ` Jan Kara
  2026-06-22  9:07     ` Carlos Maiolino
  2 siblings, 1 reply; 8+ messages in thread
From: Jan Kara @ 2026-06-15 13:17 UTC (permalink / raw)
  To: Lukas Herbolt; +Cc: hch, aalbersh, cem, linux-xfs, Darrick J. Wong, ailiopoulos

On Thu 19-02-26 12:44:09, 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.
> 
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>

During our performance testing we have noticed that this patch changes the
size of log stripe unit on a disk behind megaraid_sas controller from 4k to
64k (which is understandable as the disk reports 64k minimum_io_size). This
actually causes a performance regression (about 40%) for dbench with 1
client as it is very fsync-heavy and thus commits lots of tiny transactions
and with larger log blocks the logging overhead just seems larger. dbench
is kind of pathological load but I assume other more relevant fsync
intensive loads could experience a regression as well.

I think one of the reasons why 4k log IO actually ends up performing better
with this disk is that the controller has a battery backed cache (a few GB
large) and thus it's able to absorb smaller log writes and merge them into
properly sized IO so larger half-empty log blocks are just pure overhead
for it.

This commit sadly doesn't contain any quantification how bad small log IO
is for 4kN or 512e drives so that one could weight it against the
regression for higher-end storage. Anyway, I'm not sure something needs to
be done here as there likely isn't a value that works for everyone. I just
wanted to provide a data point that this isn't a universal win and maybe it
needs to be considered what's the most sensible default.

								Honza


> ---
>  mkfs/xfs_mkfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index b34407725f76..1b6334e9adce 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3647,7 +3647,7 @@ check_lsunit:
>  	else if (cfg->lsectorsize > XLOG_HEADER_SIZE)
>  		lsu = cfg->blocksize; /* lsunit matches filesystem block size */
>  
> -	if (lsu) {
> +	if (cli->lsu) {
>  		/* verify if lsu is a multiple block size */
>  		if (lsu % cfg->blocksize != 0) {
>  			fprintf(stderr,
> -- 
> 2.53.0
> 
> >From 2771375662c9edce25d7268bc71cc6db35a0d5c7 Mon Sep 17 00:00:00 2001
> From: Lukas Herbolt <lukas@herbolt.com>
> Date: Fri, 26 Sep 2025 12:48:39 +0200
> Subject: [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
> 
> 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.
> 
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
>  mkfs/xfs_mkfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index b34407725f76..1b6334e9adce 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3647,7 +3647,7 @@ check_lsunit:
>  	else if (cfg->lsectorsize > XLOG_HEADER_SIZE)
>  		lsu = cfg->blocksize; /* lsunit matches filesystem block size */
>  
> -	if (lsu) {
> +	if (cli->lsu) {
>  		/* verify if lsu is a multiple block size */
>  		if (lsu % cfg->blocksize != 0) {
>  			fprintf(stderr,
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
  2026-06-15 13:17   ` Jan Kara
@ 2026-06-22  9:07     ` Carlos Maiolino
  2026-06-22 10:05       ` Jan Kara
  0 siblings, 1 reply; 8+ messages in thread
From: Carlos Maiolino @ 2026-06-22  9:07 UTC (permalink / raw)
  To: Jan Kara
  Cc: Lukas Herbolt, hch, aalbersh, linux-xfs, Darrick J. Wong,
	ailiopoulos

On Mon, Jun 15, 2026 at 03:17:29PM +0200, Jan Kara wrote:
> On Thu 19-02-26 12:44:09, 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.
> > 
> > Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> 
> During our performance testing we have noticed that this patch changes the
> size of log stripe unit on a disk behind megaraid_sas controller from 4k to
> 64k (which is understandable as the disk reports 64k minimum_io_size). This
> actually causes a performance regression (about 40%) for dbench with 1
> client as it is very fsync-heavy and thus commits lots of tiny transactions
> and with larger log blocks the logging overhead just seems larger. dbench
> is kind of pathological load but I assume other more relevant fsync
> intensive loads could experience a regression as well.
> 
> I think one of the reasons why 4k log IO actually ends up performing better
> with this disk is that the controller has a battery backed cache (a few GB
> large) and thus it's able to absorb smaller log writes and merge them into
> properly sized IO so larger half-empty log blocks are just pure overhead
> for it.

Sorry the late reply FWIW...

So those seems poorly designed writes which just happened to perform ok
by accident. Properly aligning it to 64k to match its min_io_size is the
right thing to do to avoid lots of RMW cycles for once. But I'm pretty
sure you already know that :)

> 
> This commit sadly doesn't contain any quantification how bad small log IO
> is for 4kN or 512e drives so that one could weight it against the
> regression for higher-end storage.
> Anyway, I'm not sure something needs to
> be done here as there likely isn't a value that works for everyone. I just
> wanted to provide a data point that this isn't a universal win and maybe it
> needs to be considered what's the most sensible default.

Well, I don't think 'fsync heavy' applications are the default, or if
they are, specially on high-end systems, then I'd argue the application
ought to know what it's doing. I do agree that's no universal win
anywhere and yet seem to fix the sunit to the hardware reported size is
the right thing to do here.

Perhaps you could leverage mkfs config files to set different default
configs where you folks believe fits better your usecase?

Cheers.

> 
> 
> > ---
> >  mkfs/xfs_mkfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index b34407725f76..1b6334e9adce 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -3647,7 +3647,7 @@ check_lsunit:
> >  	else if (cfg->lsectorsize > XLOG_HEADER_SIZE)
> >  		lsu = cfg->blocksize; /* lsunit matches filesystem block size */
> >  
> > -	if (lsu) {
> > +	if (cli->lsu) {
> >  		/* verify if lsu is a multiple block size */
> >  		if (lsu % cfg->blocksize != 0) {
> >  			fprintf(stderr,
> > -- 
> > 2.53.0
> > 
> > >From 2771375662c9edce25d7268bc71cc6db35a0d5c7 Mon Sep 17 00:00:00 2001
> > From: Lukas Herbolt <lukas@herbolt.com>
> > Date: Fri, 26 Sep 2025 12:48:39 +0200
> > Subject: [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
> > 
> > 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.
> > 
> > Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> > ---
> >  mkfs/xfs_mkfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index b34407725f76..1b6334e9adce 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -3647,7 +3647,7 @@ check_lsunit:
> >  	else if (cfg->lsectorsize > XLOG_HEADER_SIZE)
> >  		lsu = cfg->blocksize; /* lsunit matches filesystem block size */
> >  
> > -	if (lsu) {
> > +	if (cli->lsu) {
> >  		/* verify if lsu is a multiple block size */
> >  		if (lsu % cfg->blocksize != 0) {
> >  			fprintf(stderr,
> > -- 
> > 2.53.0
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
  2026-06-22  9:07     ` Carlos Maiolino
@ 2026-06-22 10:05       ` Jan Kara
  2026-06-23 14:41         ` Christoph Hellwig
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Kara @ 2026-06-22 10:05 UTC (permalink / raw)
  To: Carlos Maiolino
  Cc: Jan Kara, Lukas Herbolt, hch, aalbersh, linux-xfs,
	Darrick J. Wong, ailiopoulos

On Mon 22-06-26 11:07:58, Carlos Maiolino wrote:
> On Mon, Jun 15, 2026 at 03:17:29PM +0200, Jan Kara wrote:
> > On Thu 19-02-26 12:44:09, 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.
> > > 
> > > Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> > 
> > During our performance testing we have noticed that this patch changes the
> > size of log stripe unit on a disk behind megaraid_sas controller from 4k to
> > 64k (which is understandable as the disk reports 64k minimum_io_size). This
> > actually causes a performance regression (about 40%) for dbench with 1
> > client as it is very fsync-heavy and thus commits lots of tiny transactions
> > and with larger log blocks the logging overhead just seems larger. dbench
> > is kind of pathological load but I assume other more relevant fsync
> > intensive loads could experience a regression as well.
> > 
> > I think one of the reasons why 4k log IO actually ends up performing better
> > with this disk is that the controller has a battery backed cache (a few GB
> > large) and thus it's able to absorb smaller log writes and merge them into
> > properly sized IO so larger half-empty log blocks are just pure overhead
> > for it.
> 
> Sorry the late reply FWIW...
> 
> So those seems poorly designed writes which just happened to perform ok
> by accident. Properly aligning it to 64k to match its min_io_size is the
> right thing to do to avoid lots of RMW cycles for once. But I'm pretty
> sure you already know that :)

Yes, I agree the XFS logging with the old defaults performed well on that
machine only due to the battery backed cache.
 
> > This commit sadly doesn't contain any quantification how bad small log IO
> > is for 4kN or 512e drives so that one could weight it against the
> > regression for higher-end storage.
> > Anyway, I'm not sure something needs to
> > be done here as there likely isn't a value that works for everyone. I just
> > wanted to provide a data point that this isn't a universal win and maybe it
> > needs to be considered what's the most sensible default.
> 
> Well, I don't think 'fsync heavy' applications are the default, or if
> they are, specially on high-end systems, then I'd argue the application
> ought to know what it's doing. I do agree that's no universal win
> anywhere and yet seem to fix the sunit to the hardware reported size is
> the right thing to do here.

Fair.

> Perhaps you could leverage mkfs config files to set different default
> configs where you folks believe fits better your usecase?

Thanks for the tip. Yes, we are considering that. I mostly wrote this email
to share the finding, make people aware, and gather some feedback about
what people think. What default we pick for our distro is a separate
discussion we need to do internally...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] mkfs.xfs fix sunit size on 512e and 4kN disks.
  2026-06-22 10:05       ` Jan Kara
@ 2026-06-23 14:41         ` Christoph Hellwig
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2026-06-23 14:41 UTC (permalink / raw)
  To: Jan Kara
  Cc: Carlos Maiolino, Lukas Herbolt, hch, aalbersh, linux-xfs,
	Darrick J. Wong, ailiopoulos, linux-scsi

On Mon, Jun 22, 2026 at 12:05:25PM +0200, Jan Kara wrote:
> > by accident. Properly aligning it to 64k to match its min_io_size is the
> > right thing to do to avoid lots of RMW cycles for once. But I'm pretty
> > sure you already know that :)
> 
> Yes, I agree the XFS logging with the old defaults performed well on that
> machine only due to the battery backed cache.

Which to me suggests it is reporting the wrong min_io_size.  But good
luck getting RAID controller firmware fixed.  Although maybe we should
quirk it in the driver?

> > Well, I don't think 'fsync heavy' applications are the default, or if
> > they are, specially on high-end systems, then I'd argue the application
> > ought to know what it's doing. I do agree that's no universal win
> > anywhere and yet seem to fix the sunit to the hardware reported size is
> > the right thing to do here.
> 
> Fair.

I think the real question is if the value/defaults have any benefits
for this setup.  And I somehow doubt it.


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-06-23 14:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-19 11:44 [PATCH 0/1] mkfs.xfs fix sunit size on 512e and 4kN disks Lukas Herbolt
2026-02-19 11:44 ` [PATCH 1/1] " Lukas Herbolt
2026-02-19 13:32   ` Christoph Hellwig
2026-03-04 19:49   ` Darrick J. Wong
2026-06-15 13:17   ` Jan Kara
2026-06-22  9:07     ` Carlos Maiolino
2026-06-22 10:05       ` Jan Kara
2026-06-23 14:41         ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox