On Wed, Jan 22, 2025 at 01:35:05PM +0800, liuhuan01@xxxxxxxxxx wrote:
> From: liuh <liuhuan01@xxxxxxxxxx>
>
> It does not take effect when maxpct is specified as 0.
>
> 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
>
> Signed-off-by: liuh <liuhuan01@xxxxxxxxxx>
> ---
> mkfs/xfs_mkfs.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 956cc295..6f0275d2 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1034,13 +1034,14 @@ struct cli_params {
> int proto_slashes_are_spaces;
> int data_concurrency;
> int log_concurrency;
> + int imaxpct;
> + int imaxpct_using_default;
>
> /* parameters where 0 is not a valid value */
> int64_t agcount;
> int64_t rgcount;
> int inodesize;
> int inopblock;
> - int imaxpct;
Why not set imaxpct to -1 when we initialize cfg in main() and then do
something like this in calculate_imaxpct():
if (cli->imaxpct >= 0) {
cfg->imaxpct = cli->imaxpct;
return;
}
/* existing 5% - 25% default calculation */
Instead of declaring extra variables?
Also, regression test needed here...
--D
> int lsectorsize;
> uuid_t uuid;
>
> @@ -1826,6 +1827,7 @@ inode_opts_parser(
> break;
> case I_MAXPCT:
> cli->imaxpct = getnum(value, opts, subopt);
> + cli->imaxpct_using_default = false;
> break;
> case I_PERBLOCK:
> cli->inopblock = getnum(value, opts, subopt);
> @@ -3835,7 +3837,7 @@ calculate_imaxpct(
> struct cli_params *cli)
> {
> cfg->imaxpct = cli->imaxpct;
> - if (cfg->imaxpct)
> + if (!cli->imaxpct_using_default)
> return;
>
> /*
> @@ -4891,6 +4893,7 @@ main(
> .data_concurrency = -1, /* auto detect non-mechanical storage */
> .log_concurrency = -1, /* auto detect non-mechanical ddev */
> .autofsck = FSPROP_AUTOFSCK_UNSET,
> + .imaxpct_using_default = true,
> };
> struct mkfs_params cfg = {};
>
> --
> 2.43.0
>
>
