* [PATCH 1/4] mkfs: move clearing LIBXFS_DIRECT into check_device_type
2025-10-29 9:07 improve the discard / reset zones code in mkfs Christoph Hellwig
@ 2025-10-29 9:07 ` Christoph Hellwig
2025-10-29 15:38 ` Darrick J. Wong
2025-10-29 9:07 ` [PATCH 2/4] libxfs: cleanup get_topology Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2025-10-29 9:07 UTC (permalink / raw)
To: Andrey Albershteyn; +Cc: Darrick J . Wong, linux-xfs
Keep it close to the block device vs regular file logic and remove
the checks for each device in the caller.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
mkfs/xfs_mkfs.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index cb7c20e3aa18..3ccd37920321 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1330,6 +1330,7 @@ nr_cpus(void)
static void
check_device_type(
+ struct cli_params *cli,
struct libxfs_dev *dev,
bool no_size,
bool dry_run,
@@ -1375,6 +1376,13 @@ check_device_type(
dev->isfile = 1;
else if (!dry_run)
dev->create = 1;
+
+ /*
+ * Explicitly disable direct IO for image files so we don't
+ * error out on sector size mismatches between the new
+ * filesystem and the underlying host filesystem.
+ */
+ cli->xi->flags &= ~LIBXFS_DIRECT;
return;
}
@@ -2378,21 +2386,14 @@ validate_sectorsize(
* Before anything else, verify that we are correctly operating on
* files or block devices and set the control parameters correctly.
*/
- check_device_type(&cli->xi->data, !cli->dsize, dry_run, "data", "d");
+ check_device_type(cli, &cli->xi->data, !cli->dsize, dry_run,
+ "data", "d");
if (!cli->loginternal)
- check_device_type(&cli->xi->log, !cli->logsize, dry_run, "log",
- "l");
+ check_device_type(cli, &cli->xi->log, !cli->logsize, dry_run,
+ "log", "l");
if (cli->xi->rt.name)
- check_device_type(&cli->xi->rt, !cli->rtsize, dry_run, "RT",
- "r");
-
- /*
- * Explicitly disable direct IO for image files so we don't error out on
- * sector size mismatches between the new filesystem and the underlying
- * host filesystem.
- */
- if (cli->xi->data.isfile || cli->xi->log.isfile || cli->xi->rt.isfile)
- cli->xi->flags &= ~LIBXFS_DIRECT;
+ check_device_type(cli, &cli->xi->rt, !cli->rtsize, dry_run,
+ "RT", "r");
memset(ft, 0, sizeof(*ft));
get_topology(cli->xi, ft, force_overwrite);
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/4] mkfs: move clearing LIBXFS_DIRECT into check_device_type
2025-10-29 9:07 ` [PATCH 1/4] mkfs: move clearing LIBXFS_DIRECT into check_device_type Christoph Hellwig
@ 2025-10-29 15:38 ` Darrick J. Wong
0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2025-10-29 15:38 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Andrey Albershteyn, linux-xfs
On Wed, Oct 29, 2025 at 10:07:29AM +0100, Christoph Hellwig wrote:
> Keep it close to the block device vs regular file logic and remove
> the checks for each device in the caller.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
I guess it makes sense to move the clearing of LIBXFS_DIRECT to where we
determine that one of the device paths pointed to a regular file...
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> mkfs/xfs_mkfs.c | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index cb7c20e3aa18..3ccd37920321 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1330,6 +1330,7 @@ nr_cpus(void)
>
> static void
> check_device_type(
> + struct cli_params *cli,
> struct libxfs_dev *dev,
> bool no_size,
> bool dry_run,
> @@ -1375,6 +1376,13 @@ check_device_type(
> dev->isfile = 1;
> else if (!dry_run)
> dev->create = 1;
> +
> + /*
> + * Explicitly disable direct IO for image files so we don't
> + * error out on sector size mismatches between the new
> + * filesystem and the underlying host filesystem.
> + */
> + cli->xi->flags &= ~LIBXFS_DIRECT;
> return;
> }
>
> @@ -2378,21 +2386,14 @@ validate_sectorsize(
> * Before anything else, verify that we are correctly operating on
> * files or block devices and set the control parameters correctly.
> */
> - check_device_type(&cli->xi->data, !cli->dsize, dry_run, "data", "d");
> + check_device_type(cli, &cli->xi->data, !cli->dsize, dry_run,
> + "data", "d");
> if (!cli->loginternal)
> - check_device_type(&cli->xi->log, !cli->logsize, dry_run, "log",
> - "l");
> + check_device_type(cli, &cli->xi->log, !cli->logsize, dry_run,
> + "log", "l");
> if (cli->xi->rt.name)
> - check_device_type(&cli->xi->rt, !cli->rtsize, dry_run, "RT",
> - "r");
> -
> - /*
> - * Explicitly disable direct IO for image files so we don't error out on
> - * sector size mismatches between the new filesystem and the underlying
> - * host filesystem.
> - */
> - if (cli->xi->data.isfile || cli->xi->log.isfile || cli->xi->rt.isfile)
> - cli->xi->flags &= ~LIBXFS_DIRECT;
> + check_device_type(cli, &cli->xi->rt, !cli->rtsize, dry_run,
> + "RT", "r");
>
> memset(ft, 0, sizeof(*ft));
> get_topology(cli->xi, ft, force_overwrite);
> --
> 2.47.3
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/4] libxfs: cleanup get_topology
2025-10-29 9:07 improve the discard / reset zones code in mkfs Christoph Hellwig
2025-10-29 9:07 ` [PATCH 1/4] mkfs: move clearing LIBXFS_DIRECT into check_device_type Christoph Hellwig
@ 2025-10-29 9:07 ` Christoph Hellwig
2025-10-29 15:39 ` Darrick J. Wong
2025-10-29 9:07 ` [PATCH 3/4] mkfs: remove duplicate struct libxfs_init arguments Christoph Hellwig
2025-10-29 9:07 ` [PATCH 4/4] mkfs: split zone reset from discard Christoph Hellwig
3 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2025-10-29 9:07 UTC (permalink / raw)
To: Andrey Albershteyn; +Cc: Darrick J . Wong, linux-xfs
Add a libxfs_ prefix to the name, clear the structure in the helper
instead of in the callers, and use a bool to pass a boolean argument.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
libxfs/topology.c | 9 +++++----
libxfs/topology.h | 7 ++-----
mkfs/xfs_mkfs.c | 3 +--
repair/sb.c | 3 +--
4 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/libxfs/topology.c b/libxfs/topology.c
index 7764687beac0..366165719c84 100644
--- a/libxfs/topology.c
+++ b/libxfs/topology.c
@@ -224,7 +224,7 @@ static void
blkid_get_topology(
const char *device,
struct device_topology *dt,
- int force_overwrite)
+ bool force_overwrite)
{
blkid_topology tp;
blkid_probe pr;
@@ -317,7 +317,7 @@ static void
get_device_topology(
struct libxfs_dev *dev,
struct device_topology *dt,
- int force_overwrite)
+ bool force_overwrite)
{
struct stat st;
@@ -364,11 +364,12 @@ get_device_topology(
}
void
-get_topology(
+libxfs_get_topology(
struct libxfs_init *xi,
struct fs_topology *ft,
- int force_overwrite)
+ bool force_overwrite)
{
+ memset(ft, 0, sizeof(*ft));
get_device_topology(&xi->data, &ft->data, force_overwrite);
get_device_topology(&xi->rt, &ft->rt, force_overwrite);
get_device_topology(&xi->log, &ft->log, force_overwrite);
diff --git a/libxfs/topology.h b/libxfs/topology.h
index f0ca65f3576e..3688d56b542f 100644
--- a/libxfs/topology.h
+++ b/libxfs/topology.h
@@ -25,11 +25,8 @@ struct fs_topology {
struct device_topology log;
};
-void
-get_topology(
- struct libxfs_init *xi,
- struct fs_topology *ft,
- int force_overwrite);
+void libxfs_get_topology(struct libxfs_init *xi, struct fs_topology *ft,
+ bool force_overwrite);
extern void
calc_default_ag_geometry(
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 3ccd37920321..0ba7798eccf6 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -2395,8 +2395,7 @@ validate_sectorsize(
check_device_type(cli, &cli->xi->rt, !cli->rtsize, dry_run,
"RT", "r");
- memset(ft, 0, sizeof(*ft));
- get_topology(cli->xi, ft, force_overwrite);
+ libxfs_get_topology(cli->xi, ft, force_overwrite);
/* set configured sector sizes in preparation for checks */
if (!cli->sectorsize) {
diff --git a/repair/sb.c b/repair/sb.c
index 0e4827e04678..ee1cc63fae64 100644
--- a/repair/sb.c
+++ b/repair/sb.c
@@ -184,8 +184,7 @@ guess_default_geometry(
uint64_t dblocks;
int multidisk;
- memset(&ft, 0, sizeof(ft));
- get_topology(x, &ft, 1);
+ libxfs_get_topology(x, &ft, true);
/*
* get geometry from get_topology result.
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/4] libxfs: cleanup get_topology
2025-10-29 9:07 ` [PATCH 2/4] libxfs: cleanup get_topology Christoph Hellwig
@ 2025-10-29 15:39 ` Darrick J. Wong
0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2025-10-29 15:39 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Andrey Albershteyn, linux-xfs
On Wed, Oct 29, 2025 at 10:07:30AM +0100, Christoph Hellwig wrote:
> Add a libxfs_ prefix to the name, clear the structure in the helper
> instead of in the callers, and use a bool to pass a boolean argument.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> libxfs/topology.c | 9 +++++----
> libxfs/topology.h | 7 ++-----
> mkfs/xfs_mkfs.c | 3 +--
> repair/sb.c | 3 +--
> 4 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/libxfs/topology.c b/libxfs/topology.c
> index 7764687beac0..366165719c84 100644
> --- a/libxfs/topology.c
> +++ b/libxfs/topology.c
> @@ -224,7 +224,7 @@ static void
> blkid_get_topology(
> const char *device,
> struct device_topology *dt,
> - int force_overwrite)
> + bool force_overwrite)
> {
> blkid_topology tp;
> blkid_probe pr;
> @@ -317,7 +317,7 @@ static void
> get_device_topology(
> struct libxfs_dev *dev,
> struct device_topology *dt,
> - int force_overwrite)
> + bool force_overwrite)
> {
> struct stat st;
>
> @@ -364,11 +364,12 @@ get_device_topology(
> }
>
> void
> -get_topology(
> +libxfs_get_topology(
> struct libxfs_init *xi,
> struct fs_topology *ft,
> - int force_overwrite)
> + bool force_overwrite)
> {
> + memset(ft, 0, sizeof(*ft));
> get_device_topology(&xi->data, &ft->data, force_overwrite);
> get_device_topology(&xi->rt, &ft->rt, force_overwrite);
> get_device_topology(&xi->log, &ft->log, force_overwrite);
> diff --git a/libxfs/topology.h b/libxfs/topology.h
> index f0ca65f3576e..3688d56b542f 100644
> --- a/libxfs/topology.h
> +++ b/libxfs/topology.h
> @@ -25,11 +25,8 @@ struct fs_topology {
> struct device_topology log;
> };
>
> -void
> -get_topology(
> - struct libxfs_init *xi,
> - struct fs_topology *ft,
> - int force_overwrite);
> +void libxfs_get_topology(struct libxfs_init *xi, struct fs_topology *ft,
> + bool force_overwrite);
>
> extern void
> calc_default_ag_geometry(
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 3ccd37920321..0ba7798eccf6 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -2395,8 +2395,7 @@ validate_sectorsize(
> check_device_type(cli, &cli->xi->rt, !cli->rtsize, dry_run,
> "RT", "r");
>
> - memset(ft, 0, sizeof(*ft));
> - get_topology(cli->xi, ft, force_overwrite);
> + libxfs_get_topology(cli->xi, ft, force_overwrite);
>
> /* set configured sector sizes in preparation for checks */
> if (!cli->sectorsize) {
> diff --git a/repair/sb.c b/repair/sb.c
> index 0e4827e04678..ee1cc63fae64 100644
> --- a/repair/sb.c
> +++ b/repair/sb.c
> @@ -184,8 +184,7 @@ guess_default_geometry(
> uint64_t dblocks;
> int multidisk;
>
> - memset(&ft, 0, sizeof(ft));
> - get_topology(x, &ft, 1);
> + libxfs_get_topology(x, &ft, true);
>
> /*
> * get geometry from get_topology result.
> --
> 2.47.3
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] mkfs: remove duplicate struct libxfs_init arguments
2025-10-29 9:07 improve the discard / reset zones code in mkfs Christoph Hellwig
2025-10-29 9:07 ` [PATCH 1/4] mkfs: move clearing LIBXFS_DIRECT into check_device_type Christoph Hellwig
2025-10-29 9:07 ` [PATCH 2/4] libxfs: cleanup get_topology Christoph Hellwig
@ 2025-10-29 9:07 ` Christoph Hellwig
2025-10-29 15:39 ` Darrick J. Wong
2025-10-29 9:07 ` [PATCH 4/4] mkfs: split zone reset from discard Christoph Hellwig
3 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2025-10-29 9:07 UTC (permalink / raw)
To: Andrey Albershteyn; +Cc: Darrick J . Wong, linux-xfs
The libxfs_init structure instance is pointed to by cli_params, so use
that were it already exists instead of passing an additional argument.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
mkfs/xfs_mkfs.c | 49 ++++++++++++++++++++++---------------------------
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 0ba7798eccf6..09a69af31be5 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -4005,8 +4005,7 @@ ddev_is_solidstate(
static void
calc_concurrency_ag_geometry(
struct mkfs_params *cfg,
- struct cli_params *cli,
- struct libxfs_init *xi)
+ struct cli_params *cli)
{
uint64_t try_agsize;
uint64_t def_agsize;
@@ -4074,11 +4073,10 @@ out:
static void
calculate_initial_ag_geometry(
struct mkfs_params *cfg,
- struct cli_params *cli,
- struct libxfs_init *xi)
+ struct cli_params *cli)
{
if (cli->data_concurrency > 0) {
- calc_concurrency_ag_geometry(cfg, cli, xi);
+ calc_concurrency_ag_geometry(cfg, cli);
} else if (cli->agsize) { /* User-specified AG size */
cfg->agsize = getnum(cli->agsize, &dopts, D_AGSIZE);
@@ -4099,8 +4097,9 @@ _("agsize (%s) not a multiple of fs blk size (%d)\n"),
cfg->agcount = cli->agcount;
cfg->agsize = cfg->dblocks / cfg->agcount +
(cfg->dblocks % cfg->agcount != 0);
- } else if (cli->data_concurrency == -1 && ddev_is_solidstate(xi)) {
- calc_concurrency_ag_geometry(cfg, cli, xi);
+ } else if (cli->data_concurrency == -1 &&
+ ddev_is_solidstate(cli->xi)) {
+ calc_concurrency_ag_geometry(cfg, cli);
} else {
calc_default_ag_geometry(cfg->blocklog, cfg->dblocks,
cfg->dsunit, &cfg->agsize,
@@ -4360,8 +4359,7 @@ rtdev_is_solidstate(
static void
calc_concurrency_rtgroup_geometry(
struct mkfs_params *cfg,
- struct cli_params *cli,
- struct libxfs_init *xi)
+ struct cli_params *cli)
{
uint64_t try_rgsize;
uint64_t def_rgsize;
@@ -4468,8 +4466,7 @@ _("realtime group count (%llu) must be less than the maximum (%u)\n"),
static void
calculate_rtgroup_geometry(
struct mkfs_params *cfg,
- struct cli_params *cli,
- struct libxfs_init *xi)
+ struct cli_params *cli)
{
if (!cli->sb_feat.metadir) {
cfg->rgcount = 0;
@@ -4510,8 +4507,9 @@ _("rgsize (%s) not a multiple of fs blk size (%d)\n"),
cfg->rgsize = cfg->rtblocks;
cfg->rgcount = 0;
} else if (cli->rtvol_concurrency > 0 ||
- (cli->rtvol_concurrency == -1 && rtdev_is_solidstate(xi))) {
- calc_concurrency_rtgroup_geometry(cfg, cli, xi);
+ (cli->rtvol_concurrency == -1 &&
+ rtdev_is_solidstate(cli->xi))) {
+ calc_concurrency_rtgroup_geometry(cfg, cli);
} else if (is_power_of_2(cfg->rtextblocks)) {
cfg->rgsize = calc_rgsize_extsize_power(cfg);
cfg->rgcount = cfg->rtblocks / cfg->rgsize +
@@ -4538,7 +4536,6 @@ static void
adjust_nr_zones(
struct mkfs_params *cfg,
struct cli_params *cli,
- struct libxfs_init *xi,
struct zone_topology *zt)
{
uint64_t new_rtblocks, slack;
@@ -4547,7 +4544,8 @@ adjust_nr_zones(
if (zt->rt.nr_zones)
max_zones = zt->rt.nr_zones;
else
- max_zones = DTOBT(xi->rt.size, cfg->blocklog) / cfg->rgsize;
+ max_zones = DTOBT(cli->xi->rt.size, cfg->blocklog) /
+ cfg->rgsize;
if (!cli->rgcount)
cfg->rgcount += XFS_RESERVED_ZONES;
@@ -4576,7 +4574,6 @@ static void
calculate_zone_geometry(
struct mkfs_params *cfg,
struct cli_params *cli,
- struct libxfs_init *xi,
struct zone_topology *zt)
{
if (cfg->rtblocks == 0) {
@@ -4645,7 +4642,7 @@ _("rgsize (%s) not a multiple of fs blk size (%d)\n"),
}
if (cli->rtsize || cli->rgcount)
- adjust_nr_zones(cfg, cli, xi, zt);
+ adjust_nr_zones(cfg, cli, zt);
if (cfg->rgcount < XFS_MIN_ZONES) {
fprintf(stderr,
@@ -4984,7 +4981,6 @@ static uint64_t
calc_concurrency_logblocks(
struct mkfs_params *cfg,
struct cli_params *cli,
- struct libxfs_init *xi,
unsigned int max_tx_bytes)
{
uint64_t log_bytes;
@@ -4992,7 +4988,7 @@ calc_concurrency_logblocks(
unsigned int new_logblocks;
if (cli->log_concurrency < 0) {
- if (!ddev_is_solidstate(xi))
+ if (!ddev_is_solidstate(cli->xi))
goto out;
cli->log_concurrency = nr_cpus();
@@ -5160,7 +5156,6 @@ static void
calculate_log_size(
struct mkfs_params *cfg,
struct cli_params *cli,
- struct libxfs_init *xi,
struct xfs_mount *mp)
{
struct xfs_sb *sbp = &mp->m_sb;
@@ -5225,8 +5220,8 @@ _("external log device size %lld blocks too small, must be at least %lld blocks\
if (cfg->lsunit) {
uint64_t max_logblocks;
- max_logblocks = min(DTOBT(xi->log.size, cfg->blocklog),
- XFS_MAX_LOG_BLOCKS);
+ max_logblocks = min(XFS_MAX_LOG_BLOCKS,
+ DTOBT(cli->xi->log.size, cfg->blocklog));
align_log_size(cfg, cfg->lsunit, max_logblocks);
}
@@ -5261,7 +5256,7 @@ _("max log size %d smaller than min log size %d, filesystem is too small\n"),
if (cli->log_concurrency != 0)
cfg->logblocks = calc_concurrency_logblocks(cfg, cli,
- xi, max_tx_bytes);
+ max_tx_bytes);
/* But don't go below a reasonable size */
cfg->logblocks = max(cfg->logblocks,
@@ -6135,12 +6130,12 @@ main(
* dependent on device sizes. Once calculated, make sure everything
* aligns to device geometry correctly.
*/
- calculate_initial_ag_geometry(&cfg, &cli, &xi);
+ calculate_initial_ag_geometry(&cfg, &cli);
align_ag_geometry(&cfg, &ft);
if (cfg.sb_feat.zoned)
- calculate_zone_geometry(&cfg, &cli, &xi, &zt);
+ calculate_zone_geometry(&cfg, &cli, &zt);
else
- calculate_rtgroup_geometry(&cfg, &cli, &xi);
+ calculate_rtgroup_geometry(&cfg, &cli);
calculate_imaxpct(&cfg, &cli);
@@ -6164,7 +6159,7 @@ main(
* With the mount set up, we can finally calculate the log size
* constraints and do default size calculations and final validation
*/
- calculate_log_size(&cfg, &cli, &xi, mp);
+ calculate_log_size(&cfg, &cli, mp);
finish_superblock_setup(&cfg, mp, sbp);
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] mkfs: remove duplicate struct libxfs_init arguments
2025-10-29 9:07 ` [PATCH 3/4] mkfs: remove duplicate struct libxfs_init arguments Christoph Hellwig
@ 2025-10-29 15:39 ` Darrick J. Wong
0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2025-10-29 15:39 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Andrey Albershteyn, linux-xfs
On Wed, Oct 29, 2025 at 10:07:31AM +0100, Christoph Hellwig wrote:
> The libxfs_init structure instance is pointed to by cli_params, so use
> that were it already exists instead of passing an additional argument.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Yay fewer args,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> mkfs/xfs_mkfs.c | 49 ++++++++++++++++++++++---------------------------
> 1 file changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 0ba7798eccf6..09a69af31be5 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -4005,8 +4005,7 @@ ddev_is_solidstate(
> static void
> calc_concurrency_ag_geometry(
> struct mkfs_params *cfg,
> - struct cli_params *cli,
> - struct libxfs_init *xi)
> + struct cli_params *cli)
> {
> uint64_t try_agsize;
> uint64_t def_agsize;
> @@ -4074,11 +4073,10 @@ out:
> static void
> calculate_initial_ag_geometry(
> struct mkfs_params *cfg,
> - struct cli_params *cli,
> - struct libxfs_init *xi)
> + struct cli_params *cli)
> {
> if (cli->data_concurrency > 0) {
> - calc_concurrency_ag_geometry(cfg, cli, xi);
> + calc_concurrency_ag_geometry(cfg, cli);
> } else if (cli->agsize) { /* User-specified AG size */
> cfg->agsize = getnum(cli->agsize, &dopts, D_AGSIZE);
>
> @@ -4099,8 +4097,9 @@ _("agsize (%s) not a multiple of fs blk size (%d)\n"),
> cfg->agcount = cli->agcount;
> cfg->agsize = cfg->dblocks / cfg->agcount +
> (cfg->dblocks % cfg->agcount != 0);
> - } else if (cli->data_concurrency == -1 && ddev_is_solidstate(xi)) {
> - calc_concurrency_ag_geometry(cfg, cli, xi);
> + } else if (cli->data_concurrency == -1 &&
> + ddev_is_solidstate(cli->xi)) {
> + calc_concurrency_ag_geometry(cfg, cli);
> } else {
> calc_default_ag_geometry(cfg->blocklog, cfg->dblocks,
> cfg->dsunit, &cfg->agsize,
> @@ -4360,8 +4359,7 @@ rtdev_is_solidstate(
> static void
> calc_concurrency_rtgroup_geometry(
> struct mkfs_params *cfg,
> - struct cli_params *cli,
> - struct libxfs_init *xi)
> + struct cli_params *cli)
> {
> uint64_t try_rgsize;
> uint64_t def_rgsize;
> @@ -4468,8 +4466,7 @@ _("realtime group count (%llu) must be less than the maximum (%u)\n"),
> static void
> calculate_rtgroup_geometry(
> struct mkfs_params *cfg,
> - struct cli_params *cli,
> - struct libxfs_init *xi)
> + struct cli_params *cli)
> {
> if (!cli->sb_feat.metadir) {
> cfg->rgcount = 0;
> @@ -4510,8 +4507,9 @@ _("rgsize (%s) not a multiple of fs blk size (%d)\n"),
> cfg->rgsize = cfg->rtblocks;
> cfg->rgcount = 0;
> } else if (cli->rtvol_concurrency > 0 ||
> - (cli->rtvol_concurrency == -1 && rtdev_is_solidstate(xi))) {
> - calc_concurrency_rtgroup_geometry(cfg, cli, xi);
> + (cli->rtvol_concurrency == -1 &&
> + rtdev_is_solidstate(cli->xi))) {
> + calc_concurrency_rtgroup_geometry(cfg, cli);
> } else if (is_power_of_2(cfg->rtextblocks)) {
> cfg->rgsize = calc_rgsize_extsize_power(cfg);
> cfg->rgcount = cfg->rtblocks / cfg->rgsize +
> @@ -4538,7 +4536,6 @@ static void
> adjust_nr_zones(
> struct mkfs_params *cfg,
> struct cli_params *cli,
> - struct libxfs_init *xi,
> struct zone_topology *zt)
> {
> uint64_t new_rtblocks, slack;
> @@ -4547,7 +4544,8 @@ adjust_nr_zones(
> if (zt->rt.nr_zones)
> max_zones = zt->rt.nr_zones;
> else
> - max_zones = DTOBT(xi->rt.size, cfg->blocklog) / cfg->rgsize;
> + max_zones = DTOBT(cli->xi->rt.size, cfg->blocklog) /
> + cfg->rgsize;
>
> if (!cli->rgcount)
> cfg->rgcount += XFS_RESERVED_ZONES;
> @@ -4576,7 +4574,6 @@ static void
> calculate_zone_geometry(
> struct mkfs_params *cfg,
> struct cli_params *cli,
> - struct libxfs_init *xi,
> struct zone_topology *zt)
> {
> if (cfg->rtblocks == 0) {
> @@ -4645,7 +4642,7 @@ _("rgsize (%s) not a multiple of fs blk size (%d)\n"),
> }
>
> if (cli->rtsize || cli->rgcount)
> - adjust_nr_zones(cfg, cli, xi, zt);
> + adjust_nr_zones(cfg, cli, zt);
>
> if (cfg->rgcount < XFS_MIN_ZONES) {
> fprintf(stderr,
> @@ -4984,7 +4981,6 @@ static uint64_t
> calc_concurrency_logblocks(
> struct mkfs_params *cfg,
> struct cli_params *cli,
> - struct libxfs_init *xi,
> unsigned int max_tx_bytes)
> {
> uint64_t log_bytes;
> @@ -4992,7 +4988,7 @@ calc_concurrency_logblocks(
> unsigned int new_logblocks;
>
> if (cli->log_concurrency < 0) {
> - if (!ddev_is_solidstate(xi))
> + if (!ddev_is_solidstate(cli->xi))
> goto out;
>
> cli->log_concurrency = nr_cpus();
> @@ -5160,7 +5156,6 @@ static void
> calculate_log_size(
> struct mkfs_params *cfg,
> struct cli_params *cli,
> - struct libxfs_init *xi,
> struct xfs_mount *mp)
> {
> struct xfs_sb *sbp = &mp->m_sb;
> @@ -5225,8 +5220,8 @@ _("external log device size %lld blocks too small, must be at least %lld blocks\
> if (cfg->lsunit) {
> uint64_t max_logblocks;
>
> - max_logblocks = min(DTOBT(xi->log.size, cfg->blocklog),
> - XFS_MAX_LOG_BLOCKS);
> + max_logblocks = min(XFS_MAX_LOG_BLOCKS,
> + DTOBT(cli->xi->log.size, cfg->blocklog));
> align_log_size(cfg, cfg->lsunit, max_logblocks);
> }
>
> @@ -5261,7 +5256,7 @@ _("max log size %d smaller than min log size %d, filesystem is too small\n"),
>
> if (cli->log_concurrency != 0)
> cfg->logblocks = calc_concurrency_logblocks(cfg, cli,
> - xi, max_tx_bytes);
> + max_tx_bytes);
>
> /* But don't go below a reasonable size */
> cfg->logblocks = max(cfg->logblocks,
> @@ -6135,12 +6130,12 @@ main(
> * dependent on device sizes. Once calculated, make sure everything
> * aligns to device geometry correctly.
> */
> - calculate_initial_ag_geometry(&cfg, &cli, &xi);
> + calculate_initial_ag_geometry(&cfg, &cli);
> align_ag_geometry(&cfg, &ft);
> if (cfg.sb_feat.zoned)
> - calculate_zone_geometry(&cfg, &cli, &xi, &zt);
> + calculate_zone_geometry(&cfg, &cli, &zt);
> else
> - calculate_rtgroup_geometry(&cfg, &cli, &xi);
> + calculate_rtgroup_geometry(&cfg, &cli);
>
> calculate_imaxpct(&cfg, &cli);
>
> @@ -6164,7 +6159,7 @@ main(
> * With the mount set up, we can finally calculate the log size
> * constraints and do default size calculations and final validation
> */
> - calculate_log_size(&cfg, &cli, &xi, mp);
> + calculate_log_size(&cfg, &cli, mp);
>
> finish_superblock_setup(&cfg, mp, sbp);
>
> --
> 2.47.3
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] mkfs: split zone reset from discard
2025-10-29 9:07 improve the discard / reset zones code in mkfs Christoph Hellwig
` (2 preceding siblings ...)
2025-10-29 9:07 ` [PATCH 3/4] mkfs: remove duplicate struct libxfs_init arguments Christoph Hellwig
@ 2025-10-29 9:07 ` Christoph Hellwig
2025-10-29 15:40 ` Darrick J. Wong
3 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2025-10-29 9:07 UTC (permalink / raw)
To: Andrey Albershteyn; +Cc: Darrick J . Wong, linux-xfs
Zone reset is a mandatory part of creating a file system on a zoned
device, unlike discard, which can be skipped. It also is implemented
a bit different, so just split the handling. This also means that we
can now support the -K option to skip discard on the data section for
zoned file systems.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
mkfs/xfs_mkfs.c | 112 +++++++++++++++++++++++-------------------------
1 file changed, 53 insertions(+), 59 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 09a69af31be5..cd4f3ba4a549 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1607,34 +1607,6 @@ discard_blocks(int fd, uint64_t nsectors, int quiet)
printf("Done.\n");
}
-static void
-reset_zones(
- struct mkfs_params *cfg,
- int fd,
- uint64_t start_sector,
- uint64_t nsectors,
- int quiet)
-{
- struct blk_zone_range range = {
- .sector = start_sector,
- .nr_sectors = nsectors,
- };
-
- if (!quiet) {
- printf("Resetting zones...");
- fflush(stdout);
- }
-
- if (ioctl(fd, BLKRESETZONE, &range) < 0) {
- if (!quiet)
- printf(" FAILED (%d)\n", -errno);
- exit(1);
- }
-
- if (!quiet)
- printf("Done.\n");
-}
-
static __attribute__((noreturn)) void
illegal_option(
const char *value,
@@ -3780,41 +3752,66 @@ discard_devices(
struct zone_topology *zt,
int quiet)
{
- /*
- * This function has to be called after libxfs has been initialized.
- */
-
if (!xi->data.isfile) {
uint64_t nsectors = xi->data.size;
- if (cfg->rtstart && zt->data.nr_zones) {
- /*
- * Note that the zone reset here includes the LBA range
- * for the data device.
- *
- * This is because doing a single zone reset all on the
- * entire device (which the kernel automatically does
- * for us for a full device range) is a lot faster than
- * resetting each zone individually and resetting
- * the conventional zones used for the data device is a
- * no-op.
- */
- reset_zones(cfg, xi->data.fd, 0,
- cfg->rtstart + xi->rt.size, quiet);
+ if (cfg->rtstart && zt->data.nr_zones)
nsectors -= cfg->rtstart;
- }
discard_blocks(xi->data.fd, nsectors, quiet);
}
- if (xi->rt.dev && !xi->rt.isfile) {
- if (zt->rt.nr_zones)
- reset_zones(cfg, xi->rt.fd, 0, xi->rt.size, quiet);
- else
- discard_blocks(xi->rt.fd, xi->rt.size, quiet);
- }
+ if (xi->rt.dev && !xi->rt.isfile && !zt->rt.nr_zones)
+ discard_blocks(xi->rt.fd, xi->rt.size, quiet);
if (xi->log.dev && xi->log.dev != xi->data.dev && !xi->log.isfile)
discard_blocks(xi->log.fd, xi->log.size, quiet);
}
+static void
+reset_zones(
+ struct mkfs_params *cfg,
+ struct libxfs_dev *dev,
+ uint64_t size,
+ bool quiet)
+{
+ struct blk_zone_range range = {
+ .nr_sectors = size,
+ };
+
+ if (!quiet) {
+ printf("Resetting zones...");
+ fflush(stdout);
+ }
+ if (ioctl(dev->fd, BLKRESETZONE, &range) < 0) {
+ if (!quiet)
+ printf(" FAILED (%d)\n", -errno);
+ exit(1);
+ }
+ if (!quiet)
+ printf("Done.\n");
+}
+
+static void
+reset_devices(
+ struct mkfs_params *cfg,
+ struct libxfs_init *xi,
+ struct zone_topology *zt,
+ int quiet)
+{
+ /*
+ * Note that the zone reset here includes the conventional zones used
+ * for the data device.
+ *
+ * It is done that way because doing a single zone reset all on the
+ * entire device (which the kernel automatically does for us for a full
+ * device range) is a lot faster than resetting each zone individually
+ * and resetting the conventional zones used for the data device is a
+ * no-op.
+ */
+ if (!xi->data.isfile && zt->data.nr_zones && cfg->rtstart)
+ reset_zones(cfg, &xi->data, cfg->rtstart + xi->rt.size, quiet);
+ if (xi->rt.dev && !xi->rt.isfile && zt->rt.nr_zones)
+ reset_zones(cfg, &xi->rt, xi->rt.size, quiet);
+}
+
static void
validate_datadev(
struct mkfs_params *cfg,
@@ -6196,13 +6193,10 @@ main(
/*
* All values have been validated, discard the old device layout.
*/
- if (cli.sb_feat.zoned && !discard) {
- fprintf(stderr,
- _("-K not support for zoned file systems.\n"));
- return 1;
- }
if (discard && !dry_run)
- discard_devices(&cfg, &xi, &zt, quiet);
+ discard_devices(&cfg, cli.xi, &zt, quiet);
+ if (cli.sb_feat.zoned && !dry_run)
+ reset_devices(&cfg, cli.xi, &zt, quiet);
/*
* we need the libxfs buffer cache from here on in.
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] mkfs: split zone reset from discard
2025-10-29 9:07 ` [PATCH 4/4] mkfs: split zone reset from discard Christoph Hellwig
@ 2025-10-29 15:40 ` Darrick J. Wong
0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2025-10-29 15:40 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Andrey Albershteyn, linux-xfs
On Wed, Oct 29, 2025 at 10:07:32AM +0100, Christoph Hellwig wrote:
> Zone reset is a mandatory part of creating a file system on a zoned
> device, unlike discard, which can be skipped. It also is implemented
> a bit different, so just split the handling. This also means that we
> can now support the -K option to skip discard on the data section for
> zoned file systems.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Nice feature improvement! ;)
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> mkfs/xfs_mkfs.c | 112 +++++++++++++++++++++++-------------------------
> 1 file changed, 53 insertions(+), 59 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 09a69af31be5..cd4f3ba4a549 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1607,34 +1607,6 @@ discard_blocks(int fd, uint64_t nsectors, int quiet)
> printf("Done.\n");
> }
>
> -static void
> -reset_zones(
> - struct mkfs_params *cfg,
> - int fd,
> - uint64_t start_sector,
> - uint64_t nsectors,
> - int quiet)
> -{
> - struct blk_zone_range range = {
> - .sector = start_sector,
> - .nr_sectors = nsectors,
> - };
> -
> - if (!quiet) {
> - printf("Resetting zones...");
> - fflush(stdout);
> - }
> -
> - if (ioctl(fd, BLKRESETZONE, &range) < 0) {
> - if (!quiet)
> - printf(" FAILED (%d)\n", -errno);
> - exit(1);
> - }
> -
> - if (!quiet)
> - printf("Done.\n");
> -}
> -
> static __attribute__((noreturn)) void
> illegal_option(
> const char *value,
> @@ -3780,41 +3752,66 @@ discard_devices(
> struct zone_topology *zt,
> int quiet)
> {
> - /*
> - * This function has to be called after libxfs has been initialized.
> - */
> -
> if (!xi->data.isfile) {
> uint64_t nsectors = xi->data.size;
>
> - if (cfg->rtstart && zt->data.nr_zones) {
> - /*
> - * Note that the zone reset here includes the LBA range
> - * for the data device.
> - *
> - * This is because doing a single zone reset all on the
> - * entire device (which the kernel automatically does
> - * for us for a full device range) is a lot faster than
> - * resetting each zone individually and resetting
> - * the conventional zones used for the data device is a
> - * no-op.
> - */
> - reset_zones(cfg, xi->data.fd, 0,
> - cfg->rtstart + xi->rt.size, quiet);
> + if (cfg->rtstart && zt->data.nr_zones)
> nsectors -= cfg->rtstart;
> - }
> discard_blocks(xi->data.fd, nsectors, quiet);
> }
> - if (xi->rt.dev && !xi->rt.isfile) {
> - if (zt->rt.nr_zones)
> - reset_zones(cfg, xi->rt.fd, 0, xi->rt.size, quiet);
> - else
> - discard_blocks(xi->rt.fd, xi->rt.size, quiet);
> - }
> + if (xi->rt.dev && !xi->rt.isfile && !zt->rt.nr_zones)
> + discard_blocks(xi->rt.fd, xi->rt.size, quiet);
> if (xi->log.dev && xi->log.dev != xi->data.dev && !xi->log.isfile)
> discard_blocks(xi->log.fd, xi->log.size, quiet);
> }
>
> +static void
> +reset_zones(
> + struct mkfs_params *cfg,
> + struct libxfs_dev *dev,
> + uint64_t size,
> + bool quiet)
> +{
> + struct blk_zone_range range = {
> + .nr_sectors = size,
> + };
> +
> + if (!quiet) {
> + printf("Resetting zones...");
> + fflush(stdout);
> + }
> + if (ioctl(dev->fd, BLKRESETZONE, &range) < 0) {
> + if (!quiet)
> + printf(" FAILED (%d)\n", -errno);
> + exit(1);
> + }
> + if (!quiet)
> + printf("Done.\n");
> +}
> +
> +static void
> +reset_devices(
> + struct mkfs_params *cfg,
> + struct libxfs_init *xi,
> + struct zone_topology *zt,
> + int quiet)
> +{
> + /*
> + * Note that the zone reset here includes the conventional zones used
> + * for the data device.
> + *
> + * It is done that way because doing a single zone reset all on the
> + * entire device (which the kernel automatically does for us for a full
> + * device range) is a lot faster than resetting each zone individually
> + * and resetting the conventional zones used for the data device is a
> + * no-op.
> + */
> + if (!xi->data.isfile && zt->data.nr_zones && cfg->rtstart)
> + reset_zones(cfg, &xi->data, cfg->rtstart + xi->rt.size, quiet);
> + if (xi->rt.dev && !xi->rt.isfile && zt->rt.nr_zones)
> + reset_zones(cfg, &xi->rt, xi->rt.size, quiet);
> +}
> +
> static void
> validate_datadev(
> struct mkfs_params *cfg,
> @@ -6196,13 +6193,10 @@ main(
> /*
> * All values have been validated, discard the old device layout.
> */
> - if (cli.sb_feat.zoned && !discard) {
> - fprintf(stderr,
> - _("-K not support for zoned file systems.\n"));
> - return 1;
> - }
> if (discard && !dry_run)
> - discard_devices(&cfg, &xi, &zt, quiet);
> + discard_devices(&cfg, cli.xi, &zt, quiet);
> + if (cli.sb_feat.zoned && !dry_run)
> + reset_devices(&cfg, cli.xi, &zt, quiet);
>
> /*
> * we need the libxfs buffer cache from here on in.
> --
> 2.47.3
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread