From: "Darrick J. Wong" <djwong@kernel.org>
To: Lukas Herbolt <lukas@herbolt.com>
Cc: sandeen@sandeen.net, aalbersh@kernel.org, linux-xfs@vger.kernel.org
Subject: Re: [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file.
Date: Thu, 14 May 2026 09:27:18 -0700 [thread overview]
Message-ID: <20260514162718.GX9555@frogsfrogsfrogs> (raw)
In-Reply-To: <20260514143716.893814-3-lukas@herbolt.com>
On Thu, May 14, 2026 at 04:37:17PM +0200, Lukas Herbolt wrote:
> Various users may prefer different default values. Having a default config
> file will allow them to utilize it without the need specifying configuration
> file on command line.
>
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> ---
> include/builddefs.in | 1 +
> mkfs/Makefile | 3 ++
> mkfs/mkfs.xfs.conf | 50 ++++++++++++++++++++++++++++++
> mkfs/xfs_mkfs.c | 72 +++++++++++++++++++++++++++++++++++++++-----
> 4 files changed, 119 insertions(+), 7 deletions(-)
> create mode 100644 mkfs/mkfs.xfs.conf
>
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 3b52d1afd703..b635a7cd08a6 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -59,6 +59,7 @@ PKG_DOC_DIR = @datadir@/doc/@pkg_name@
> PKG_LOCALE_DIR = @datadir@/locale
> PKG_DATA_DIR = @datadir@/@pkg_name@
> MKFS_CFG_DIR = @datadir@/@pkg_name@/mkfs
> +MKFS_SYSCONF_DIR = @sysconfdir@
> PKG_STATE_DIR = @localstatedir@/lib/@pkg_name@
>
> XFS_SCRUB_ALL_AUTO_MEDIA_SCAN_STAMP=$(PKG_STATE_DIR)/xfs_scrub_all_media.stamp
> diff --git a/mkfs/Makefile b/mkfs/Makefile
> index fb1473324cde..57cee687eb1e 100644
> --- a/mkfs/Makefile
> +++ b/mkfs/Makefile
> @@ -21,6 +21,7 @@ CFGFILES = \
> lts_6.12.conf \
> lts_6.18.conf
>
> +LCFLAGS += -DMKFS_CFG_DIR=\"$(MKFS_CFG_DIR)\" -DMKFS_SYSCONF_DIR=\"$(MKFS_SYSCONF_DIR)\"
> LLDLIBS += $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBRT) $(LIBBLKID) \
> $(LIBUUID) $(LIBINIH) $(LIBURCU) $(LIBPTHREAD)
> LTDEPENDENCIES += $(LIBXFS) $(LIBXCMD) $(LIBFROG)
> @@ -45,6 +46,8 @@ install: default
> $(INSTALL) -m 755 $(XFS_PROTOFILE) $(PKG_SBIN_DIR)/xfs_protofile
> $(INSTALL) -m 755 -d $(MKFS_CFG_DIR)
> $(INSTALL) -m 644 $(CFGFILES) $(MKFS_CFG_DIR)
> + $(INSTALL) -m 755 -d $(MKFS_SYSCONF_DIR)
> + $(INSTALL) -m 644 mkfs.xfs.conf $(MKFS_SYSCONF_DIR)
At first I thought "gee, are we not supposed to dump config files into
/etc anymore?" but maybe that's a crazy ro-root/systemd thing. I guess
that at least it /is/ in the spirit of /etc/mke2fs.conf.
> install-dev:
>
> diff --git a/mkfs/mkfs.xfs.conf b/mkfs/mkfs.xfs.conf
> new file mode 100644
> index 000000000000..76f5ab4d4d8e
> --- /dev/null
> +++ b/mkfs/mkfs.xfs.conf
> @@ -0,0 +1,50 @@
> +# mkfs.xfs default configuration file
> +#
> +# This file documents some of the options recognised by mkfs.xfs config file.
> +# Adjust any value to override it system-wide.
> +#
> +# Command-line options always take precedence over values in this file.
> +# A specific config file can be selected with: mkfs.xfs -c options=<path>
> +
> +[block]
> +#size = 4096
> +
> +[metadata]
> +#crc = 1
> +#finobt = 1
> +#inobtcount = 1
> +#rmapbt = 1
> +#reflink = 1
> +#bigtime = 1
> +#metadir = 0
> +#autofsck = 0
> +
> +[inode]
> +#align = 1
> +# The default value is 25% for filesystems under 1TB,#5% for filesystems under
> +# 50TB and 1% for filesystems over 50TB.
> +#
> +#maxpct = 25
> +#size = 512
> +#perblock = 8
> +#attr = 2
> +#projid32bit = 1
> +#sparse = 1
> +#nrext64 = 1
> +#exchange = 1
> +
> +[data]
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
> +
> +[log]
> +#internal = 1
> +#version = 2
> +#lazy-count = 1
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
> +
> +[realtime]
> +# -1 = autodetect (use CPU count only on solid-state devices), 0 = disabled
> +#concurrency = -1
Oh, I see. The "-1" changes scattered throughout are to make it so that
you can specify that as a concurrency= option. I had mistakenly thought
pre-coffee that they were adding a new value.
However, I think "concurrency=auto" would be more ergonomic for users.
/*
* "nr_cpus" or "1" means set the concurrency level to the CPU count.
* If this cannot be determined, fall back to the default AG geometry.
* "auto" means use CPU count only solid-state devices
*/
if (!value || !strcmp(value, "nr_cpus"))
optnum = 1;
else if (!strcmp(value, "auto"))
optnum = -1;
else
optnum = getnum(value, opts, subopt);
if (optnum == 1)
cli->data_concurrency = nr_cpus();
else
cli->data_concurrency = optnum;
Also note that the explicit strcmp carveout for "auto" (and "nr_cpus")
means you don't have to tweak minval below.
Whatever we end up adding for a "just use the defaults" value, that
ought to be a separate patch from the one that adds the defaults config
file.
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index dd8a48c3633e..dbf15eca3442 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -44,6 +44,11 @@
> */
> #define WHACK_SIZE (128 * 1024)
>
> +/*
> + * Default configuration file which can keep distro specific values.
> + */
> +#define MKFS_DEFAULT_CFGFILE MKFS_SYSCONF_DIR "/mkfs.xfs.conf"
> +
> /*
> * XXX: The configured block and sector sizes are defined as global variables so
> * that they don't need to be passed to getnum/cvtnum().
> @@ -51,6 +56,11 @@
> static unsigned int blocksize;
> static unsigned int sectorsize;
>
> +/*
> + * Set to true while parsing the config file so option handlers know the source
> + */
> +static bool parsing_cfgfile;
> +
> /*
> * Enums for each CLI parameter type are declared first so we can calculate the
> * maximum array size needed to hold them automatically.
> @@ -264,6 +274,7 @@ struct opt_params {
> bool str_seen;
> bool convert;
> bool is_power_2;
> + bool from_file;
> struct _conflict {
> struct opt_params *opts;
> int subopt;
> @@ -472,7 +483,7 @@ static struct opt_params dopts = {
> .conflicts = { { &dopts, D_AGCOUNT },
> { &dopts, D_AGSIZE },
> { NULL, LAST_CONFLICT } },
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -672,7 +683,7 @@ static struct opt_params lopts = {
> { &lopts, L_FILE },
> { &lopts, L_DEV },
> { NULL, LAST_CONFLICT } },
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -827,7 +838,7 @@ static struct opt_params ropts = {
> { &ropts, R_RGSIZE },
> { NULL, LAST_CONFLICT } },
> .convert = true,
> - .minval = 0,
> + .minval = -1,
> .maxval = INT_MAX,
> .defaultval = 1,
> },
> @@ -1072,6 +1083,7 @@ struct cli_params {
> int blocksize;
>
> char *cfgfile;
> + bool cfgfile_had_options;
> char *protofile;
>
> enum fsprop_autofsck autofsck;
> @@ -1654,10 +1666,12 @@ check_opt(
> if (sp->seen)
> respec(opts->name, opts->subopts, index);
> sp->seen = true;
> + sp->from_file = parsing_cfgfile;
> } else {
> if (sp->str_seen)
> respec(opts->name, opts->subopts, index);
> sp->str_seen = true;
> + sp->from_file = parsing_cfgfile;
> }
>
> /* check for conflicts with the option */
> @@ -1806,6 +1820,7 @@ set_data_concurrency(
> /*
> * "nr_cpus" or "1" means set the concurrency level to the CPU count.
> * If this cannot be determined, fall back to the default AG geometry.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -1814,6 +1829,8 @@ set_data_concurrency(
>
> if (optnum == 1)
> cli->data_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->data_concurrency = -1;
> else
> cli->data_concurrency = optnum;
> }
> @@ -1954,6 +1971,7 @@ set_log_concurrency(
> /*
> * "nr_cpus" or 1 means set the concurrency level to the CPU count. If
> * this cannot be determined, fall back to the default computation.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -1962,6 +1980,8 @@ set_log_concurrency(
>
> if (optnum == 1)
> cli->log_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->log_concurrency = -1;
> else
> cli->log_concurrency = optnum;
> }
> @@ -2175,6 +2195,7 @@ set_rtvol_concurrency(
> * "nr_cpus" or "1" means set the concurrency level to the CPU count.
> * If this cannot be determined, fall back to the default rtgroup
> * geometry.
> + * -1 means autodetect (use CPU count only on solid-state devices).
> */
> if (!value || !strcmp(value, "nr_cpus"))
> optnum = 1;
> @@ -2183,6 +2204,8 @@ set_rtvol_concurrency(
>
> if (optnum == 1)
> cli->rtvol_concurrency = nr_cpus();
> + else if (optnum == -1)
> + cli->rtvol_concurrency = -1;
> else
> cli->rtvol_concurrency = optnum;
> }
> @@ -2336,9 +2359,32 @@ parse_cfgopt(
> if (!subopts[i])
> break;
> if (strcasecmp(name, subopts[i]) == 0) {
> + struct subopt_param *sp = &sop->opts->subopt_params[i];
> + int j;
> +
> + /*
> + * Command line options take precedence over config file
> + * options. If this option or any option that conflicts
> + * with it was already set from the command line, skip
> + * the config file value silently.
Currently, config file options take precedence over CLI options:
$ mkfs.xfs -c options=/usr/share/xfsprogs/mkfs/lts_6.6.conf -i nrext64=0 /tmp/a
-i nrext64 option respecified
So I guess this patch is changing that policy too?
--D
> + */
> + if ((sp->seen || sp->str_seen) && !sp->from_file)
> + return true;
> + for (j = 0; j < MAX_CONFLICTS; j++) {
> + struct _conflict *con = &sp->conflicts[j];
> + struct subopt_param *csp;
> +
> + if (con->subopt == LAST_CONFLICT)
> + break;
> + csp = &con->opts->subopt_params[con->subopt];
> + if ((csp->seen || csp->str_seen) && !csp->from_file)
> + return true;
> + }
> +
> ret = (sop->parser)(sop->opts, i, value, cli);
> if (ret)
> goto invalid_opt;
> + cli->cfgfile_had_options = true;
> return true;
> }
> }
> @@ -5749,10 +5795,21 @@ cfgfile_parse(
> {
> int error;
>
> - if (!cli->cfgfile)
> - return;
> + bool default_cfgfile = !cli->cfgfile;
> +
> + if (!cli->cfgfile) {
> + /*
> + * No config file specified on the command line. Use the default
> + * system-wide config file if it exists, otherwise do nothing.
> + */
> + if (access(MKFS_DEFAULT_CFGFILE, F_OK) != 0)
> + return;
> + cli->cfgfile = MKFS_DEFAULT_CFGFILE;
> + }
>
> + parsing_cfgfile = true;
> error = ini_parse(cli->cfgfile, cfgfile_parse_ini, cli);
> + parsing_cfgfile = false;
> if (error) {
> if (error > 0) {
> fprintf(stderr,
> @@ -5773,8 +5830,9 @@ cfgfile_parse(
> }
> exit(1);
> }
> - printf(_("Parameters parsed from config file %s successfully\n"),
> - cli->cfgfile);
> + if (!default_cfgfile || cli->cfgfile_had_options)
> + printf(_("Parameters parsed from config file %s successfully\n"),
> + cli->cfgfile);
> }
>
> static void
> --
> 2.54.0
>
>
next prev parent reply other threads:[~2026-05-14 16:27 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-14 14:37 [RFC PATCH 0/1] Add default config file for mkfs.xfs Lukas Herbolt
2026-05-14 14:37 ` [RFC PATCH 1/1] xfsprogs: mkfs.xfs add default configuration file Lukas Herbolt
2026-05-14 15:21 ` Carlos Maiolino
2026-05-14 16:05 ` Eric Sandeen
2026-05-14 16:29 ` Darrick J. Wong
2026-05-14 22:27 ` Dave Chinner
2026-05-14 16:27 ` Darrick J. Wong [this message]
2026-05-14 23:35 ` Dave Chinner
2026-05-14 15:05 ` [RFC PATCH 0/1] Add default config file for mkfs.xfs 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=20260514162718.GX9555@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=aalbersh@kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=lukas@herbolt.com \
--cc=sandeen@sandeen.net \
/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.