From: Carlos Maiolino <cmaiolino@redhat.com>
To: Pavel Reichl <preichl@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH v2] mkfs.xfs: fix ASSERT on too-small device with stripe geometry
Date: Wed, 23 Sep 2020 09:53:01 +0200 [thread overview]
Message-ID: <20200923075301.kriakz7ivnwhejje@eorzea> (raw)
In-Reply-To: <20200916204056.29247-1-preichl@redhat.com>
On Wed, Sep 16, 2020 at 10:40:56PM +0200, Pavel Reichl wrote:
> When a too-small device is created with stripe geometry, we hit an
> assert in align_ag_geometry():
>
> mkfs.xfs: xfs_mkfs.c:2834: align_ag_geometry: Assertion `cfg->agcount != 0' failed.
>
> This is because align_ag_geometry() finds that the size of the last
> (only) AG is too small, and attempts to trim it off. Obviously 0
> AGs is invalid, and we hit the ASSERT.
>
Patch looks good, next time please keep the Changelog :)
Cheers.
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> ---
> include/xfs_multidisk.h | 14 +++++++-------
> mkfs/xfs_mkfs.c | 6 +++---
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h
> index 1b9936ec..abfb50ce 100644
> --- a/include/xfs_multidisk.h
> +++ b/include/xfs_multidisk.h
> @@ -14,7 +14,6 @@
> #define XFS_DFL_BLOCKSIZE_LOG 12 /* 4096 byte blocks */
> #define XFS_DINODE_DFL_LOG 8 /* 256 byte inodes */
> #define XFS_DINODE_DFL_CRC_LOG 9 /* 512 byte inodes for CRCs */
> -#define XFS_MIN_DATA_BLOCKS 100
> #define XFS_MIN_INODE_PERBLOCK 2 /* min inodes per block */
> #define XFS_DFL_IMAXIMUM_PCT 25 /* max % of space for inodes */
> #define XFS_MIN_REC_DIRSIZE 12 /* 4096 byte dirblocks (V2) */
> @@ -25,13 +24,14 @@
> * accept w/o warnings
> */
>
> -#define XFS_AG_BYTES(bblog) ((long long)BBSIZE << (bblog))
> -#define XFS_AG_MIN_BYTES ((XFS_AG_BYTES(15))) /* 16 MB */
> -#define XFS_AG_MAX_BYTES ((XFS_AG_BYTES(31))) /* 1 TB */
> -#define XFS_AG_MIN_BLOCKS(blog) (XFS_AG_MIN_BYTES >> (blog))
> -#define XFS_AG_MAX_BLOCKS(blog) ((XFS_AG_MAX_BYTES - 1) >> (blog))
> +#define XFS_AG_BYTES(bblog) ((long long)BBSIZE << (bblog))
> +#define XFS_MIN_DATA_BLOCKS(cfg) (XFS_AG_MIN_BLOCKS((cfg)->blocklog))
> +#define XFS_AG_MIN_BYTES ((XFS_AG_BYTES(15))) /* 16 MB */
> +#define XFS_AG_MAX_BYTES ((XFS_AG_BYTES(31))) /* 1 TB */
> +#define XFS_AG_MIN_BLOCKS(blog) (XFS_AG_MIN_BYTES >> (blog))
> +#define XFS_AG_MAX_BLOCKS(blog) ((XFS_AG_MAX_BYTES - 1) >> (blog))
>
> -#define XFS_MAX_AGNUMBER ((xfs_agnumber_t)(NULLAGNUMBER - 1))
> +#define XFS_MAX_AGNUMBER ((xfs_agnumber_t)(NULLAGNUMBER - 1))
>
> /*
> * These values define what we consider a "multi-disk" filesystem. That is, a
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index a687f385..204dfff1 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -2530,10 +2530,10 @@ _("size %s specified for data subvolume is too large, maximum is %lld blocks\n")
> cfg->dblocks = DTOBT(xi->dsize, cfg->blocklog);
> }
>
> - if (cfg->dblocks < XFS_MIN_DATA_BLOCKS) {
> + if (cfg->dblocks < XFS_MIN_DATA_BLOCKS(cfg)) {
> fprintf(stderr,
> -_("size %lld of data subvolume is too small, minimum %d blocks\n"),
> - (long long)cfg->dblocks, XFS_MIN_DATA_BLOCKS);
> +_("size %lld of data subvolume is too small, minimum %lld blocks\n"),
> + (long long)cfg->dblocks, XFS_MIN_DATA_BLOCKS(cfg));
> usage();
> }
>
> --
> 2.26.2
>
--
Carlos
next prev parent reply other threads:[~2020-09-23 7:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-16 20:40 [PATCH v2] mkfs.xfs: fix ASSERT on too-small device with stripe geometry Pavel Reichl
2020-09-17 8:10 ` Christoph Hellwig
2020-09-23 7:53 ` Carlos Maiolino [this message]
-- strict thread matches above, loose matches on Subject: below --
2020-09-14 18:26 [PATCH] " Eric Sandeen
2020-09-14 19:00 ` [PATCH V2] " Eric Sandeen
2020-09-14 19:24 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200923075301.kriakz7ivnwhejje@eorzea \
--to=cmaiolino@redhat.com \
--cc=linux-xfs@vger.kernel.org \
--cc=preichl@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.