| Message ID | 20230915102246.108709-1-cem@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V2] mkfs: Improve warning when AG size is a multiple of stripe width | expand |
On Fri, Sep 15, 2023 at 12:22:46PM +0200, cem@kernel.org wrote: > From: Carlos Maiolino <cmaiolino@redhat.com> > > The current output message prints out a suggestion of an AG size to be > used in lieu of the user-defined one. > The problem is this suggestion is printed in filesystem blocks, without > specifying the suffix to be used. > > This patch tries to make user's life easier by outputing the option as > it should be used by the mkfs, so users can just copy/paste it. > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Reviewed-by: Bill O'Donnell <bodonnel@redhat.com> > --- > > V2: > - Keep printing it in FSBs, just add the agsize= and the 'b' suffix at > the end > > mkfs/xfs_mkfs.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index d3a15cf44..b61934e57 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -3179,9 +3179,9 @@ _("agsize rounded to %lld, sunit = %d\n"), > if (cli_opt_set(&dopts, D_AGCOUNT) || > cli_opt_set(&dopts, D_AGSIZE)) { > 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"), > +"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: agsize=%llub\n"), > (unsigned long long)cfg->agsize - dsunit); > fflush(stdout); > goto validate; > -- > 2.39.2 >
On Fri, Sep 15, 2023 at 12:22:46PM +0200, cem@kernel.org wrote: > From: Carlos Maiolino <cmaiolino@redhat.com> > > The current output message prints out a suggestion of an AG size to be > used in lieu of the user-defined one. > The problem is this suggestion is printed in filesystem blocks, without > specifying the suffix to be used. > > This patch tries to make user's life easier by outputing the option as > it should be used by the mkfs, so users can just copy/paste it. > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> I'm fine with this but, as I said in my previous comment, a man page update for agsize is also needed. Something like this: agsize=value This is an alternative to using the agcount suboption. The value is the desired size of the - allocation group expressed in bytes (usually using - the m or g suffixes). This value must be a multiple - of the filesystem block size, and must be at least + allocation group expressed as a multiple of the + filesystem block size. It must be at least 16MiB, and no more than 1TiB, and may be automatically adjusted to properly align with the stripe geometry. The agcount and agsize suboptions are mutually exclusive. -Dave.
On Mon, Sep 18, 2023 at 08:52:49AM +1000, Dave Chinner wrote: > On Fri, Sep 15, 2023 at 12:22:46PM +0200, cem@kernel.org wrote: > > From: Carlos Maiolino <cmaiolino@redhat.com> > > > > The current output message prints out a suggestion of an AG size to be > > used in lieu of the user-defined one. > > The problem is this suggestion is printed in filesystem blocks, without > > specifying the suffix to be used. > > > > This patch tries to make user's life easier by outputing the option as > > it should be used by the mkfs, so users can just copy/paste it. > > > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > > I'm fine with this but, as I said in my previous comment, a man page > update for agsize is also needed. Something like this: Thanks, yes I am planning for the manpage update, I just wanted to send the message fix first before going into the discussion of the manpage, I'll try to prepare a patch for it today. > > agsize=value > This is an alternative to using the agcount > suboption. The value is the desired size of the > - allocation group expressed in bytes (usually using > - the m or g suffixes). This value must be a multiple > - of the filesystem block size, and must be at least > + allocation group expressed as a multiple of the > + filesystem block size. It must be at least > 16MiB, and no more than 1TiB, and may be > automatically adjusted to properly align with the > stripe geometry. The agcount and agsize suboptions > are mutually exclusive. Thanks for the suggestion. Carlos > > -Dave. > -- > Dave Chinner > david@fromorbit.com
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index d3a15cf44..b61934e57 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -3179,9 +3179,9 @@ _("agsize rounded to %lld, sunit = %d\n"), if (cli_opt_set(&dopts, D_AGCOUNT) || cli_opt_set(&dopts, D_AGSIZE)) { 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"), +"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: agsize=%llub\n"), (unsigned long long)cfg->agsize - dsunit); fflush(stdout); goto validate;