[PATCH 0/3] xfs: consolidate metadir creation code

[PATCH 0/3] xfs: consolidate metadir creation code

[Johannes Thumshirn]

The XFS metadata directory tree has grown several nearly identical
open-coded copies of the "create a metadata inode" sequence: allocate the
creation transaction, create and link the inode, do a bit of type
specific inode setup, commit, and finish or tear down the inode on both
the success and error paths.

Factor that boilerplate into a single helper, xfs_metadir_create_file(),
which takes the file mode and an optional callback to finish initializing
the new inode inside the creation transaction, and convert the existing
users - xfs_metadir_mkdir(), the quota inodes and the rtgroup inodes -
over to it.

Having a single helper also makes it straightforward to add new metadata
inode types without copying the sequence yet again.

No functional change intended otherwise.

Johannes Thumshirn (3):
  xfs: add xfs_metadir_create_file helper
  xfs: create quota metadir inodes using xfs_metadir_create_file
  xfs: create rtgroup metadir inodes using xfs_metadir_create_file

 fs/xfs/libxfs/xfs_dquot_buf.c | 29 +++++++-----------
 fs/xfs/libxfs/xfs_metadir.c   | 58 ++++++++++++++++++++++-------------
 fs/xfs/libxfs/xfs_metadir.h   |  6 ++++
 fs/xfs/libxfs/xfs_rtgroup.c   | 58 ++++++++++++++++-------------------
 4 files changed, 80 insertions(+), 71 deletions(-)

-- 
2.54.0

[PATCH 1/3] xfs: add xfs_metadir_create_file helper

[Johannes Thumshirn]

Factor the metadata inode create/commit/cleanup lifecycle out of
xfs_metadir_mkdir into a reusable helper that takes an optional callback
to initialize the new inode, and convert xfs_metadir_mkdir to it.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/xfs/libxfs/xfs_metadir.c | 58 +++++++++++++++++++++++--------------
 fs/xfs/libxfs/xfs_metadir.h |  6 ++++
 2 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_metadir.c b/fs/xfs/libxfs/xfs_metadir.c
index 74c4596ee4cf..0d6a153bc9e9 100644
--- a/fs/xfs/libxfs/xfs_metadir.c
+++ b/fs/xfs/libxfs/xfs_metadir.c
@@ -438,48 +438,64 @@ xfs_metadir_cancel(
 	xfs_metadir_teardown(upd, error);
 }
 
-/* Create a metadata for the last component of the path. */
 int
-xfs_metadir_mkdir(
-	struct xfs_inode		*dp,
-	const char			*path,
+xfs_metadir_create_file(
+	struct xfs_metadir_update	*upd,
+	umode_t				mode,
+	xfs_metadir_createfn		create,
+	void				*priv,
 	struct xfs_inode		**ipp)
 {
-	struct xfs_metadir_update	upd = {
-		.dp			= dp,
-		.path			= path,
-		.metafile_type		= XFS_METAFILE_DIR,
-	};
 	int				error;
 
-	if (xfs_is_shutdown(dp->i_mount))
+	if (xfs_is_shutdown(upd->dp->i_mount))
 		return -EIO;
 
-	/* Allocate a transaction to create the last directory. */
-	error = xfs_metadir_start_create(&upd);
+	error = xfs_metadir_start_create(upd);
 	if (error)
 		return error;
 
-	/* Create the subdirectory and take our reference. */
-	error = xfs_metadir_create(&upd, S_IFDIR);
+	error = xfs_metadir_create(upd, mode);
 	if (error)
 		goto out_cancel;
 
-	error = xfs_metadir_commit(&upd);
+	if (create) {
+		error = create(upd, priv);
+		if (error)
+			goto out_cancel;
+	}
+
+	error = xfs_metadir_commit(upd);
 	if (error)
 		goto out_irele;
 
-	xfs_finish_inode_setup(upd.ip);
-	*ipp = upd.ip;
+	xfs_finish_inode_setup(upd->ip);
+	*ipp = upd->ip;
 	return 0;
 
 out_cancel:
-	xfs_metadir_cancel(&upd, error);
+	xfs_metadir_cancel(upd, error);
 out_irele:
 	/* Have to finish setting up the inode to ensure it's deleted. */
-	if (upd.ip) {
-		xfs_finish_inode_setup(upd.ip);
-		xfs_irele(upd.ip);
+	if (upd->ip) {
+		xfs_finish_inode_setup(upd->ip);
+		xfs_irele(upd->ip);
 	}
 	return error;
 }
+
+/* Create a metadata for the last component of the path. */
+int
+xfs_metadir_mkdir(
+	struct xfs_inode		*dp,
+	const char			*path,
+	struct xfs_inode		**ipp)
+{
+	struct xfs_metadir_update	upd = {
+		.dp			= dp,
+		.path			= path,
+		.metafile_type		= XFS_METAFILE_DIR,
+	};
+
+	return xfs_metadir_create_file(&upd, S_IFDIR, NULL, NULL, ipp);
+}
diff --git a/fs/xfs/libxfs/xfs_metadir.h b/fs/xfs/libxfs/xfs_metadir.h
index bfecac7d3d14..a795a2d0e3fe 100644
--- a/fs/xfs/libxfs/xfs_metadir.h
+++ b/fs/xfs/libxfs/xfs_metadir.h
@@ -35,6 +35,12 @@ int xfs_metadir_load(struct xfs_trans *tp, struct xfs_inode *dp,
 int xfs_metadir_start_create(struct xfs_metadir_update *upd);
 int xfs_metadir_create(struct xfs_metadir_update *upd, umode_t mode);
 
+typedef int (*xfs_metadir_createfn)(struct xfs_metadir_update *upd, void *priv);
+
+int xfs_metadir_create_file(struct xfs_metadir_update *upd, umode_t mode,
+		xfs_metadir_createfn create, void *priv,
+		struct xfs_inode **ipp);
+
 int xfs_metadir_start_link(struct xfs_metadir_update *upd);
 int xfs_metadir_link(struct xfs_metadir_update *upd);
 
-- 
2.54.0

[PATCH 2/3] xfs: create quota metadir inodes using xfs_metadir_create_file

[Johannes Thumshirn]

Now that we have xfs_metadir_create_file() use it in
xfs_dqinode_metadir_create().

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/xfs/libxfs/xfs_dquot_buf.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index ce767b40482f..f960474bed3d 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -416,6 +416,15 @@ xfs_dqinode_load(
 	return 0;
 }
 
+static int
+xfs_dqinode_init(
+	struct xfs_metadir_update	*upd,
+	void				*priv)
+{
+	xfs_trans_log_inode(upd->tp, upd->ip, XFS_ILOG_CORE);
+	return 0;
+}
+
 /* Create a metadata directory quota inode. */
 int
 xfs_dqinode_metadir_create(
@@ -428,25 +437,9 @@ xfs_dqinode_metadir_create(
 		.metafile_type		= xfs_dqinode_metafile_type(type),
 		.path			= xfs_dqinode_path(type),
 	};
-	int				error;
 
-	error = xfs_metadir_start_create(&upd);
-	if (error)
-		return error;
-
-	error = xfs_metadir_create(&upd, S_IFREG);
-	if (error)
-		return error;
-
-	xfs_trans_log_inode(upd.tp, upd.ip, XFS_ILOG_CORE);
-
-	error = xfs_metadir_commit(&upd);
-	if (error)
-		return error;
-
-	xfs_finish_inode_setup(upd.ip);
-	*ipp = upd.ip;
-	return 0;
+	return xfs_metadir_create_file(&upd, S_IFREG, xfs_dqinode_init, NULL,
+			ipp);
 }
 
 #ifndef __KERNEL__
-- 
2.54.0

[PATCH 3/3] xfs: create rtgroup metadir inodes using xfs_metadir_create_file

[Johannes Thumshirn]

Now that we have xfs_metadir_create_file() use it in
xfs_rtginode_create().

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/xfs/libxfs/xfs_rtgroup.c | 58 +++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_rtgroup.c b/fs/xfs/libxfs/xfs_rtgroup.c
index caecc1e1f723..b6844b9af390 100644
--- a/fs/xfs/libxfs/xfs_rtgroup.c
+++ b/fs/xfs/libxfs/xfs_rtgroup.c
@@ -547,6 +547,25 @@ xfs_rtginode_irele(
 	*ipp = NULL;
 }
 
+struct xfs_rtginode_create {
+	struct xfs_rtgroup		*rtg;
+	enum xfs_rtg_inodes		type;
+	bool				init;
+};
+
+static int
+xfs_rtginode_init(
+	struct xfs_metadir_update	*upd,
+	void				*priv)
+{
+	struct xfs_rtginode_create	*rc = priv;
+	const struct xfs_rtginode_ops	*ops = &xfs_rtginode_ops[rc->type];
+
+	xfs_rtginode_lockdep_setup(upd->ip, rtg_rgno(rc->rtg), rc->type);
+	upd->ip->i_projid = rtg_rgno(rc->rtg);
+	return ops->create(rc->rtg, upd->ip, upd->tp, rc->init);
+}
+
 /* Add a metadata inode for a realtime rmap btree. */
 int
 xfs_rtginode_create(
@@ -556,6 +575,11 @@ xfs_rtginode_create(
 {
 	const struct xfs_rtginode_ops	*ops = &xfs_rtginode_ops[type];
 	struct xfs_mount		*mp = rtg_mount(rtg);
+	struct xfs_rtginode_create	rc = {
+		.rtg			= rtg,
+		.type			= type,
+		.init			= init,
+	};
 	struct xfs_metadir_update	upd = {
 		.dp			= mp->m_rtdirip,
 		.metafile_type		= ops->metafile_type,
@@ -574,38 +598,8 @@ xfs_rtginode_create(
 	if (!upd.path)
 		return -ENOMEM;
 
-	error = xfs_metadir_start_create(&upd);
-	if (error)
-		goto out_path;
-
-	error = xfs_metadir_create(&upd, S_IFREG);
-	if (error)
-		goto out_cancel;
-
-	xfs_rtginode_lockdep_setup(upd.ip, rtg_rgno(rtg), type);
-
-	upd.ip->i_projid = rtg_rgno(rtg);
-	error = ops->create(rtg, upd.ip, upd.tp, init);
-	if (error)
-		goto out_cancel;
-
-	error = xfs_metadir_commit(&upd);
-	if (error)
-		goto out_path;
-
-	kfree(upd.path);
-	xfs_finish_inode_setup(upd.ip);
-	rtg->rtg_inodes[type] = upd.ip;
-	return 0;
-
-out_cancel:
-	xfs_metadir_cancel(&upd, error);
-	/* Have to finish setting up the inode to ensure it's deleted. */
-	if (upd.ip) {
-		xfs_finish_inode_setup(upd.ip);
-		xfs_irele(upd.ip);
-	}
-out_path:
+	error = xfs_metadir_create_file(&upd, S_IFREG, xfs_rtginode_init, &rc,
+			&rtg->rtg_inodes[type]);
 	kfree(upd.path);
 	return error;
 }
-- 
2.54.0

Re: [PATCH 1/3] xfs: add xfs_metadir_create_file helper

[Christoph Hellwig]

On Sun, Jul 12, 2026 at 11:33:15AM +0200, Johannes Thumshirn wrote:
> Factor the metadata inode create/commit/cleanup lifecycle out of
> xfs_metadir_mkdir into a reusable helper that takes an optional callback
> to initialize the new inode, and convert xfs_metadir_mkdir to it.

This needs to be rebased on top of commit 45de375b2506 ("xfs: fix memory
leak in xfs_dqinode_metadir_create()"), which is already in mainline.
Without that revert the patch currently fails to apply.

Otherwise this looks good.

Re: [PATCH 2/3] xfs: create quota metadir inodes using xfs_metadir_create_file

[Christoph Hellwig]

On Sun, Jul 12, 2026 at 11:33:16AM +0200, Johannes Thumshirn wrote:
> +static int
> +xfs_dqinode_init(
> +	struct xfs_metadir_update	*upd,
> +	void				*priv)
> +{
> +	xfs_trans_log_inode(upd->tp, upd->ip, XFS_ILOG_CORE);
> +	return 0;
> +}

I wonder if we should just unconditionally re-log the inode in the
caller.  File creation already logs the inode core anyway, so this
really is a no-op.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/3] xfs: create rtgroup metadir inodes using xfs_metadir_create_file

[Christoph Hellwig]

On Sun, Jul 12, 2026 at 11:33:17AM +0200, Johannes Thumshirn wrote:
> Now that we have xfs_metadir_create_file() use it in
> xfs_rtginode_create().
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  fs/xfs/libxfs/xfs_rtgroup.c | 58 +++++++++++++++++--------------------
>  1 file changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_rtgroup.c b/fs/xfs/libxfs/xfs_rtgroup.c
> index caecc1e1f723..b6844b9af390 100644
> --- a/fs/xfs/libxfs/xfs_rtgroup.c
> +++ b/fs/xfs/libxfs/xfs_rtgroup.c
> @@ -547,6 +547,25 @@ xfs_rtginode_irele(
>  	*ipp = NULL;
>  }
>  
> +struct xfs_rtginode_create {
> +	struct xfs_rtgroup		*rtg;
> +	enum xfs_rtg_inodes		type;
> +	bool				init;
> +};
> +
> +static int
> +xfs_rtginode_init(
> +	struct xfs_metadir_update	*upd,
> +	void				*priv)
> +{
> +	struct xfs_rtginode_create	*rc = priv;
> +	const struct xfs_rtginode_ops	*ops = &xfs_rtginode_ops[rc->type];
> +
> +	xfs_rtginode_lockdep_setup(upd->ip, rtg_rgno(rc->rtg), rc->type);
> +	upd->ip->i_projid = rtg_rgno(rc->rtg);
> +	return ops->create(rc->rtg, upd->ip, upd->tp, rc->init);
> +}

The double indirection is a bit annoying, but given that this is not
a performance pass not a major issue.  I can't really hink of a good
way to get rid of it either.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Please add another patch at the end to mark xfs_metadir_start_create,
xfs_metadir_create and xfs_metadir_cancel static.