| Message ID | 20250124065510.11574-1-liuhuan01@kylinos.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mkfs: fix the issue of maxpct set to 0 not taking effect | expand |
On Fri, Jan 24, 2025 at 02:55:10PM +0800, liuhuan01@kylinos.cn wrote: > From: liuh <liuhuan01@kylinos.cn> > > It does not take effect when maxpct is specified as 0. Style note: Please don't start the first sentence of the first paragraph with "It". Introduce what you're talking about, e.g. "If a filesystem has the sb_imax_pct field set to zero, there is no limit to the number of inode blocks in the filesystem." > Firstly, the man mkfs.xfs shows that setting maxpct to 0 means that all of the filesystem can become inode blocks. > However, when using mkfs.xfs and specifying maxpct = 0, the result is not as expected. > [root@fs ~]# mkfs.xfs -f -i maxpct=0 xfs.img > data = bsize=4096 blocks=262144, imaxpct=25 > = sunit=0 swidth=0 blks > > The reason is that the judging condition will never succeed when specifying maxpct = 0. As a result, the default algorithm was applied. > cfg->imaxpct = cli->imaxpct; > if (cfg->imaxpct) > return; > It's important that maxpct can be set to 0 within the kernel xfs code. > > The result with patch: > [root@fs ~]# mkfs.xfs -f -i maxpct=0 xfs.img > data = bsize=4096 blocks=262144, imaxpct=0 > = sunit=0 swidth=0 blks > > [root@fs ~]# mkfs.xfs -f xfs.img > data = bsize=4096 blocks=262144, imaxpct=25 > = sunit=0 swidth=0 blks > > Signed-off-by: liuh <liuhuan01@kylinos.cn> Bugfixes really ought to have a fixes tag, even for xfsprogs. > --- > mkfs/xfs_mkfs.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index 956cc295..f4216000 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -1034,13 +1034,13 @@ struct cli_params { > int proto_slashes_are_spaces; > int data_concurrency; > int log_concurrency; > + int imaxpct; > > /* parameters where 0 is not a valid value */ > int64_t agcount; > int64_t rgcount; > int inodesize; > int inopblock; > - int imaxpct; > int lsectorsize; > uuid_t uuid; > > @@ -3834,9 +3834,10 @@ calculate_imaxpct( > struct mkfs_params *cfg, > struct cli_params *cli) > { > - cfg->imaxpct = cli->imaxpct; > - if (cfg->imaxpct) > + if (cli->imaxpct >= 0) { > + cfg->imaxpct = cli->imaxpct; > return; > + } > > /* > * This returns the % of the disk space that is used for > @@ -4891,6 +4892,7 @@ main( > .data_concurrency = -1, /* auto detect non-mechanical storage */ > .log_concurrency = -1, /* auto detect non-mechanical ddev */ > .autofsck = FSPROP_AUTOFSCK_UNSET, > + .imaxpct = -1, Might want to leave a comment about what -1 means? .imaxpct = -1, /* set sb_imax_pct automatically */ With those bits cleaned up and this added: Cc: <linux-xfs@vger.kernel.org> # v4.15.0 Fixes: d7240c965389e1 ("mkfs: rework imaxpct calculation") You can add: Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > }; > struct mkfs_params cfg = {}; > > -- > 2.43.0 > >
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index 956cc295..f4216000 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -1034,13 +1034,13 @@ struct cli_params { int proto_slashes_are_spaces; int data_concurrency; int log_concurrency; + int imaxpct; /* parameters where 0 is not a valid value */ int64_t agcount; int64_t rgcount; int inodesize; int inopblock; - int imaxpct; int lsectorsize; uuid_t uuid; @@ -3834,9 +3834,10 @@ calculate_imaxpct( struct mkfs_params *cfg, struct cli_params *cli) { - cfg->imaxpct = cli->imaxpct; - if (cfg->imaxpct) + if (cli->imaxpct >= 0) { + cfg->imaxpct = cli->imaxpct; return; + } /* * This returns the % of the disk space that is used for @@ -4891,6 +4892,7 @@ main( .data_concurrency = -1, /* auto detect non-mechanical storage */ .log_concurrency = -1, /* auto detect non-mechanical ddev */ .autofsck = FSPROP_AUTOFSCK_UNSET, + .imaxpct = -1, }; struct mkfs_params cfg = {};