Re: [PATCH 5/7] mkfs: autodetect log stripe unit for external log devices

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

On Wed, Jul 09, 2025 at 04:57:49PM +0100, John Garry wrote:
> On 01/07/2025 19:08, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > If we're using an external log device and the caller doesn't give us a
> > lsunit, use the block device geometry (if present) to set it.
> 
> this seems fine, but I am not imitatively familiar with the code. So, FWIW:
> 
> Reviewed-by: John Garry <john.g.garry@oracle.com>
> 
> There is a small question below, though.
> 
> > 
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> >   mkfs/xfs_mkfs.c |    4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > 
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index 8b946f3ef817da..6c8cc715d3476b 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> 
> nit: maybe the comment on method of calculation (not shown, but begins
> "check the log sunit is modulo ...") could be updated

Ok.

	/*
	 * check that log sunit is modulo fsblksize; default it to dsunit for
	 * an internal log; or the log device stripe unit if it's external.
	 */

Thanks for the review.

--D

> > @@ -3624,6 +3624,10 @@ _("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
> >   		   cfg->loginternal && cfg->dsunit) {
> >   		/* lsunit and dsunit now in fs blocks */
> >   		cfg->lsunit = cfg->dsunit;
> > +	} else if (cfg->sb_feat.log_version == 2 &&
> > +		   !cfg->loginternal) {
> > +		/* use the external log device properties */
> > +		cfg->lsunit = DTOBT(ft->log.sunit, cfg->blocklog);
> >   	}
> >   	if (cfg->sb_feat.log_version == 2 &&
> > 
> 
>