* [PATCH v2] mkfs.xfs: fix ASSERT on too-small device with stripe geometry
@ 2020-09-16 20:40 Pavel Reichl
2020-09-17 8:10 ` Christoph Hellwig
2020-09-23 7:53 ` Carlos Maiolino
0 siblings, 2 replies; 5+ messages in thread
From: Pavel Reichl @ 2020-09-16 20:40 UTC (permalink / raw)
To: linux-xfs
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.
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] mkfs.xfs: fix ASSERT on too-small device with stripe geometry
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
1 sibling, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2020-09-17 8:10 UTC (permalink / raw)
To: Pavel Reichl; +Cc: linux-xfs
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.
>
> Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
> Suggested-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] mkfs.xfs: fix ASSERT on too-small device with stripe geometry
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
1 sibling, 0 replies; 5+ messages in thread
From: Carlos Maiolino @ 2020-09-23 7:53 UTC (permalink / raw)
To: Pavel Reichl; +Cc: linux-xfs
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] mkfs.xfs: fix ASSERT on too-small device with stripe geometry
2020-09-14 19:00 ` [PATCH V2] " Eric Sandeen
@ 2020-09-14 19:24 ` Darrick J. Wong
0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2020-09-14 19:24 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs, Zdenek Kabelac
On Mon, Sep 14, 2020 at 02:00:52PM -0500, Eric Sandeen wrote:
> When a too-small device is created with stripe geometry, we hit an
> assert in align_ag_geometry():
>
> # truncate --size=10444800 testfile
> # mkfs.xfs -dsu=65536,sw=1 testfile
> 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.
>
> Fix this by skipping the last-ag-trim if there is only one AG, and
> add a new test to validate_ag_geometry() which offers a very specific,
> clear warning if the device (in dblocks) is smaller than the minimum
> allowed AG size.
>
> Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Seems fine to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
>
> V2: remove stray printf, sorry
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index a687f385..2139aedb 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1038,6 +1038,15 @@ validate_ag_geometry(
> uint64_t agsize,
> uint64_t agcount)
> {
> + /* Is this device simply too small? */
> + if (dblocks < XFS_AG_MIN_BLOCKS(blocklog)) {
> + fprintf(stderr,
> + _("device (%lld blocks) too small, need at least %lld blocks\n"),
> + (long long)dblocks,
> + (long long)XFS_AG_MIN_BLOCKS(blocklog));
> + usage();
> + }
> +
> if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
> fprintf(stderr,
> _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
> @@ -2827,11 +2836,11 @@ validate:
> * and drop the blocks.
> */
> if (cfg->dblocks % cfg->agsize != 0 &&
> + cfg->agcount > 1 &&
> (cfg->dblocks % cfg->agsize < XFS_AG_MIN_BLOCKS(cfg->blocklog))) {
> ASSERT(!cli_opt_set(&dopts, D_AGCOUNT));
> cfg->dblocks = (xfs_rfsblock_t)((cfg->agcount - 1) * cfg->agsize);
> cfg->agcount--;
> - ASSERT(cfg->agcount != 0);
> }
>
> validate_ag_geometry(cfg->blocklog, cfg->dblocks,
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V2] mkfs.xfs: fix ASSERT on too-small device with stripe geometry
2020-09-14 18:26 [PATCH] " Eric Sandeen
@ 2020-09-14 19:00 ` Eric Sandeen
2020-09-14 19:24 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Eric Sandeen @ 2020-09-14 19:00 UTC (permalink / raw)
To: xfs; +Cc: Zdenek Kabelac
When a too-small device is created with stripe geometry, we hit an
assert in align_ag_geometry():
# truncate --size=10444800 testfile
# mkfs.xfs -dsu=65536,sw=1 testfile
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.
Fix this by skipping the last-ag-trim if there is only one AG, and
add a new test to validate_ag_geometry() which offers a very specific,
clear warning if the device (in dblocks) is smaller than the minimum
allowed AG size.
Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: remove stray printf, sorry
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index a687f385..2139aedb 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1038,6 +1038,15 @@ validate_ag_geometry(
uint64_t agsize,
uint64_t agcount)
{
+ /* Is this device simply too small? */
+ if (dblocks < XFS_AG_MIN_BLOCKS(blocklog)) {
+ fprintf(stderr,
+ _("device (%lld blocks) too small, need at least %lld blocks\n"),
+ (long long)dblocks,
+ (long long)XFS_AG_MIN_BLOCKS(blocklog));
+ usage();
+ }
+
if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
fprintf(stderr,
_("agsize (%lld blocks) too small, need at least %lld blocks\n"),
@@ -2827,11 +2836,11 @@ validate:
* and drop the blocks.
*/
if (cfg->dblocks % cfg->agsize != 0 &&
+ cfg->agcount > 1 &&
(cfg->dblocks % cfg->agsize < XFS_AG_MIN_BLOCKS(cfg->blocklog))) {
ASSERT(!cli_opt_set(&dopts, D_AGCOUNT));
cfg->dblocks = (xfs_rfsblock_t)((cfg->agcount - 1) * cfg->agsize);
cfg->agcount--;
- ASSERT(cfg->agcount != 0);
}
validate_ag_geometry(cfg->blocklog, cfg->dblocks,
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-23 7:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
-- 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
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.