[PATCH] mkfs: warning about misaligned AGs and RAID stripes is not an error

[PATCH] mkfs: warning about misaligned AGs and RAID stripes is not an error

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

I've noticed a fair number of fstests failures when we create a scratch
device on a RAID device and the test specifies an explicit AG count or
AG size:

--- /tmp/fstests/tests/xfs/042.out	2022-09-01 15:09:11.484679979 -0700
+++ /var/tmp/fstests/xfs/042.out.bad	2023-04-25 19:59:04.040000000 -0700
@@ -1,5 +1,8 @@
 QA output created by 042
-Make a 96 megabyte filesystem on SCRATCH_DEV and mount... done
+Make a 96 megabyte filesystem on SCRATCH_DEV and mount... Warning: AG size is a multiple of stripe width.  This can cause performance
+problems by aligning all AGs on the same disk.  To avoid this, run mkfs with
+an AG size that is one stripe unit smaller or larger, for example 8160.
+done

Emitting this warning on stderr is silly -- nothing has failed, and we
aren't going to abort the format either.  Send the warning to stdout.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 mkfs/xfs_mkfs.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 6dc0f335..2f2995e1 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3167,11 +3167,12 @@ _("agsize rounded to %lld, sunit = %d\n"),
 
 		if (cli_opt_set(&dopts, D_AGCOUNT) ||
 		    cli_opt_set(&dopts, D_AGSIZE)) {
-			fprintf(stderr, _(
+			printf(_(
 "Warning: AG size is a multiple of stripe width.  This can cause performance\n\
 problems by aligning all AGs on the same disk.  To avoid this, run mkfs with\n\
 an AG size that is one stripe unit smaller or larger, for example %llu.\n"),
 				(unsigned long long)cfg->agsize - dsunit);
+			fflush(stdout);
 			goto validate;
 		}
 

Re: [PATCH] mkfs: warning about misaligned AGs and RAID stripes is not an error

[Carlos Maiolino]

On Thu, Apr 27, 2023 at 03:45:40PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> I've noticed a fair number of fstests failures when we create a scratch
> device on a RAID device and the test specifies an explicit AG count or
> AG size:
> 
> --- /tmp/fstests/tests/xfs/042.out	2022-09-01 15:09:11.484679979 -0700
> +++ /var/tmp/fstests/xfs/042.out.bad	2023-04-25 19:59:04.040000000 -0700
> @@ -1,5 +1,8 @@
>  QA output created by 042
> -Make a 96 megabyte filesystem on SCRATCH_DEV and mount... done
> +Make a 96 megabyte filesystem on SCRATCH_DEV and mount... Warning: AG size is a multiple of stripe width.  This can cause performance
> +problems by aligning all AGs on the same disk.  To avoid this, run mkfs with
> +an AG size that is one stripe unit smaller or larger, for example 8160.
> +done
> 
> Emitting this warning on stderr is silly -- nothing has failed, and we
> aren't going to abort the format either.  Send the warning to stdout.

I agree with it, looks better printing it out to stdout other than stderr, so,
with one caveat below:
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

I think this might impact user tools monitoring stderr, but for mkfs I don't
think it's a big deal, just wanted to raise this concern.

> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  mkfs/xfs_mkfs.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 6dc0f335..2f2995e1 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3167,11 +3167,12 @@ _("agsize rounded to %lld, sunit = %d\n"),
> 
>  		if (cli_opt_set(&dopts, D_AGCOUNT) ||
>  		    cli_opt_set(&dopts, D_AGSIZE)) {
> -			fprintf(stderr, _(
> +			printf(_(
>  "Warning: AG size is a multiple of stripe width.  This can cause performance\n\
>  problems by aligning all AGs on the same disk.  To avoid this, run mkfs with\n\
>  an AG size that is one stripe unit smaller or larger, for example %llu.\n"),
>  				(unsigned long long)cfg->agsize - dsunit);
> +			fflush(stdout);
>  			goto validate;
>  		}
> 

-- 
Carlos Maiolino