stats and error injection for zoned GC

stats and error injection for zoned GC

[Christoph Hellwig]

Hi all,

this series adds error injection for zoned resets and zoned GC stats.
My initial use is for a test case that verifies zone reset error
handling, but that stats should also be very useful for monitoring
tools like PCP and benchmarking.

As part of the error injection work I've also added a mount option to
enable error ibjection at mount time so that it can exercise the
mount code as well and cleaned up various other bits of the error
injection code.

Diffstat:
 Documentation/admin-guide/xfs.rst |    6 +
 fs/xfs/libxfs/xfs_errortag.h      |    8 +-
 fs/xfs/xfs_error.c                |  142 +++++++++++++++++++++-----------------
 fs/xfs/xfs_error.h                |   23 ++----
 fs/xfs/xfs_stats.c                |    7 +
 fs/xfs/xfs_stats.h                |    6 +
 fs/xfs/xfs_super.c                |   20 +++++
 fs/xfs/xfs_zone_gc.c              |   72 ++++++++++++-------
 8 files changed, 180 insertions(+), 104 deletions(-)

[PATCH 01/10] xfs: fix the errno sign for the xfs_errortag_{add,clearall} stubs

[Christoph Hellwig]

All errno values should be negative in the kernel.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_error.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index fe6a71bbe9cd..3a78c8dfaec8 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -60,8 +60,8 @@ int xfs_errortag_clearall(struct xfs_mount *mp);
 #define xfs_errortag_del(mp)
 #define XFS_TEST_ERROR(mp, tag)			(false)
 #define XFS_ERRORTAG_DELAY(mp, tag)		((void)0)
-#define xfs_errortag_add(mp, tag)		(ENOSYS)
-#define xfs_errortag_clearall(mp)		(ENOSYS)
+#define xfs_errortag_add(mp, tag)		(-ENOSYS)
+#define xfs_errortag_clearall(mp)		(-ENOSYS)
 #endif /* DEBUG */
 
 /*
-- 
2.47.3

Re: [PATCH 01/10] xfs: fix the errno sign for the xfs_errortag_{add,clearall} stubs

[Darrick J. Wong]

On Tue, Jan 27, 2026 at 05:05:41PM +0100, Christoph Hellwig wrote:
> All errno values should be negative in the kernel.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Yeah, that always seemed like a logic bomb waiting to go off...
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_error.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index fe6a71bbe9cd..3a78c8dfaec8 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -60,8 +60,8 @@ int xfs_errortag_clearall(struct xfs_mount *mp);
>  #define xfs_errortag_del(mp)
>  #define XFS_TEST_ERROR(mp, tag)			(false)
>  #define XFS_ERRORTAG_DELAY(mp, tag)		((void)0)
> -#define xfs_errortag_add(mp, tag)		(ENOSYS)
> -#define xfs_errortag_clearall(mp)		(ENOSYS)
> +#define xfs_errortag_add(mp, tag)		(-ENOSYS)
> +#define xfs_errortag_clearall(mp)		(-ENOSYS)
>  #endif /* DEBUG */
>  
>  /*
> -- 
> 2.47.3
> 
> 

Re: [PATCH 01/10] xfs: fix the errno sign for the xfs_errortag_{add,clearall} stubs

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:05:41PM +0100, Christoph Hellwig wrote:
> All errno values should be negative in the kernel.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_error.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index fe6a71bbe9cd..3a78c8dfaec8 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -60,8 +60,8 @@ int xfs_errortag_clearall(struct xfs_mount *mp);
>  #define xfs_errortag_del(mp)
>  #define XFS_TEST_ERROR(mp, tag)			(false)
>  #define XFS_ERRORTAG_DELAY(mp, tag)		((void)0)
> -#define xfs_errortag_add(mp, tag)		(ENOSYS)
> -#define xfs_errortag_clearall(mp)		(ENOSYS)
> +#define xfs_errortag_add(mp, tag)		(-ENOSYS)
> +#define xfs_errortag_clearall(mp)		(-ENOSYS)
>  #endif /* DEBUG */

Looks fine.

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

>  
>  /*
> -- 
> 2.47.3
> 
> 

Re: [PATCH 01/10] xfs: fix the errno sign for the xfs_errortag_{add,clearall} stubs

[Hans Holmberg]

On 27/01/2026 17:06, Christoph Hellwig wrote:
> All errno values should be negative in the kernel.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_error.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index fe6a71bbe9cd..3a78c8dfaec8 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -60,8 +60,8 @@ int xfs_errortag_clearall(struct xfs_mount *mp);
>  #define xfs_errortag_del(mp)
>  #define XFS_TEST_ERROR(mp, tag)			(false)
>  #define XFS_ERRORTAG_DELAY(mp, tag)		((void)0)
> -#define xfs_errortag_add(mp, tag)		(ENOSYS)
> -#define xfs_errortag_clearall(mp)		(ENOSYS)
> +#define xfs_errortag_add(mp, tag)		(-ENOSYS)
> +#define xfs_errortag_clearall(mp)		(-ENOSYS)
>  #endif /* DEBUG */
>  
>  /*

Looks good,

Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>

[PATCH 02/10] xfs: allocate m_errortag early

[Christoph Hellwig]

Ensure the mount structure always has a valid m_errortag for debug
builds.  This removes the NULL checking from the runtime code, and
prepares for allowing to set errortags from mount.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_error.c | 26 +-------------------------
 fs/xfs/xfs_super.c | 12 ++++++++++++
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 873f2d1a134c..dfa4abf9fd1a 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -114,18 +114,8 @@ int
 xfs_errortag_init(
 	struct xfs_mount	*mp)
 {
-	int ret;
-
-	mp->m_errortag = kzalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
-				GFP_KERNEL | __GFP_RETRY_MAYFAIL);
-	if (!mp->m_errortag)
-		return -ENOMEM;
-
-	ret = xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
+	return xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
 				&mp->m_kobj, "errortag");
-	if (ret)
-		kfree(mp->m_errortag);
-	return ret;
 }
 
 void
@@ -133,7 +123,6 @@ xfs_errortag_del(
 	struct xfs_mount	*mp)
 {
 	xfs_sysfs_del(&mp->m_errortag_kobj);
-	kfree(mp->m_errortag);
 }
 
 static bool
@@ -154,8 +143,6 @@ xfs_errortag_enabled(
 	struct xfs_mount	*mp,
 	unsigned int		tag)
 {
-	if (!mp->m_errortag)
-		return false;
 	if (!xfs_errortag_valid(tag))
 		return false;
 
@@ -171,17 +158,6 @@ xfs_errortag_test(
 {
 	unsigned int		randfactor;
 
-	/*
-	 * To be able to use error injection anywhere, we need to ensure error
-	 * injection mechanism is already initialized.
-	 *
-	 * Code paths like I/O completion can be called before the
-	 * initialization is complete, but be able to inject errors in such
-	 * places is still useful.
-	 */
-	if (!mp->m_errortag)
-		return false;
-
 	if (!xfs_errortag_valid(error_tag))
 		return false;
 
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index e05bf62a5413..ee335dbe5811 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -40,6 +40,7 @@
 #include "xfs_defer.h"
 #include "xfs_attr_item.h"
 #include "xfs_xattr.h"
+#include "xfs_errortag.h"
 #include "xfs_iunlink_item.h"
 #include "xfs_dahash_test.h"
 #include "xfs_rtbitmap.h"
@@ -822,6 +823,9 @@ xfs_mount_free(
 	debugfs_remove(mp->m_debugfs);
 	kfree(mp->m_rtname);
 	kfree(mp->m_logname);
+#ifdef DEBUG
+	kfree(mp->m_errortag);
+#endif
 	kfree(mp);
 }
 
@@ -2254,6 +2258,14 @@ xfs_init_fs_context(
 	mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL);
 	if (!mp)
 		return -ENOMEM;
+#ifdef DEBUG
+	mp->m_errortag = kcalloc(XFS_ERRTAG_MAX, sizeof(*mp->m_errortag),
+			GFP_KERNEL);
+	if (!mp->m_errortag) {
+		kfree(mp);
+		return -ENOMEM;
+	}
+#endif
 
 	spin_lock_init(&mp->m_sb_lock);
 	for (i = 0; i < XG_TYPE_MAX; i++)
-- 
2.47.3

Re: [PATCH 02/10] xfs: allocate m_errortag early

[Darrick J. Wong]

On Tue, Jan 27, 2026 at 05:05:42PM +0100, Christoph Hellwig wrote:
> Ensure the mount structure always has a valid m_errortag for debug
> builds.  This removes the NULL checking from the runtime code, and
> prepares for allowing to set errortags from mount.

Hrmm.  Are you /sure/ you want to allow errortag mount options?
Saying that only because I really hate mount options.

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

Pre-failing the errortag array creation doesn't bother me that much
though, so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_error.c | 26 +-------------------------
>  fs/xfs/xfs_super.c | 12 ++++++++++++
>  2 files changed, 13 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 873f2d1a134c..dfa4abf9fd1a 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -114,18 +114,8 @@ int
>  xfs_errortag_init(
>  	struct xfs_mount	*mp)
>  {
> -	int ret;
> -
> -	mp->m_errortag = kzalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
> -				GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> -	if (!mp->m_errortag)
> -		return -ENOMEM;
> -
> -	ret = xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
> +	return xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
>  				&mp->m_kobj, "errortag");
> -	if (ret)
> -		kfree(mp->m_errortag);
> -	return ret;
>  }
>  
>  void
> @@ -133,7 +123,6 @@ xfs_errortag_del(
>  	struct xfs_mount	*mp)
>  {
>  	xfs_sysfs_del(&mp->m_errortag_kobj);
> -	kfree(mp->m_errortag);
>  }
>  
>  static bool
> @@ -154,8 +143,6 @@ xfs_errortag_enabled(
>  	struct xfs_mount	*mp,
>  	unsigned int		tag)
>  {
> -	if (!mp->m_errortag)
> -		return false;
>  	if (!xfs_errortag_valid(tag))
>  		return false;
>  
> @@ -171,17 +158,6 @@ xfs_errortag_test(
>  {
>  	unsigned int		randfactor;
>  
> -	/*
> -	 * To be able to use error injection anywhere, we need to ensure error
> -	 * injection mechanism is already initialized.
> -	 *
> -	 * Code paths like I/O completion can be called before the
> -	 * initialization is complete, but be able to inject errors in such
> -	 * places is still useful.
> -	 */
> -	if (!mp->m_errortag)
> -		return false;
> -
>  	if (!xfs_errortag_valid(error_tag))
>  		return false;
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e05bf62a5413..ee335dbe5811 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -40,6 +40,7 @@
>  #include "xfs_defer.h"
>  #include "xfs_attr_item.h"
>  #include "xfs_xattr.h"
> +#include "xfs_errortag.h"
>  #include "xfs_iunlink_item.h"
>  #include "xfs_dahash_test.h"
>  #include "xfs_rtbitmap.h"
> @@ -822,6 +823,9 @@ xfs_mount_free(
>  	debugfs_remove(mp->m_debugfs);
>  	kfree(mp->m_rtname);
>  	kfree(mp->m_logname);
> +#ifdef DEBUG
> +	kfree(mp->m_errortag);
> +#endif
>  	kfree(mp);
>  }
>  
> @@ -2254,6 +2258,14 @@ xfs_init_fs_context(
>  	mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL);
>  	if (!mp)
>  		return -ENOMEM;
> +#ifdef DEBUG
> +	mp->m_errortag = kcalloc(XFS_ERRTAG_MAX, sizeof(*mp->m_errortag),
> +			GFP_KERNEL);
> +	if (!mp->m_errortag) {
> +		kfree(mp);
> +		return -ENOMEM;
> +	}
> +#endif
>  
>  	spin_lock_init(&mp->m_sb_lock);
>  	for (i = 0; i < XG_TYPE_MAX; i++)
> -- 
> 2.47.3
> 
> 

Re: [PATCH 02/10] xfs: allocate m_errortag early

[Christoph Hellwig]

On Tue, Jan 27, 2026 at 05:32:59PM -0800, Darrick J. Wong wrote:
> On Tue, Jan 27, 2026 at 05:05:42PM +0100, Christoph Hellwig wrote:
> > Ensure the mount structure always has a valid m_errortag for debug
> > builds.  This removes the NULL checking from the runtime code, and
> > prepares for allowing to set errortags from mount.
> 
> Hrmm.  Are you /sure/ you want to allow errortag mount options?
> Saying that only because I really hate mount options.

Well, if you have any other good idea to add error tags at mount
time I'm all ears :)

Re: [PATCH 02/10] xfs: allocate m_errortag early

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:05:42PM +0100, Christoph Hellwig wrote:
> Ensure the mount structure always has a valid m_errortag for debug
> builds.  This removes the NULL checking from the runtime code, and
> prepares for allowing to set errortags from mount.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_error.c | 26 +-------------------------
>  fs/xfs/xfs_super.c | 12 ++++++++++++
>  2 files changed, 13 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 873f2d1a134c..dfa4abf9fd1a 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -114,18 +114,8 @@ int
>  xfs_errortag_init(
>  	struct xfs_mount	*mp)
>  {
> -	int ret;
> -
> -	mp->m_errortag = kzalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
> -				GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> -	if (!mp->m_errortag)
> -		return -ENOMEM;
> -
> -	ret = xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
> +	return xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
>  				&mp->m_kobj, "errortag");
> -	if (ret)
> -		kfree(mp->m_errortag);
> -	return ret;
>  }
>  
>  void
> @@ -133,7 +123,6 @@ xfs_errortag_del(
>  	struct xfs_mount	*mp)
>  {
>  	xfs_sysfs_del(&mp->m_errortag_kobj);
> -	kfree(mp->m_errortag);
>  }
>  
>  static bool
> @@ -154,8 +143,6 @@ xfs_errortag_enabled(
>  	struct xfs_mount	*mp,
>  	unsigned int		tag)
>  {
> -	if (!mp->m_errortag)
> -		return false;
>  	if (!xfs_errortag_valid(tag))
>  		return false;
>  
> @@ -171,17 +158,6 @@ xfs_errortag_test(
>  {
>  	unsigned int		randfactor;
>  
> -	/*
> -	 * To be able to use error injection anywhere, we need to ensure error
> -	 * injection mechanism is already initialized.
> -	 *
> -	 * Code paths like I/O completion can be called before the
> -	 * initialization is complete, but be able to inject errors in such
> -	 * places is still useful.
> -	 */
> -	if (!mp->m_errortag)
> -		return false;
> -
>  	if (!xfs_errortag_valid(error_tag))
>  		return false;
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e05bf62a5413..ee335dbe5811 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -40,6 +40,7 @@
>  #include "xfs_defer.h"
>  #include "xfs_attr_item.h"
>  #include "xfs_xattr.h"
> +#include "xfs_errortag.h"
>  #include "xfs_iunlink_item.h"
>  #include "xfs_dahash_test.h"
>  #include "xfs_rtbitmap.h"
> @@ -822,6 +823,9 @@ xfs_mount_free(
>  	debugfs_remove(mp->m_debugfs);
>  	kfree(mp->m_rtname);
>  	kfree(mp->m_logname);
> +#ifdef DEBUG
> +	kfree(mp->m_errortag);
> +#endif
>  	kfree(mp);
>  }
>  
> @@ -2254,6 +2258,14 @@ xfs_init_fs_context(
>  	mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL);
>  	if (!mp)
>  		return -ENOMEM;
> +#ifdef DEBUG
> +	mp->m_errortag = kcalloc(XFS_ERRTAG_MAX, sizeof(*mp->m_errortag),
> +			GFP_KERNEL);
> +	if (!mp->m_errortag) {
> +		kfree(mp);
> +		return -ENOMEM;
> +	}
> +#endif

Looks fine to me

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

>  
>  	spin_lock_init(&mp->m_sb_lock);
>  	for (i = 0; i < XG_TYPE_MAX; i++)
> -- 
> 2.47.3
> 
> 

Re: [PATCH 02/10] xfs: allocate m_errortag early

[Hans Holmberg]

On 27/01/2026 17:06, Christoph Hellwig wrote:
> Ensure the mount structure always has a valid m_errortag for debug
> builds.  This removes the NULL checking from the runtime code, and
> prepares for allowing to set errortags from mount.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_error.c | 26 +-------------------------
>  fs/xfs/xfs_super.c | 12 ++++++++++++
>  2 files changed, 13 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 873f2d1a134c..dfa4abf9fd1a 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -114,18 +114,8 @@ int
>  xfs_errortag_init(
>  	struct xfs_mount	*mp)
>  {
> -	int ret;
> -
> -	mp->m_errortag = kzalloc(sizeof(unsigned int) * XFS_ERRTAG_MAX,
> -				GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> -	if (!mp->m_errortag)
> -		return -ENOMEM;
> -
> -	ret = xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
> +	return xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
>  				&mp->m_kobj, "errortag");
> -	if (ret)
> -		kfree(mp->m_errortag);
> -	return ret;
>  }
>  
>  void
> @@ -133,7 +123,6 @@ xfs_errortag_del(
>  	struct xfs_mount	*mp)
>  {
>  	xfs_sysfs_del(&mp->m_errortag_kobj);
> -	kfree(mp->m_errortag);
>  }
>  
>  static bool
> @@ -154,8 +143,6 @@ xfs_errortag_enabled(
>  	struct xfs_mount	*mp,
>  	unsigned int		tag)
>  {
> -	if (!mp->m_errortag)
> -		return false;
>  	if (!xfs_errortag_valid(tag))
>  		return false;
>  
> @@ -171,17 +158,6 @@ xfs_errortag_test(
>  {
>  	unsigned int		randfactor;
>  
> -	/*
> -	 * To be able to use error injection anywhere, we need to ensure error
> -	 * injection mechanism is already initialized.
> -	 *
> -	 * Code paths like I/O completion can be called before the
> -	 * initialization is complete, but be able to inject errors in such
> -	 * places is still useful.
> -	 */
> -	if (!mp->m_errortag)
> -		return false;
> -
>  	if (!xfs_errortag_valid(error_tag))
>  		return false;
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e05bf62a5413..ee335dbe5811 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -40,6 +40,7 @@
>  #include "xfs_defer.h"
>  #include "xfs_attr_item.h"
>  #include "xfs_xattr.h"
> +#include "xfs_errortag.h"
>  #include "xfs_iunlink_item.h"
>  #include "xfs_dahash_test.h"
>  #include "xfs_rtbitmap.h"
> @@ -822,6 +823,9 @@ xfs_mount_free(
>  	debugfs_remove(mp->m_debugfs);
>  	kfree(mp->m_rtname);
>  	kfree(mp->m_logname);
> +#ifdef DEBUG
> +	kfree(mp->m_errortag);
> +#endif
>  	kfree(mp);
>  }
>  
> @@ -2254,6 +2258,14 @@ xfs_init_fs_context(
>  	mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL);
>  	if (!mp)
>  		return -ENOMEM;
> +#ifdef DEBUG
> +	mp->m_errortag = kcalloc(XFS_ERRTAG_MAX, sizeof(*mp->m_errortag),
> +			GFP_KERNEL);
> +	if (!mp->m_errortag) {
> +		kfree(mp);
> +		return -ENOMEM;
> +	}
> +#endif
>  
>  	spin_lock_init(&mp->m_sb_lock);
>  	for (i = 0; i < XG_TYPE_MAX; i++)

Looks good,

Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>

[PATCH 03/10] xfs: don't validate error tags in the I/O path

[Christoph Hellwig]

We can trust XFS developers enough to not pass random stuff to
XFS_ERROR_TEST/DELAY.  Open code the validity check in xfs_errortag_add,
which is the only place that receives unvalidated error tag values from
user space, and drop the now pointless xfs_errortag_enabled helper.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_errortag.h |  2 +-
 fs/xfs/xfs_error.c           | 38 ++++++++++--------------------------
 fs/xfs/xfs_error.h           |  2 +-
 3 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
index 57e47077c75a..b7d98471684b 100644
--- a/fs/xfs/libxfs/xfs_errortag.h
+++ b/fs/xfs/libxfs/xfs_errortag.h
@@ -53,7 +53,7 @@
  * Drop-writes support removed because write error handling cannot trash
  * pre-existing delalloc extents in any useful way anymore. We retain the
  * definition so that we can reject it as an invalid value in
- * xfs_errortag_valid().
+ * xfs_errortag_add().
  */
 #define XFS_ERRTAG_DROP_WRITES				28
 #define XFS_ERRTAG_LOG_BAD_CRC				29
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index dfa4abf9fd1a..52a1d51126e3 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -125,30 +125,6 @@ xfs_errortag_del(
 	xfs_sysfs_del(&mp->m_errortag_kobj);
 }
 
-static bool
-xfs_errortag_valid(
-	unsigned int		error_tag)
-{
-	if (error_tag >= XFS_ERRTAG_MAX)
-		return false;
-
-	/* Error out removed injection types */
-	if (error_tag == XFS_ERRTAG_DROP_WRITES)
-		return false;
-	return true;
-}
-
-bool
-xfs_errortag_enabled(
-	struct xfs_mount	*mp,
-	unsigned int		tag)
-{
-	if (!xfs_errortag_valid(tag))
-		return false;
-
-	return mp->m_errortag[tag] != 0;
-}
-
 bool
 xfs_errortag_test(
 	struct xfs_mount	*mp,
@@ -158,9 +134,6 @@ xfs_errortag_test(
 {
 	unsigned int		randfactor;
 
-	if (!xfs_errortag_valid(error_tag))
-		return false;
-
 	randfactor = mp->m_errortag[error_tag];
 	if (!randfactor || get_random_u32_below(randfactor))
 		return false;
@@ -178,8 +151,17 @@ xfs_errortag_add(
 {
 	BUILD_BUG_ON(ARRAY_SIZE(xfs_errortag_random_default) != XFS_ERRTAG_MAX);
 
-	if (!xfs_errortag_valid(error_tag))
+	if (error_tag >= XFS_ERRTAG_MAX)
+		return -EINVAL;
+
+	/* Error out removed injection types */
+	switch (error_tag) {
+	case XFS_ERRTAG_DROP_WRITES:
 		return -EINVAL;
+	default:
+		break;
+	}
+
 	mp->m_errortag[error_tag] = xfs_errortag_random_default[error_tag];
 	return 0;
 }
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index 3a78c8dfaec8..ec22546a8ca8 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -44,7 +44,7 @@ bool xfs_errortag_enabled(struct xfs_mount *mp, unsigned int tag);
 #define XFS_ERRORTAG_DELAY(mp, tag)		\
 	do { \
 		might_sleep(); \
-		if (!xfs_errortag_enabled((mp), (tag))) \
+		if (!mp->m_errortag[tag]) \
 			break; \
 		xfs_warn_ratelimited((mp), \
 "Injecting %ums delay at file %s, line %d, on filesystem \"%s\"", \
-- 
2.47.3

Re: [PATCH 03/10] xfs: don't validate error tags in the I/O path

[Darrick J. Wong]

On Tue, Jan 27, 2026 at 05:05:43PM +0100, Christoph Hellwig wrote:
> We can trust XFS developers enough to not pass random stuff to
> XFS_ERROR_TEST/DELAY.  Open code the validity check in xfs_errortag_add,
> which is the only place that receives unvalidated error tag values from
> user space, and drop the now pointless xfs_errortag_enabled helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

That seems reasonable; these are debug knobs after all.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_errortag.h |  2 +-
>  fs/xfs/xfs_error.c           | 38 ++++++++++--------------------------
>  fs/xfs/xfs_error.h           |  2 +-
>  3 files changed, 12 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index 57e47077c75a..b7d98471684b 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -53,7 +53,7 @@
>   * Drop-writes support removed because write error handling cannot trash
>   * pre-existing delalloc extents in any useful way anymore. We retain the
>   * definition so that we can reject it as an invalid value in
> - * xfs_errortag_valid().
> + * xfs_errortag_add().
>   */
>  #define XFS_ERRTAG_DROP_WRITES				28
>  #define XFS_ERRTAG_LOG_BAD_CRC				29
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index dfa4abf9fd1a..52a1d51126e3 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -125,30 +125,6 @@ xfs_errortag_del(
>  	xfs_sysfs_del(&mp->m_errortag_kobj);
>  }
>  
> -static bool
> -xfs_errortag_valid(
> -	unsigned int		error_tag)
> -{
> -	if (error_tag >= XFS_ERRTAG_MAX)
> -		return false;
> -
> -	/* Error out removed injection types */
> -	if (error_tag == XFS_ERRTAG_DROP_WRITES)
> -		return false;
> -	return true;
> -}
> -
> -bool
> -xfs_errortag_enabled(
> -	struct xfs_mount	*mp,
> -	unsigned int		tag)
> -{
> -	if (!xfs_errortag_valid(tag))
> -		return false;
> -
> -	return mp->m_errortag[tag] != 0;
> -}
> -
>  bool
>  xfs_errortag_test(
>  	struct xfs_mount	*mp,
> @@ -158,9 +134,6 @@ xfs_errortag_test(
>  {
>  	unsigned int		randfactor;
>  
> -	if (!xfs_errortag_valid(error_tag))
> -		return false;
> -
>  	randfactor = mp->m_errortag[error_tag];
>  	if (!randfactor || get_random_u32_below(randfactor))
>  		return false;
> @@ -178,8 +151,17 @@ xfs_errortag_add(
>  {
>  	BUILD_BUG_ON(ARRAY_SIZE(xfs_errortag_random_default) != XFS_ERRTAG_MAX);
>  
> -	if (!xfs_errortag_valid(error_tag))
> +	if (error_tag >= XFS_ERRTAG_MAX)
> +		return -EINVAL;
> +
> +	/* Error out removed injection types */
> +	switch (error_tag) {
> +	case XFS_ERRTAG_DROP_WRITES:
>  		return -EINVAL;
> +	default:
> +		break;
> +	}
> +
>  	mp->m_errortag[error_tag] = xfs_errortag_random_default[error_tag];
>  	return 0;
>  }
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index 3a78c8dfaec8..ec22546a8ca8 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -44,7 +44,7 @@ bool xfs_errortag_enabled(struct xfs_mount *mp, unsigned int tag);
>  #define XFS_ERRORTAG_DELAY(mp, tag)		\
>  	do { \
>  		might_sleep(); \
> -		if (!xfs_errortag_enabled((mp), (tag))) \
> +		if (!mp->m_errortag[tag]) \
>  			break; \
>  		xfs_warn_ratelimited((mp), \
>  "Injecting %ums delay at file %s, line %d, on filesystem \"%s\"", \
> -- 
> 2.47.3
> 
> 

Re: [PATCH 03/10] xfs: don't validate error tags in the I/O path

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:05:43PM +0100, Christoph Hellwig wrote:
> We can trust XFS developers enough to not pass random stuff to
> XFS_ERROR_TEST/DELAY.  Open code the validity check in xfs_errortag_add,
> which is the only place that receives unvalidated error tag values from
> user space, and drop the now pointless xfs_errortag_enabled helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_errortag.h |  2 +-
>  fs/xfs/xfs_error.c           | 38 ++++++++++--------------------------
>  fs/xfs/xfs_error.h           |  2 +-
>  3 files changed, 12 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index 57e47077c75a..b7d98471684b 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -53,7 +53,7 @@
>   * Drop-writes support removed because write error handling cannot trash
>   * pre-existing delalloc extents in any useful way anymore. We retain the
>   * definition so that we can reject it as an invalid value in
> - * xfs_errortag_valid().
> + * xfs_errortag_add().

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

>   */
>  #define XFS_ERRTAG_DROP_WRITES				28
>  #define XFS_ERRTAG_LOG_BAD_CRC				29
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index dfa4abf9fd1a..52a1d51126e3 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -125,30 +125,6 @@ xfs_errortag_del(
>  	xfs_sysfs_del(&mp->m_errortag_kobj);
>  }
>  
> -static bool
> -xfs_errortag_valid(
> -	unsigned int		error_tag)
> -{
> -	if (error_tag >= XFS_ERRTAG_MAX)
> -		return false;
> -
> -	/* Error out removed injection types */
> -	if (error_tag == XFS_ERRTAG_DROP_WRITES)
> -		return false;
> -	return true;
> -}
> -
> -bool
> -xfs_errortag_enabled(
> -	struct xfs_mount	*mp,
> -	unsigned int		tag)
> -{
> -	if (!xfs_errortag_valid(tag))
> -		return false;
> -
> -	return mp->m_errortag[tag] != 0;
> -}
> -
>  bool
>  xfs_errortag_test(
>  	struct xfs_mount	*mp,
> @@ -158,9 +134,6 @@ xfs_errortag_test(
>  {
>  	unsigned int		randfactor;
>  
> -	if (!xfs_errortag_valid(error_tag))
> -		return false;
> -
>  	randfactor = mp->m_errortag[error_tag];
>  	if (!randfactor || get_random_u32_below(randfactor))
>  		return false;
> @@ -178,8 +151,17 @@ xfs_errortag_add(
>  {
>  	BUILD_BUG_ON(ARRAY_SIZE(xfs_errortag_random_default) != XFS_ERRTAG_MAX);
>  
> -	if (!xfs_errortag_valid(error_tag))
> +	if (error_tag >= XFS_ERRTAG_MAX)
> +		return -EINVAL;
> +
> +	/* Error out removed injection types */
> +	switch (error_tag) {
> +	case XFS_ERRTAG_DROP_WRITES:
>  		return -EINVAL;
> +	default:
> +		break;
> +	}
> +
>  	mp->m_errortag[error_tag] = xfs_errortag_random_default[error_tag];
>  	return 0;
>  }
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index 3a78c8dfaec8..ec22546a8ca8 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -44,7 +44,7 @@ bool xfs_errortag_enabled(struct xfs_mount *mp, unsigned int tag);
>  #define XFS_ERRORTAG_DELAY(mp, tag)		\
>  	do { \
>  		might_sleep(); \
> -		if (!xfs_errortag_enabled((mp), (tag))) \
> +		if (!mp->m_errortag[tag]) \
>  			break; \
>  		xfs_warn_ratelimited((mp), \
>  "Injecting %ums delay at file %s, line %d, on filesystem \"%s\"", \
> -- 
> 2.47.3
> 
> 

Re: [PATCH 03/10] xfs: don't validate error tags in the I/O path

[Hans Holmberg]

On 27/01/2026 17:06, Christoph Hellwig wrote:
> We can trust XFS developers enough to not pass random stuff to
> XFS_ERROR_TEST/DELAY.  Open code the validity check in xfs_errortag_add,
> which is the only place that receives unvalidated error tag values from
> user space, and drop the now pointless xfs_errortag_enabled helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_errortag.h |  2 +-
>  fs/xfs/xfs_error.c           | 38 ++++++++++--------------------------
>  fs/xfs/xfs_error.h           |  2 +-
>  3 files changed, 12 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index 57e47077c75a..b7d98471684b 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -53,7 +53,7 @@
>   * Drop-writes support removed because write error handling cannot trash
>   * pre-existing delalloc extents in any useful way anymore. We retain the
>   * definition so that we can reject it as an invalid value in
> - * xfs_errortag_valid().
> + * xfs_errortag_add().
>   */
>  #define XFS_ERRTAG_DROP_WRITES				28
>  #define XFS_ERRTAG_LOG_BAD_CRC				29
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index dfa4abf9fd1a..52a1d51126e3 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -125,30 +125,6 @@ xfs_errortag_del(
>  	xfs_sysfs_del(&mp->m_errortag_kobj);
>  }
>  
> -static bool
> -xfs_errortag_valid(
> -	unsigned int		error_tag)
> -{
> -	if (error_tag >= XFS_ERRTAG_MAX)
> -		return false;
> -
> -	/* Error out removed injection types */
> -	if (error_tag == XFS_ERRTAG_DROP_WRITES)
> -		return false;
> -	return true;
> -}
> -
> -bool
> -xfs_errortag_enabled(
> -	struct xfs_mount	*mp,
> -	unsigned int		tag)
> -{
> -	if (!xfs_errortag_valid(tag))
> -		return false;
> -
> -	return mp->m_errortag[tag] != 0;
> -}
> -
>  bool
>  xfs_errortag_test(
>  	struct xfs_mount	*mp,
> @@ -158,9 +134,6 @@ xfs_errortag_test(
>  {
>  	unsigned int		randfactor;
>  
> -	if (!xfs_errortag_valid(error_tag))
> -		return false;
> -
>  	randfactor = mp->m_errortag[error_tag];
>  	if (!randfactor || get_random_u32_below(randfactor))
>  		return false;
> @@ -178,8 +151,17 @@ xfs_errortag_add(
>  {
>  	BUILD_BUG_ON(ARRAY_SIZE(xfs_errortag_random_default) != XFS_ERRTAG_MAX);
>  
> -	if (!xfs_errortag_valid(error_tag))
> +	if (error_tag >= XFS_ERRTAG_MAX)
> +		return -EINVAL;
> +
> +	/* Error out removed injection types */
> +	switch (error_tag) {
> +	case XFS_ERRTAG_DROP_WRITES:
>  		return -EINVAL;
> +	default:
> +		break;
> +	}
> +
>  	mp->m_errortag[error_tag] = xfs_errortag_random_default[error_tag];
>  	return 0;
>  }
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index 3a78c8dfaec8..ec22546a8ca8 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -44,7 +44,7 @@ bool xfs_errortag_enabled(struct xfs_mount *mp, unsigned int tag);
>  #define XFS_ERRORTAG_DELAY(mp, tag)		\
>  	do { \
>  		might_sleep(); \
> -		if (!xfs_errortag_enabled((mp), (tag))) \
> +		if (!mp->m_errortag[tag]) \
>  			break; \
>  		xfs_warn_ratelimited((mp), \
>  "Injecting %ums delay at file %s, line %d, on filesystem \"%s\"", \

Looks good,

Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>

[PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Christoph Hellwig]

Mirror what is done for the more common XFS_ERRORTAG_TEST version,
and also only look at the error tag value once now that we can
easily have a local variable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_error.c | 21 +++++++++++++++++++++
 fs/xfs/xfs_error.h | 15 +++------------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 52a1d51126e3..a6f160a4d0e9 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -144,6 +144,27 @@ xfs_errortag_test(
 	return true;
 }
 
+void
+xfs_errortag_delay(
+	struct xfs_mount	*mp,
+	const char		*file,
+	int			line,
+	unsigned int		error_tag)
+{
+	unsigned int		delay = mp->m_errortag[error_tag];
+
+	might_sleep();
+
+	if (!delay)
+		return;
+
+	xfs_warn_ratelimited(mp,
+"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"",
+		delay, file, line,
+		mp->m_super->s_id);
+	mdelay(delay);
+}
+
 int
 xfs_errortag_add(
 	struct xfs_mount	*mp,
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index ec22546a8ca8..b40e7c671d2a 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -40,19 +40,10 @@ bool xfs_errortag_test(struct xfs_mount *mp, const char *file, int line,
 		unsigned int error_tag);
 #define XFS_TEST_ERROR(mp, tag)		\
 	xfs_errortag_test((mp), __FILE__, __LINE__, (tag))
-bool xfs_errortag_enabled(struct xfs_mount *mp, unsigned int tag);
+void xfs_errortag_delay(struct xfs_mount *mp, const char *file, int line,
+		unsigned int error_tag);
 #define XFS_ERRORTAG_DELAY(mp, tag)		\
-	do { \
-		might_sleep(); \
-		if (!mp->m_errortag[tag]) \
-			break; \
-		xfs_warn_ratelimited((mp), \
-"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"", \
-				(mp)->m_errortag[(tag)], __FILE__, __LINE__, \
-				(mp)->m_super->s_id); \
-		mdelay((mp)->m_errortag[(tag)]); \
-	} while (0)
-
+	xfs_errortag_delay((mp), __FILE__, __LINE__, (tag))
 int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
 int xfs_errortag_clearall(struct xfs_mount *mp);
 #else
-- 
2.47.3

Re: [PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Darrick J. Wong]

On Tue, Jan 27, 2026 at 05:05:44PM +0100, Christoph Hellwig wrote:
> Mirror what is done for the more common XFS_ERRORTAG_TEST version,
> and also only look at the error tag value once now that we can
> easily have a local variable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_error.c | 21 +++++++++++++++++++++
>  fs/xfs/xfs_error.h | 15 +++------------
>  2 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 52a1d51126e3..a6f160a4d0e9 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -144,6 +144,27 @@ xfs_errortag_test(
>  	return true;
>  }
>  
> +void
> +xfs_errortag_delay(
> +	struct xfs_mount	*mp,
> +	const char		*file,
> +	int			line,
> +	unsigned int		error_tag)
> +{
> +	unsigned int		delay = mp->m_errortag[error_tag];
> +
> +	might_sleep();
> +
> +	if (!delay)
> +		return;
> +
> +	xfs_warn_ratelimited(mp,
> +"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"",
> +		delay, file, line,
> +		mp->m_super->s_id);

Hrm.  This changes the logging ratelimiting from per-injection-site to
global for the whole kernel.  I'm mostly ok with that since I rarely
read dmesg, but does anyone else care?

If not, then
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> +	mdelay(delay);
> +}
> +
>  int
>  xfs_errortag_add(
>  	struct xfs_mount	*mp,
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index ec22546a8ca8..b40e7c671d2a 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -40,19 +40,10 @@ bool xfs_errortag_test(struct xfs_mount *mp, const char *file, int line,
>  		unsigned int error_tag);
>  #define XFS_TEST_ERROR(mp, tag)		\
>  	xfs_errortag_test((mp), __FILE__, __LINE__, (tag))
> -bool xfs_errortag_enabled(struct xfs_mount *mp, unsigned int tag);
> +void xfs_errortag_delay(struct xfs_mount *mp, const char *file, int line,
> +		unsigned int error_tag);
>  #define XFS_ERRORTAG_DELAY(mp, tag)		\
> -	do { \
> -		might_sleep(); \
> -		if (!mp->m_errortag[tag]) \
> -			break; \
> -		xfs_warn_ratelimited((mp), \
> -"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"", \
> -				(mp)->m_errortag[(tag)], __FILE__, __LINE__, \
> -				(mp)->m_super->s_id); \
> -		mdelay((mp)->m_errortag[(tag)]); \
> -	} while (0)
> -
> +	xfs_errortag_delay((mp), __FILE__, __LINE__, (tag))
>  int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
>  int xfs_errortag_clearall(struct xfs_mount *mp);
>  #else
> -- 
> 2.47.3
> 
> 

Re: [PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Christoph Hellwig]

On Tue, Jan 27, 2026 at 05:35:25PM -0800, Darrick J. Wong wrote:
> > +	xfs_warn_ratelimited(mp,
> > +"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"",
> > +		delay, file, line,
> > +		mp->m_super->s_id);
> 
> Hrm.  This changes the logging ratelimiting from per-injection-site to
> global for the whole kernel.

True.  But this has a total of three callsites and now matches what the
more common XFS_TEST_ERROR does.  So if this was an issue, we'd have
noticed it with XFS_TEST_ERROR.

Talking about dmesg - one thing that annoys me for both of them is
that the messages is prefixed with the file system ID, but then
also adds ', on filesystem \"%s\"' at the end, which is a bit silly.

Re: [PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Darrick J. Wong]

On Wed, Jan 28, 2026 at 04:44:59AM +0100, Christoph Hellwig wrote:
> On Tue, Jan 27, 2026 at 05:35:25PM -0800, Darrick J. Wong wrote:
> > > +	xfs_warn_ratelimited(mp,
> > > +"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"",
> > > +		delay, file, line,
> > > +		mp->m_super->s_id);
> > 
> > Hrm.  This changes the logging ratelimiting from per-injection-site to
> > global for the whole kernel.
> 
> True.  But this has a total of three callsites and now matches what the
> more common XFS_TEST_ERROR does.  So if this was an issue, we'd have
> noticed it with XFS_TEST_ERROR.
> 
> Talking about dmesg - one thing that annoys me for both of them is
> that the messages is prefixed with the file system ID, but then
> also adds ', on filesystem \"%s\"' at the end, which is a bit silly.

<nod> I think it's silly too.  Want to send a patch for me to rvb?

--D

Re: [PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Christoph Hellwig]

On Tue, Jan 27, 2026 at 09:02:55PM -0800, Darrick J. Wong wrote:
> > Talking about dmesg - one thing that annoys me for both of them is
> > that the messages is prefixed with the file system ID, but then
> > also adds ', on filesystem \"%s\"' at the end, which is a bit silly.
> 
> <nod> I think it's silly too.  Want to send a patch for me to rvb?

Once I get through the rest of my urgent and medium priority queue,
sure..

Re: [PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:35:25PM -0800, Darrick J. Wong wrote:
> On Tue, Jan 27, 2026 at 05:05:44PM +0100, Christoph Hellwig wrote:
> > Mirror what is done for the more common XFS_ERRORTAG_TEST version,
> > and also only look at the error tag value once now that we can
> > easily have a local variable.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/xfs_error.c | 21 +++++++++++++++++++++
> >  fs/xfs/xfs_error.h | 15 +++------------
> >  2 files changed, 24 insertions(+), 12 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> > index 52a1d51126e3..a6f160a4d0e9 100644
> > --- a/fs/xfs/xfs_error.c
> > +++ b/fs/xfs/xfs_error.c
> > @@ -144,6 +144,27 @@ xfs_errortag_test(
> >  	return true;
> >  }
> >  
> > +void
> > +xfs_errortag_delay(
> > +	struct xfs_mount	*mp,
> > +	const char		*file,
> > +	int			line,
> > +	unsigned int		error_tag)
> > +{
> > +	unsigned int		delay = mp->m_errortag[error_tag];
> > +
> > +	might_sleep();
> > +
> > +	if (!delay)
> > +		return;
> > +
> > +	xfs_warn_ratelimited(mp,
> > +"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"",
> > +		delay, file, line,
> > +		mp->m_super->s_id);
> 
> Hrm.  This changes the logging ratelimiting from per-injection-site to
> global for the whole kernel.  I'm mostly ok with that since I rarely
> read dmesg, but does anyone else care?

It's a valid concern IMHO, but the default rates are high enough for
this purpose, so I don't see an issue with it.

I'm curious though if is there any reason why those are ratelimited?
It's not like users would be running with error injection enabled or,
they would be flooding the message ring buffer with it.

> 
> If not, then
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> --D
> 
> > +	mdelay(delay);
> > +}
> > +
> >  int
> >  xfs_errortag_add(
> >  	struct xfs_mount	*mp,
> > diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> > index ec22546a8ca8..b40e7c671d2a 100644
> > --- a/fs/xfs/xfs_error.h
> > +++ b/fs/xfs/xfs_error.h
> > @@ -40,19 +40,10 @@ bool xfs_errortag_test(struct xfs_mount *mp, const char *file, int line,
> >  		unsigned int error_tag);
> >  #define XFS_TEST_ERROR(mp, tag)		\
> >  	xfs_errortag_test((mp), __FILE__, __LINE__, (tag))
> > -bool xfs_errortag_enabled(struct xfs_mount *mp, unsigned int tag);
> > +void xfs_errortag_delay(struct xfs_mount *mp, const char *file, int line,
> > +		unsigned int error_tag);
> >  #define XFS_ERRORTAG_DELAY(mp, tag)		\
> > -	do { \
> > -		might_sleep(); \
> > -		if (!mp->m_errortag[tag]) \
> > -			break; \
> > -		xfs_warn_ratelimited((mp), \
> > -"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"", \
> > -				(mp)->m_errortag[(tag)], __FILE__, __LINE__, \
> > -				(mp)->m_super->s_id); \
> > -		mdelay((mp)->m_errortag[(tag)]); \
> > -	} while (0)
> > -
> > +	xfs_errortag_delay((mp), __FILE__, __LINE__, (tag))
> >  int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
> >  int xfs_errortag_clearall(struct xfs_mount *mp);
> >  #else
> > -- 
> > 2.47.3
> > 
> > 
> 

Re: [PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Christoph Hellwig]

On Wed, Jan 28, 2026 at 12:18:38PM +0100, Carlos Maiolino wrote:
> I'm curious though if is there any reason why those are ratelimited?
> It's not like users would be running with error injection enabled or,
> they would be flooding the message ring buffer with it.

Good question.  I'd rather not open that can of worms in this series,
though.

Re: [PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Darrick J. Wong]

On Wed, Jan 28, 2026 at 03:10:16PM +0100, Christoph Hellwig wrote:
> On Wed, Jan 28, 2026 at 12:18:38PM +0100, Carlos Maiolino wrote:
> > I'm curious though if is there any reason why those are ratelimited?
> > It's not like users would be running with error injection enabled or,
> > they would be flooding the message ring buffer with it.
> 
> Good question.  I'd rather not open that can of worms in this series,
> though.

Well... activating an error injector /can/ trigger a lot of activations,
and it's annoying to try to sift through pages and pages of the same
message whilst trying to figure out why an fstest broke.  So I'd leave
the ratelimiting in.

--D

Re: [PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Carlos Maiolino]

On Wed, Jan 28, 2026 at 08:09:37AM -0800, Darrick J. Wong wrote:
> On Wed, Jan 28, 2026 at 03:10:16PM +0100, Christoph Hellwig wrote:
> > On Wed, Jan 28, 2026 at 12:18:38PM +0100, Carlos Maiolino wrote:
> > > I'm curious though if is there any reason why those are ratelimited?
> > > It's not like users would be running with error injection enabled or,
> > > they would be flooding the message ring buffer with it.
> > 
> > Good question.  I'd rather not open that can of worms in this series,
> > though.

Oh, for sure. I'm not talking about this series, it's mostly a food for
thought..

> 
> Well... activating an error injector /can/ trigger a lot of activations,
> and it's annoying to try to sift through pages and pages of the same
> message whilst trying to figure out why an fstest broke.  So I'd leave
> the ratelimiting in.

Fair enough.

> 
> --D
> 

Re: [PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:05:44PM +0100, Christoph Hellwig wrote:
> Mirror what is done for the more common XFS_ERRORTAG_TEST version,
> and also only look at the error tag value once now that we can
> easily have a local variable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_error.c | 21 +++++++++++++++++++++
>  fs/xfs/xfs_error.h | 15 +++------------
>  2 files changed, 24 insertions(+), 12 deletions(-)

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 52a1d51126e3..a6f160a4d0e9 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -144,6 +144,27 @@ xfs_errortag_test(
>  	return true;
>  }
>  
> +void
> +xfs_errortag_delay(
> +	struct xfs_mount	*mp,
> +	const char		*file,
> +	int			line,
> +	unsigned int		error_tag)
> +{
> +	unsigned int		delay = mp->m_errortag[error_tag];
> +
> +	might_sleep();
> +
> +	if (!delay)
> +		return;
> +
> +	xfs_warn_ratelimited(mp,
> +"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"",
> +		delay, file, line,
> +		mp->m_super->s_id);
> +	mdelay(delay);
> +}
> +
>  int
>  xfs_errortag_add(
>  	struct xfs_mount	*mp,
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index ec22546a8ca8..b40e7c671d2a 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -40,19 +40,10 @@ bool xfs_errortag_test(struct xfs_mount *mp, const char *file, int line,
>  		unsigned int error_tag);
>  #define XFS_TEST_ERROR(mp, tag)		\
>  	xfs_errortag_test((mp), __FILE__, __LINE__, (tag))
> -bool xfs_errortag_enabled(struct xfs_mount *mp, unsigned int tag);
> +void xfs_errortag_delay(struct xfs_mount *mp, const char *file, int line,
> +		unsigned int error_tag);
>  #define XFS_ERRORTAG_DELAY(mp, tag)		\
> -	do { \
> -		might_sleep(); \
> -		if (!mp->m_errortag[tag]) \
> -			break; \
> -		xfs_warn_ratelimited((mp), \
> -"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"", \
> -				(mp)->m_errortag[(tag)], __FILE__, __LINE__, \
> -				(mp)->m_super->s_id); \
> -		mdelay((mp)->m_errortag[(tag)]); \
> -	} while (0)
> -
> +	xfs_errortag_delay((mp), __FILE__, __LINE__, (tag))
>  int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
>  int xfs_errortag_clearall(struct xfs_mount *mp);
>  #else
> -- 
> 2.47.3
> 
> 

Re: [PATCH 04/10] xfs: move the guts of XFS_ERRORTAG_DELAY out of line

[Hans Holmberg]

On 27/01/2026 17:06, Christoph Hellwig wrote:
> Mirror what is done for the more common XFS_ERRORTAG_TEST version,
> and also only look at the error tag value once now that we can
> easily have a local variable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_error.c | 21 +++++++++++++++++++++
>  fs/xfs/xfs_error.h | 15 +++------------
>  2 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 52a1d51126e3..a6f160a4d0e9 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -144,6 +144,27 @@ xfs_errortag_test(
>  	return true;
>  }
>  
> +void
> +xfs_errortag_delay(
> +	struct xfs_mount	*mp,
> +	const char		*file,
> +	int			line,
> +	unsigned int		error_tag)
> +{
> +	unsigned int		delay = mp->m_errortag[error_tag];
> +
> +	might_sleep();
> +
> +	if (!delay)
> +		return;
> +
> +	xfs_warn_ratelimited(mp,
> +"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"",
> +		delay, file, line,
> +		mp->m_super->s_id);
> +	mdelay(delay);
> +}
> +
>  int
>  xfs_errortag_add(
>  	struct xfs_mount	*mp,
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index ec22546a8ca8..b40e7c671d2a 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -40,19 +40,10 @@ bool xfs_errortag_test(struct xfs_mount *mp, const char *file, int line,
>  		unsigned int error_tag);
>  #define XFS_TEST_ERROR(mp, tag)		\
>  	xfs_errortag_test((mp), __FILE__, __LINE__, (tag))
> -bool xfs_errortag_enabled(struct xfs_mount *mp, unsigned int tag);
> +void xfs_errortag_delay(struct xfs_mount *mp, const char *file, int line,
> +		unsigned int error_tag);
>  #define XFS_ERRORTAG_DELAY(mp, tag)		\
> -	do { \
> -		might_sleep(); \
> -		if (!mp->m_errortag[tag]) \
> -			break; \
> -		xfs_warn_ratelimited((mp), \
> -"Injecting %ums delay at file %s, line %d, on filesystem \"%s\"", \
> -				(mp)->m_errortag[(tag)], __FILE__, __LINE__, \
> -				(mp)->m_super->s_id); \
> -		mdelay((mp)->m_errortag[(tag)]); \
> -	} while (0)
> -
> +	xfs_errortag_delay((mp), __FILE__, __LINE__, (tag))
>  int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
>  int xfs_errortag_clearall(struct xfs_mount *mp);
>  #else


Looks good,

Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>

[PATCH 05/10] xfs: use WRITE_ONCE/READ_ONCE for m_errortag

[Christoph Hellwig]

There is no synchronization for updating m_errortag, which is fine as
it's just a debug tool.  It would still be nice to fully avoid the
theoretical case of torn values, so use WRITE_ONCE and READ_ONCE to
access the members.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_error.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index a6f160a4d0e9..53704f1ed791 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -50,17 +50,18 @@ xfs_errortag_attr_store(
 {
 	struct xfs_mount	*mp = to_mp(kobject);
 	unsigned int		error_tag = to_attr(attr)->tag;
+	unsigned int		val;
 	int			ret;
 
 	if (strcmp(buf, "default") == 0) {
-		mp->m_errortag[error_tag] =
-			xfs_errortag_random_default[error_tag];
+		val = xfs_errortag_random_default[error_tag];
 	} else {
-		ret = kstrtouint(buf, 0, &mp->m_errortag[error_tag]);
+		ret = kstrtouint(buf, 0, &val);
 		if (ret)
 			return ret;
 	}
 
+	WRITE_ONCE(mp->m_errortag[error_tag], val);
 	return count;
 }
 
@@ -71,9 +72,9 @@ xfs_errortag_attr_show(
 	char			*buf)
 {
 	struct xfs_mount	*mp = to_mp(kobject);
-	unsigned int		error_tag = to_attr(attr)->tag;
 
-	return snprintf(buf, PAGE_SIZE, "%u\n", mp->m_errortag[error_tag]);
+	return snprintf(buf, PAGE_SIZE, "%u\n",
+			READ_ONCE(mp->m_errortag[to_attr(attr)->tag]));
 }
 
 static const struct sysfs_ops xfs_errortag_sysfs_ops = {
@@ -134,7 +135,7 @@ xfs_errortag_test(
 {
 	unsigned int		randfactor;
 
-	randfactor = mp->m_errortag[error_tag];
+	randfactor = READ_ONCE(mp->m_errortag[error_tag]);
 	if (!randfactor || get_random_u32_below(randfactor))
 		return false;
 
@@ -151,7 +152,7 @@ xfs_errortag_delay(
 	int			line,
 	unsigned int		error_tag)
 {
-	unsigned int		delay = mp->m_errortag[error_tag];
+	unsigned int		delay = READ_ONCE(mp->m_errortag[error_tag]);
 
 	might_sleep();
 
@@ -183,7 +184,8 @@ xfs_errortag_add(
 		break;
 	}
 
-	mp->m_errortag[error_tag] = xfs_errortag_random_default[error_tag];
+	WRITE_ONCE(mp->m_errortag[error_tag],
+		   xfs_errortag_random_default[error_tag]);
 	return 0;
 }
 
@@ -191,7 +193,10 @@ int
 xfs_errortag_clearall(
 	struct xfs_mount	*mp)
 {
-	memset(mp->m_errortag, 0, sizeof(unsigned int) * XFS_ERRTAG_MAX);
+	unsigned int		i;
+
+	for (i = 0; i < XFS_ERRTAG_MAX; i++)
+		WRITE_ONCE(mp->m_errortag[i], 0);
 	return 0;
 }
 #endif /* DEBUG */
-- 
2.47.3

Re: [PATCH 05/10] xfs: use WRITE_ONCE/READ_ONCE for m_errortag

[Darrick J. Wong]

On Tue, Jan 27, 2026 at 05:05:45PM +0100, Christoph Hellwig wrote:
> There is no synchronization for updating m_errortag, which is fine as
> it's just a debug tool.  It would still be nice to fully avoid the
> theoretical case of torn values, so use WRITE_ONCE and READ_ONCE to
> access the members.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks fine,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_error.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index a6f160a4d0e9..53704f1ed791 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -50,17 +50,18 @@ xfs_errortag_attr_store(
>  {
>  	struct xfs_mount	*mp = to_mp(kobject);
>  	unsigned int		error_tag = to_attr(attr)->tag;
> +	unsigned int		val;
>  	int			ret;
>  
>  	if (strcmp(buf, "default") == 0) {
> -		mp->m_errortag[error_tag] =
> -			xfs_errortag_random_default[error_tag];
> +		val = xfs_errortag_random_default[error_tag];
>  	} else {
> -		ret = kstrtouint(buf, 0, &mp->m_errortag[error_tag]);
> +		ret = kstrtouint(buf, 0, &val);
>  		if (ret)
>  			return ret;
>  	}
>  
> +	WRITE_ONCE(mp->m_errortag[error_tag], val);
>  	return count;
>  }
>  
> @@ -71,9 +72,9 @@ xfs_errortag_attr_show(
>  	char			*buf)
>  {
>  	struct xfs_mount	*mp = to_mp(kobject);
> -	unsigned int		error_tag = to_attr(attr)->tag;
>  
> -	return snprintf(buf, PAGE_SIZE, "%u\n", mp->m_errortag[error_tag]);
> +	return snprintf(buf, PAGE_SIZE, "%u\n",
> +			READ_ONCE(mp->m_errortag[to_attr(attr)->tag]));
>  }
>  
>  static const struct sysfs_ops xfs_errortag_sysfs_ops = {
> @@ -134,7 +135,7 @@ xfs_errortag_test(
>  {
>  	unsigned int		randfactor;
>  
> -	randfactor = mp->m_errortag[error_tag];
> +	randfactor = READ_ONCE(mp->m_errortag[error_tag]);
>  	if (!randfactor || get_random_u32_below(randfactor))
>  		return false;
>  
> @@ -151,7 +152,7 @@ xfs_errortag_delay(
>  	int			line,
>  	unsigned int		error_tag)
>  {
> -	unsigned int		delay = mp->m_errortag[error_tag];
> +	unsigned int		delay = READ_ONCE(mp->m_errortag[error_tag]);
>  
>  	might_sleep();
>  
> @@ -183,7 +184,8 @@ xfs_errortag_add(
>  		break;
>  	}
>  
> -	mp->m_errortag[error_tag] = xfs_errortag_random_default[error_tag];
> +	WRITE_ONCE(mp->m_errortag[error_tag],
> +		   xfs_errortag_random_default[error_tag]);
>  	return 0;
>  }
>  
> @@ -191,7 +193,10 @@ int
>  xfs_errortag_clearall(
>  	struct xfs_mount	*mp)
>  {
> -	memset(mp->m_errortag, 0, sizeof(unsigned int) * XFS_ERRTAG_MAX);
> +	unsigned int		i;
> +
> +	for (i = 0; i < XFS_ERRTAG_MAX; i++)
> +		WRITE_ONCE(mp->m_errortag[i], 0);
>  	return 0;
>  }
>  #endif /* DEBUG */
> -- 
> 2.47.3
> 
> 

Re: [PATCH 05/10] xfs: use WRITE_ONCE/READ_ONCE for m_errortag

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:05:45PM +0100, Christoph Hellwig wrote:
> There is no synchronization for updating m_errortag, which is fine as
> it's just a debug tool.  It would still be nice to fully avoid the
> theoretical case of torn values, so use WRITE_ONCE and READ_ONCE to
> access the members.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/xfs_error.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index a6f160a4d0e9..53704f1ed791 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -50,17 +50,18 @@ xfs_errortag_attr_store(
>  {
>  	struct xfs_mount	*mp = to_mp(kobject);
>  	unsigned int		error_tag = to_attr(attr)->tag;
> +	unsigned int		val;
>  	int			ret;
>  
>  	if (strcmp(buf, "default") == 0) {
> -		mp->m_errortag[error_tag] =
> -			xfs_errortag_random_default[error_tag];
> +		val = xfs_errortag_random_default[error_tag];
>  	} else {
> -		ret = kstrtouint(buf, 0, &mp->m_errortag[error_tag]);
> +		ret = kstrtouint(buf, 0, &val);
>  		if (ret)
>  			return ret;
>  	}
>  
> +	WRITE_ONCE(mp->m_errortag[error_tag], val);
>  	return count;
>  }
>  
> @@ -71,9 +72,9 @@ xfs_errortag_attr_show(
>  	char			*buf)
>  {
>  	struct xfs_mount	*mp = to_mp(kobject);
> -	unsigned int		error_tag = to_attr(attr)->tag;
>  
> -	return snprintf(buf, PAGE_SIZE, "%u\n", mp->m_errortag[error_tag]);
> +	return snprintf(buf, PAGE_SIZE, "%u\n",
> +			READ_ONCE(mp->m_errortag[to_attr(attr)->tag]));
>  }
>  
>  static const struct sysfs_ops xfs_errortag_sysfs_ops = {
> @@ -134,7 +135,7 @@ xfs_errortag_test(
>  {
>  	unsigned int		randfactor;
>  
> -	randfactor = mp->m_errortag[error_tag];
> +	randfactor = READ_ONCE(mp->m_errortag[error_tag]);
>  	if (!randfactor || get_random_u32_below(randfactor))
>  		return false;
>  
> @@ -151,7 +152,7 @@ xfs_errortag_delay(
>  	int			line,
>  	unsigned int		error_tag)
>  {
> -	unsigned int		delay = mp->m_errortag[error_tag];
> +	unsigned int		delay = READ_ONCE(mp->m_errortag[error_tag]);
>  
>  	might_sleep();
>  
> @@ -183,7 +184,8 @@ xfs_errortag_add(
>  		break;
>  	}
>  
> -	mp->m_errortag[error_tag] = xfs_errortag_random_default[error_tag];
> +	WRITE_ONCE(mp->m_errortag[error_tag],
> +		   xfs_errortag_random_default[error_tag]);
>  	return 0;
>  }
>  
> @@ -191,7 +193,10 @@ int
>  xfs_errortag_clearall(
>  	struct xfs_mount	*mp)
>  {
> -	memset(mp->m_errortag, 0, sizeof(unsigned int) * XFS_ERRTAG_MAX);
> +	unsigned int		i;
> +
> +	for (i = 0; i < XFS_ERRTAG_MAX; i++)
> +		WRITE_ONCE(mp->m_errortag[i], 0);
>  	return 0;
>  }
>  #endif /* DEBUG */
> -- 
> 2.47.3
> 
> 

Re: [PATCH 05/10] xfs: use WRITE_ONCE/READ_ONCE for m_errortag

[Hans Holmberg]

On 27/01/2026 17:06, Christoph Hellwig wrote:
> There is no synchronization for updating m_errortag, which is fine as
> it's just a debug tool.  It would still be nice to fully avoid the
> theoretical case of torn values, so use WRITE_ONCE and READ_ONCE to
> access the members.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_error.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index a6f160a4d0e9..53704f1ed791 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -50,17 +50,18 @@ xfs_errortag_attr_store(
>  {
>  	struct xfs_mount	*mp = to_mp(kobject);
>  	unsigned int		error_tag = to_attr(attr)->tag;
> +	unsigned int		val;
>  	int			ret;
>  
>  	if (strcmp(buf, "default") == 0) {
> -		mp->m_errortag[error_tag] =
> -			xfs_errortag_random_default[error_tag];
> +		val = xfs_errortag_random_default[error_tag];
>  	} else {
> -		ret = kstrtouint(buf, 0, &mp->m_errortag[error_tag]);
> +		ret = kstrtouint(buf, 0, &val);
>  		if (ret)
>  			return ret;
>  	}
>  
> +	WRITE_ONCE(mp->m_errortag[error_tag], val);
>  	return count;
>  }
>  
> @@ -71,9 +72,9 @@ xfs_errortag_attr_show(
>  	char			*buf)
>  {
>  	struct xfs_mount	*mp = to_mp(kobject);
> -	unsigned int		error_tag = to_attr(attr)->tag;
>  
> -	return snprintf(buf, PAGE_SIZE, "%u\n", mp->m_errortag[error_tag]);
> +	return snprintf(buf, PAGE_SIZE, "%u\n",
> +			READ_ONCE(mp->m_errortag[to_attr(attr)->tag]));
>  }
>  
>  static const struct sysfs_ops xfs_errortag_sysfs_ops = {
> @@ -134,7 +135,7 @@ xfs_errortag_test(
>  {
>  	unsigned int		randfactor;
>  
> -	randfactor = mp->m_errortag[error_tag];
> +	randfactor = READ_ONCE(mp->m_errortag[error_tag]);
>  	if (!randfactor || get_random_u32_below(randfactor))
>  		return false;
>  
> @@ -151,7 +152,7 @@ xfs_errortag_delay(
>  	int			line,
>  	unsigned int		error_tag)
>  {
> -	unsigned int		delay = mp->m_errortag[error_tag];
> +	unsigned int		delay = READ_ONCE(mp->m_errortag[error_tag]);
>  
>  	might_sleep();
>  
> @@ -183,7 +184,8 @@ xfs_errortag_add(
>  		break;
>  	}
>  
> -	mp->m_errortag[error_tag] = xfs_errortag_random_default[error_tag];
> +	WRITE_ONCE(mp->m_errortag[error_tag],
> +		   xfs_errortag_random_default[error_tag]);
>  	return 0;
>  }
>  
> @@ -191,7 +193,10 @@ int
>  xfs_errortag_clearall(
>  	struct xfs_mount	*mp)
>  {
> -	memset(mp->m_errortag, 0, sizeof(unsigned int) * XFS_ERRTAG_MAX);
> +	unsigned int		i;
> +
> +	for (i = 0; i < XFS_ERRTAG_MAX; i++)
> +		WRITE_ONCE(mp->m_errortag[i], 0);
>  	return 0;
>  }
>  #endif /* DEBUG */


Looks good,

Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>

[PATCH 06/10] xfs: allow setting errortags at mount time

[Christoph Hellwig]

Add an errortag mount option that enables an errortag with the default
injection frequency.  This allows injecting errors into the mount
process instead of just on live file systems, and thus test mount
error handling.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/admin-guide/xfs.rst |  6 ++++++
 fs/xfs/xfs_error.c                | 36 +++++++++++++++++++++++++++++++
 fs/xfs/xfs_error.h                |  4 ++++
 fs/xfs/xfs_super.c                |  8 ++++++-
 4 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
index c85cd327af28..cb8cd12660d7 100644
--- a/Documentation/admin-guide/xfs.rst
+++ b/Documentation/admin-guide/xfs.rst
@@ -215,6 +215,12 @@ When mounting an XFS filesystem, the following options are accepted.
 	inconsistent namespace presentation during or after a
 	failover event.
 
+  errortag=tagname
+	When specified, enables the error inject tag named "tagname" with the
+	default frequency.  Can be specified multiple times to enable multiple
+	errortags.  Specifying this option on remount will reset the error tag
+	to the default value if it was set to any other value before.
+
 Deprecation of V4 Format
 ========================
 
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 53704f1ed791..d652240a1dca 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -22,6 +22,12 @@
 static const unsigned int xfs_errortag_random_default[] = { XFS_ERRTAGS };
 #undef XFS_ERRTAG
 
+#define XFS_ERRTAG(_tag, _name, _default) \
+        [XFS_ERRTAG_##_tag]	=  __stringify(_name),
+#include "xfs_errortag.h"
+static const char *xfs_errortag_names[] = { XFS_ERRTAGS };
+#undef XFS_ERRTAG
+
 struct xfs_errortag_attr {
 	struct attribute	attr;
 	unsigned int		tag;
@@ -189,6 +195,36 @@ xfs_errortag_add(
 	return 0;
 }
 
+int
+xfs_errortag_add_name(
+	struct xfs_mount	*mp,
+	const char		*tag_name)
+{
+	unsigned int		i;
+
+	for (i = 0; i < XFS_ERRTAG_MAX; i++) {
+		if (xfs_errortag_names[i] &&
+		    !strcmp(xfs_errortag_names[i], tag_name))
+			return xfs_errortag_add(mp, i);
+	}
+
+	return -EINVAL;
+}
+
+void
+xfs_errortag_copy(
+	struct xfs_mount	*dst_mp,
+	struct xfs_mount	*src_mp)
+{
+	unsigned int		val, i;
+
+	for (i = 0; i < XFS_ERRTAG_MAX; i++) {
+		val = READ_ONCE(src_mp->m_errortag[i]);
+		if (val)
+			WRITE_ONCE(dst_mp->m_errortag[i], val);
+	}
+}
+
 int
 xfs_errortag_clearall(
 	struct xfs_mount	*mp)
diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
index b40e7c671d2a..05fc1d1cf521 100644
--- a/fs/xfs/xfs_error.h
+++ b/fs/xfs/xfs_error.h
@@ -45,6 +45,8 @@ void xfs_errortag_delay(struct xfs_mount *mp, const char *file, int line,
 #define XFS_ERRORTAG_DELAY(mp, tag)		\
 	xfs_errortag_delay((mp), __FILE__, __LINE__, (tag))
 int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
+int xfs_errortag_add_name(struct xfs_mount *mp, const char *tag_name);
+void xfs_errortag_copy(struct xfs_mount *dst_mp, struct xfs_mount *src_mp);
 int xfs_errortag_clearall(struct xfs_mount *mp);
 #else
 #define xfs_errortag_init(mp)			(0)
@@ -52,6 +54,8 @@ int xfs_errortag_clearall(struct xfs_mount *mp);
 #define XFS_TEST_ERROR(mp, tag)			(false)
 #define XFS_ERRORTAG_DELAY(mp, tag)		((void)0)
 #define xfs_errortag_add(mp, tag)		(-ENOSYS)
+#define xfs_errortag_copy(dst_mp, src_mp)	((void)0)
+#define xfs_errortag_add_name(mp, tag_name)	(-ENOSYS)
 #define xfs_errortag_clearall(mp)		(-ENOSYS)
 #endif /* DEBUG */
 
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index ee335dbe5811..d5aec07c3a5b 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -40,6 +40,7 @@
 #include "xfs_defer.h"
 #include "xfs_attr_item.h"
 #include "xfs_xattr.h"
+#include "xfs_error.h"
 #include "xfs_errortag.h"
 #include "xfs_iunlink_item.h"
 #include "xfs_dahash_test.h"
@@ -112,7 +113,7 @@ enum {
 	Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
 	Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
 	Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum, Opt_max_open_zones,
-	Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write,
+	Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write, Opt_errortag,
 };
 
 #define fsparam_dead(NAME) \
@@ -171,6 +172,7 @@ static const struct fs_parameter_spec xfs_fs_parameters[] = {
 	fsparam_flag("lifetime",	Opt_lifetime),
 	fsparam_flag("nolifetime",	Opt_nolifetime),
 	fsparam_string("max_atomic_write",	Opt_max_atomic_write),
+	fsparam_string("errortag",	Opt_errortag),
 	{}
 };
 
@@ -1581,6 +1583,8 @@ xfs_fs_parse_param(
 			return -EINVAL;
 		}
 		return 0;
+	case Opt_errortag:
+		return xfs_errortag_add_name(parsing_mp, param->string);
 	default:
 		xfs_warn(parsing_mp, "unknown mount option [%s].", param->key);
 		return -EINVAL;
@@ -2172,6 +2176,8 @@ xfs_fs_reconfigure(
 	if (error)
 		return error;
 
+	xfs_errortag_copy(mp, new_mp);
+
 	/* Validate new max_atomic_write option before making other changes */
 	if (mp->m_awu_max_bytes != new_mp->m_awu_max_bytes) {
 		error = xfs_set_max_atomic_write_opt(mp,
-- 
2.47.3

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Darrick J. Wong]

On Tue, Jan 27, 2026 at 05:05:46PM +0100, Christoph Hellwig wrote:
> Add an errortag mount option that enables an errortag with the default
> injection frequency.  This allows injecting errors into the mount
> process instead of just on live file systems, and thus test mount
> error handling.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  Documentation/admin-guide/xfs.rst |  6 ++++++
>  fs/xfs/xfs_error.c                | 36 +++++++++++++++++++++++++++++++
>  fs/xfs/xfs_error.h                |  4 ++++
>  fs/xfs/xfs_super.c                |  8 ++++++-
>  4 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
> index c85cd327af28..cb8cd12660d7 100644
> --- a/Documentation/admin-guide/xfs.rst
> +++ b/Documentation/admin-guide/xfs.rst
> @@ -215,6 +215,12 @@ When mounting an XFS filesystem, the following options are accepted.
>  	inconsistent namespace presentation during or after a
>  	failover event.
>  
> +  errortag=tagname
> +	When specified, enables the error inject tag named "tagname" with the
> +	default frequency.  Can be specified multiple times to enable multiple
> +	errortags.  Specifying this option on remount will reset the error tag
> +	to the default value if it was set to any other value before.

Any interest in allowing people to specify the value too?  Seeing as we
allow that the sysfs version of the interface.

--D

> +
>  Deprecation of V4 Format
>  ========================
>  
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 53704f1ed791..d652240a1dca 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -22,6 +22,12 @@
>  static const unsigned int xfs_errortag_random_default[] = { XFS_ERRTAGS };
>  #undef XFS_ERRTAG
>  
> +#define XFS_ERRTAG(_tag, _name, _default) \
> +        [XFS_ERRTAG_##_tag]	=  __stringify(_name),
> +#include "xfs_errortag.h"
> +static const char *xfs_errortag_names[] = { XFS_ERRTAGS };
> +#undef XFS_ERRTAG
> +
>  struct xfs_errortag_attr {
>  	struct attribute	attr;
>  	unsigned int		tag;
> @@ -189,6 +195,36 @@ xfs_errortag_add(
>  	return 0;
>  }
>  
> +int
> +xfs_errortag_add_name(
> +	struct xfs_mount	*mp,
> +	const char		*tag_name)
> +{
> +	unsigned int		i;
> +
> +	for (i = 0; i < XFS_ERRTAG_MAX; i++) {
> +		if (xfs_errortag_names[i] &&
> +		    !strcmp(xfs_errortag_names[i], tag_name))
> +			return xfs_errortag_add(mp, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +void
> +xfs_errortag_copy(
> +	struct xfs_mount	*dst_mp,
> +	struct xfs_mount	*src_mp)
> +{
> +	unsigned int		val, i;
> +
> +	for (i = 0; i < XFS_ERRTAG_MAX; i++) {
> +		val = READ_ONCE(src_mp->m_errortag[i]);
> +		if (val)
> +			WRITE_ONCE(dst_mp->m_errortag[i], val);
> +	}
> +}
> +
>  int
>  xfs_errortag_clearall(
>  	struct xfs_mount	*mp)
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index b40e7c671d2a..05fc1d1cf521 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -45,6 +45,8 @@ void xfs_errortag_delay(struct xfs_mount *mp, const char *file, int line,
>  #define XFS_ERRORTAG_DELAY(mp, tag)		\
>  	xfs_errortag_delay((mp), __FILE__, __LINE__, (tag))
>  int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
> +int xfs_errortag_add_name(struct xfs_mount *mp, const char *tag_name);
> +void xfs_errortag_copy(struct xfs_mount *dst_mp, struct xfs_mount *src_mp);
>  int xfs_errortag_clearall(struct xfs_mount *mp);
>  #else
>  #define xfs_errortag_init(mp)			(0)
> @@ -52,6 +54,8 @@ int xfs_errortag_clearall(struct xfs_mount *mp);
>  #define XFS_TEST_ERROR(mp, tag)			(false)
>  #define XFS_ERRORTAG_DELAY(mp, tag)		((void)0)
>  #define xfs_errortag_add(mp, tag)		(-ENOSYS)
> +#define xfs_errortag_copy(dst_mp, src_mp)	((void)0)
> +#define xfs_errortag_add_name(mp, tag_name)	(-ENOSYS)
>  #define xfs_errortag_clearall(mp)		(-ENOSYS)
>  #endif /* DEBUG */
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index ee335dbe5811..d5aec07c3a5b 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -40,6 +40,7 @@
>  #include "xfs_defer.h"
>  #include "xfs_attr_item.h"
>  #include "xfs_xattr.h"
> +#include "xfs_error.h"
>  #include "xfs_errortag.h"
>  #include "xfs_iunlink_item.h"
>  #include "xfs_dahash_test.h"
> @@ -112,7 +113,7 @@ enum {
>  	Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
>  	Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
>  	Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum, Opt_max_open_zones,
> -	Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write,
> +	Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write, Opt_errortag,
>  };
>  
>  #define fsparam_dead(NAME) \
> @@ -171,6 +172,7 @@ static const struct fs_parameter_spec xfs_fs_parameters[] = {
>  	fsparam_flag("lifetime",	Opt_lifetime),
>  	fsparam_flag("nolifetime",	Opt_nolifetime),
>  	fsparam_string("max_atomic_write",	Opt_max_atomic_write),
> +	fsparam_string("errortag",	Opt_errortag),
>  	{}
>  };
>  
> @@ -1581,6 +1583,8 @@ xfs_fs_parse_param(
>  			return -EINVAL;
>  		}
>  		return 0;
> +	case Opt_errortag:
> +		return xfs_errortag_add_name(parsing_mp, param->string);
>  	default:
>  		xfs_warn(parsing_mp, "unknown mount option [%s].", param->key);
>  		return -EINVAL;
> @@ -2172,6 +2176,8 @@ xfs_fs_reconfigure(
>  	if (error)
>  		return error;
>  
> +	xfs_errortag_copy(mp, new_mp);
> +
>  	/* Validate new max_atomic_write option before making other changes */
>  	if (mp->m_awu_max_bytes != new_mp->m_awu_max_bytes) {
>  		error = xfs_set_max_atomic_write_opt(mp,
> -- 
> 2.47.3
> 
> 

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Christoph Hellwig]

On Tue, Jan 27, 2026 at 05:37:30PM -0800, Darrick J. Wong wrote:
> > +  errortag=tagname
> > +	When specified, enables the error inject tag named "tagname" with the
> > +	default frequency.  Can be specified multiple times to enable multiple
> > +	errortags.  Specifying this option on remount will reset the error tag
> > +	to the default value if it was set to any other value before.
> 
> Any interest in allowing people to specify the value too?  Seeing as we
> allow that the sysfs version of the interface.

I don't need it for my uses yet, but it would be nice in general.
The reason I didn't do it is because it adds complex non-standard
parsing, and I'd have to find a way to separate the tag and the value
that is not "=", which is already taken by the mount option parsing.

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Darrick J. Wong]

On Wed, Jan 28, 2026 at 04:45:57AM +0100, Christoph Hellwig wrote:
> On Tue, Jan 27, 2026 at 05:37:30PM -0800, Darrick J. Wong wrote:
> > > +  errortag=tagname
> > > +	When specified, enables the error inject tag named "tagname" with the
> > > +	default frequency.  Can be specified multiple times to enable multiple
> > > +	errortags.  Specifying this option on remount will reset the error tag
> > > +	to the default value if it was set to any other value before.
> > 
> > Any interest in allowing people to specify the value too?  Seeing as we
> > allow that the sysfs version of the interface.
> 
> I don't need it for my uses yet, but it would be nice in general.
> The reason I didn't do it is because it adds complex non-standard
> parsing, and I'd have to find a way to separate the tag and the value
> that is not "=", which is already taken by the mount option parsing.

Huh.  I could've sworn there was a way to do suboption parsing with the
new mount api, but maybe I hallucinated that.

--D

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Christoph Hellwig]

On Tue, Jan 27, 2026 at 09:07:10PM -0800, Darrick J. Wong wrote:
> On Wed, Jan 28, 2026 at 04:45:57AM +0100, Christoph Hellwig wrote:
> > On Tue, Jan 27, 2026 at 05:37:30PM -0800, Darrick J. Wong wrote:
> > > > +  errortag=tagname
> > > > +	When specified, enables the error inject tag named "tagname" with the
> > > > +	default frequency.  Can be specified multiple times to enable multiple
> > > > +	errortags.  Specifying this option on remount will reset the error tag
> > > > +	to the default value if it was set to any other value before.
> > > 
> > > Any interest in allowing people to specify the value too?  Seeing as we
> > > allow that the sysfs version of the interface.
> > 
> > I don't need it for my uses yet, but it would be nice in general.
> > The reason I didn't do it is because it adds complex non-standard
> > parsing, and I'd have to find a way to separate the tag and the value
> > that is not "=", which is already taken by the mount option parsing.
> 
> Huh.  I could've sworn there was a way to do suboption parsing with the
> new mount api, but maybe I hallucinated that.

Or I'm too stupid to find it.  I think for now the current version
does, and if needed we can extend it.

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:05:46PM +0100, Christoph Hellwig wrote:
> Add an errortag mount option that enables an errortag with the default
> injection frequency.  This allows injecting errors into the mount
> process instead of just on live file systems, and thus test mount
> error handling.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  Documentation/admin-guide/xfs.rst |  6 ++++++
>  fs/xfs/xfs_error.c                | 36 +++++++++++++++++++++++++++++++
>  fs/xfs/xfs_error.h                |  4 ++++
>  fs/xfs/xfs_super.c                |  8 ++++++-
>  4 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
> index c85cd327af28..cb8cd12660d7 100644
> --- a/Documentation/admin-guide/xfs.rst
> +++ b/Documentation/admin-guide/xfs.rst
> @@ -215,6 +215,12 @@ When mounting an XFS filesystem, the following options are accepted.
>  	inconsistent namespace presentation during or after a
>  	failover event.
>  
> +  errortag=tagname
> +	When specified, enables the error inject tag named "tagname" with the
> +	default frequency.  Can be specified multiple times to enable multiple
> +	errortags.  Specifying this option on remount will reset the error tag
> +	to the default value if it was set to any other value before.
> +

I think this should include the fact it is only supported with DEBUG
enabled.
I'm sure we'll get user complains about why 'errortag=foo' is not working?
And, in some unfortunate case somebody has DEBUG enabled when they
shouldn't have, at least the documentation says so this shouldn't be
used...

I'm happy taking this patch though and we work on the above later, so...
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

>  Deprecation of V4 Format
>  ========================
>  
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 53704f1ed791..d652240a1dca 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -22,6 +22,12 @@
>  static const unsigned int xfs_errortag_random_default[] = { XFS_ERRTAGS };
>  #undef XFS_ERRTAG
>  
> +#define XFS_ERRTAG(_tag, _name, _default) \
> +        [XFS_ERRTAG_##_tag]	=  __stringify(_name),
> +#include "xfs_errortag.h"
> +static const char *xfs_errortag_names[] = { XFS_ERRTAGS };
> +#undef XFS_ERRTAG
> +
>  struct xfs_errortag_attr {
>  	struct attribute	attr;
>  	unsigned int		tag;
> @@ -189,6 +195,36 @@ xfs_errortag_add(
>  	return 0;
>  }
>  
> +int
> +xfs_errortag_add_name(
> +	struct xfs_mount	*mp,
> +	const char		*tag_name)
> +{
> +	unsigned int		i;
> +
> +	for (i = 0; i < XFS_ERRTAG_MAX; i++) {
> +		if (xfs_errortag_names[i] &&
> +		    !strcmp(xfs_errortag_names[i], tag_name))
> +			return xfs_errortag_add(mp, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +void
> +xfs_errortag_copy(
> +	struct xfs_mount	*dst_mp,
> +	struct xfs_mount	*src_mp)
> +{
> +	unsigned int		val, i;
> +
> +	for (i = 0; i < XFS_ERRTAG_MAX; i++) {
> +		val = READ_ONCE(src_mp->m_errortag[i]);
> +		if (val)
> +			WRITE_ONCE(dst_mp->m_errortag[i], val);
> +	}
> +}
> +
>  int
>  xfs_errortag_clearall(
>  	struct xfs_mount	*mp)
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index b40e7c671d2a..05fc1d1cf521 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -45,6 +45,8 @@ void xfs_errortag_delay(struct xfs_mount *mp, const char *file, int line,
>  #define XFS_ERRORTAG_DELAY(mp, tag)		\
>  	xfs_errortag_delay((mp), __FILE__, __LINE__, (tag))
>  int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
> +int xfs_errortag_add_name(struct xfs_mount *mp, const char *tag_name);
> +void xfs_errortag_copy(struct xfs_mount *dst_mp, struct xfs_mount *src_mp);
>  int xfs_errortag_clearall(struct xfs_mount *mp);
>  #else
>  #define xfs_errortag_init(mp)			(0)
> @@ -52,6 +54,8 @@ int xfs_errortag_clearall(struct xfs_mount *mp);
>  #define XFS_TEST_ERROR(mp, tag)			(false)
>  #define XFS_ERRORTAG_DELAY(mp, tag)		((void)0)
>  #define xfs_errortag_add(mp, tag)		(-ENOSYS)
> +#define xfs_errortag_copy(dst_mp, src_mp)	((void)0)
> +#define xfs_errortag_add_name(mp, tag_name)	(-ENOSYS)
>  #define xfs_errortag_clearall(mp)		(-ENOSYS)
>  #endif /* DEBUG */
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index ee335dbe5811..d5aec07c3a5b 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -40,6 +40,7 @@
>  #include "xfs_defer.h"
>  #include "xfs_attr_item.h"
>  #include "xfs_xattr.h"
> +#include "xfs_error.h"
>  #include "xfs_errortag.h"
>  #include "xfs_iunlink_item.h"
>  #include "xfs_dahash_test.h"
> @@ -112,7 +113,7 @@ enum {
>  	Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
>  	Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
>  	Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum, Opt_max_open_zones,
> -	Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write,
> +	Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write, Opt_errortag,
>  };
>  
>  #define fsparam_dead(NAME) \
> @@ -171,6 +172,7 @@ static const struct fs_parameter_spec xfs_fs_parameters[] = {
>  	fsparam_flag("lifetime",	Opt_lifetime),
>  	fsparam_flag("nolifetime",	Opt_nolifetime),
>  	fsparam_string("max_atomic_write",	Opt_max_atomic_write),
> +	fsparam_string("errortag",	Opt_errortag),
>  	{}
>  };
>  
> @@ -1581,6 +1583,8 @@ xfs_fs_parse_param(
>  			return -EINVAL;
>  		}
>  		return 0;
> +	case Opt_errortag:
> +		return xfs_errortag_add_name(parsing_mp, param->string);
>  	default:
>  		xfs_warn(parsing_mp, "unknown mount option [%s].", param->key);
>  		return -EINVAL;
> @@ -2172,6 +2176,8 @@ xfs_fs_reconfigure(
>  	if (error)
>  		return error;
>  
> +	xfs_errortag_copy(mp, new_mp);
> +
>  	/* Validate new max_atomic_write option before making other changes */
>  	if (mp->m_awu_max_bytes != new_mp->m_awu_max_bytes) {
>  		error = xfs_set_max_atomic_write_opt(mp,
> -- 
> 2.47.3
> 
> 

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Christoph Hellwig]

On Wed, Jan 28, 2026 at 12:30:05PM +0100, Carlos Maiolino wrote:
> I think this should include the fact it is only supported with DEBUG
> enabled.
> I'm sure we'll get user complains about why 'errortag=foo' is not working?
> And, in some unfortunate case somebody has DEBUG enabled when they
> shouldn't have, at least the documentation says so this shouldn't be
> used...

Yeah, there should be blurb about that.  Right now for !DEBUG it
will return an error, but even that needs an explanation.

> 

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Darrick J. Wong]

On Wed, Jan 28, 2026 at 12:30:05PM +0100, Carlos Maiolino wrote:
> On Tue, Jan 27, 2026 at 05:05:46PM +0100, Christoph Hellwig wrote:
> > Add an errortag mount option that enables an errortag with the default
> > injection frequency.  This allows injecting errors into the mount
> > process instead of just on live file systems, and thus test mount
> > error handling.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  Documentation/admin-guide/xfs.rst |  6 ++++++
> >  fs/xfs/xfs_error.c                | 36 +++++++++++++++++++++++++++++++
> >  fs/xfs/xfs_error.h                |  4 ++++
> >  fs/xfs/xfs_super.c                |  8 ++++++-
> >  4 files changed, 53 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
> > index c85cd327af28..cb8cd12660d7 100644
> > --- a/Documentation/admin-guide/xfs.rst
> > +++ b/Documentation/admin-guide/xfs.rst
> > @@ -215,6 +215,12 @@ When mounting an XFS filesystem, the following options are accepted.
> >  	inconsistent namespace presentation during or after a
> >  	failover event.
> >  
> > +  errortag=tagname
> > +	When specified, enables the error inject tag named "tagname" with the
> > +	default frequency.  Can be specified multiple times to enable multiple
> > +	errortags.  Specifying this option on remount will reset the error tag
> > +	to the default value if it was set to any other value before.
> > +
> 
> I think this should include the fact it is only supported with DEBUG
> enabled.
> I'm sure we'll get user complains about why 'errortag=foo' is not working?
> And, in some unfortunate case somebody has DEBUG enabled when they
> shouldn't have, at least the documentation says so this shouldn't be
> used...

Should we explicitly state here that the errortag=XXX will /not/ be
echoed back via /proc/mounts?  Seeing as we recently had bug reports
about scripts encoding /proc/mounts into /etc/fstab.

(That's why I hate mount options)

--D

> I'm happy taking this patch though and we work on the above later, so...
> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> 
> >  Deprecation of V4 Format
> >  ========================
> >  
> > diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> > index 53704f1ed791..d652240a1dca 100644
> > --- a/fs/xfs/xfs_error.c
> > +++ b/fs/xfs/xfs_error.c
> > @@ -22,6 +22,12 @@
> >  static const unsigned int xfs_errortag_random_default[] = { XFS_ERRTAGS };
> >  #undef XFS_ERRTAG
> >  
> > +#define XFS_ERRTAG(_tag, _name, _default) \
> > +        [XFS_ERRTAG_##_tag]	=  __stringify(_name),
> > +#include "xfs_errortag.h"
> > +static const char *xfs_errortag_names[] = { XFS_ERRTAGS };
> > +#undef XFS_ERRTAG
> > +
> >  struct xfs_errortag_attr {
> >  	struct attribute	attr;
> >  	unsigned int		tag;
> > @@ -189,6 +195,36 @@ xfs_errortag_add(
> >  	return 0;
> >  }
> >  
> > +int
> > +xfs_errortag_add_name(
> > +	struct xfs_mount	*mp,
> > +	const char		*tag_name)
> > +{
> > +	unsigned int		i;
> > +
> > +	for (i = 0; i < XFS_ERRTAG_MAX; i++) {
> > +		if (xfs_errortag_names[i] &&
> > +		    !strcmp(xfs_errortag_names[i], tag_name))
> > +			return xfs_errortag_add(mp, i);
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +void
> > +xfs_errortag_copy(
> > +	struct xfs_mount	*dst_mp,
> > +	struct xfs_mount	*src_mp)
> > +{
> > +	unsigned int		val, i;
> > +
> > +	for (i = 0; i < XFS_ERRTAG_MAX; i++) {
> > +		val = READ_ONCE(src_mp->m_errortag[i]);
> > +		if (val)
> > +			WRITE_ONCE(dst_mp->m_errortag[i], val);
> > +	}
> > +}
> > +
> >  int
> >  xfs_errortag_clearall(
> >  	struct xfs_mount	*mp)
> > diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> > index b40e7c671d2a..05fc1d1cf521 100644
> > --- a/fs/xfs/xfs_error.h
> > +++ b/fs/xfs/xfs_error.h
> > @@ -45,6 +45,8 @@ void xfs_errortag_delay(struct xfs_mount *mp, const char *file, int line,
> >  #define XFS_ERRORTAG_DELAY(mp, tag)		\
> >  	xfs_errortag_delay((mp), __FILE__, __LINE__, (tag))
> >  int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
> > +int xfs_errortag_add_name(struct xfs_mount *mp, const char *tag_name);
> > +void xfs_errortag_copy(struct xfs_mount *dst_mp, struct xfs_mount *src_mp);
> >  int xfs_errortag_clearall(struct xfs_mount *mp);
> >  #else
> >  #define xfs_errortag_init(mp)			(0)
> > @@ -52,6 +54,8 @@ int xfs_errortag_clearall(struct xfs_mount *mp);
> >  #define XFS_TEST_ERROR(mp, tag)			(false)
> >  #define XFS_ERRORTAG_DELAY(mp, tag)		((void)0)
> >  #define xfs_errortag_add(mp, tag)		(-ENOSYS)
> > +#define xfs_errortag_copy(dst_mp, src_mp)	((void)0)
> > +#define xfs_errortag_add_name(mp, tag_name)	(-ENOSYS)
> >  #define xfs_errortag_clearall(mp)		(-ENOSYS)
> >  #endif /* DEBUG */
> >  
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index ee335dbe5811..d5aec07c3a5b 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -40,6 +40,7 @@
> >  #include "xfs_defer.h"
> >  #include "xfs_attr_item.h"
> >  #include "xfs_xattr.h"
> > +#include "xfs_error.h"
> >  #include "xfs_errortag.h"
> >  #include "xfs_iunlink_item.h"
> >  #include "xfs_dahash_test.h"
> > @@ -112,7 +113,7 @@ enum {
> >  	Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
> >  	Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
> >  	Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum, Opt_max_open_zones,
> > -	Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write,
> > +	Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write, Opt_errortag,
> >  };
> >  
> >  #define fsparam_dead(NAME) \
> > @@ -171,6 +172,7 @@ static const struct fs_parameter_spec xfs_fs_parameters[] = {
> >  	fsparam_flag("lifetime",	Opt_lifetime),
> >  	fsparam_flag("nolifetime",	Opt_nolifetime),
> >  	fsparam_string("max_atomic_write",	Opt_max_atomic_write),
> > +	fsparam_string("errortag",	Opt_errortag),
> >  	{}
> >  };
> >  
> > @@ -1581,6 +1583,8 @@ xfs_fs_parse_param(
> >  			return -EINVAL;
> >  		}
> >  		return 0;
> > +	case Opt_errortag:
> > +		return xfs_errortag_add_name(parsing_mp, param->string);
> >  	default:
> >  		xfs_warn(parsing_mp, "unknown mount option [%s].", param->key);
> >  		return -EINVAL;
> > @@ -2172,6 +2176,8 @@ xfs_fs_reconfigure(
> >  	if (error)
> >  		return error;
> >  
> > +	xfs_errortag_copy(mp, new_mp);
> > +
> >  	/* Validate new max_atomic_write option before making other changes */
> >  	if (mp->m_awu_max_bytes != new_mp->m_awu_max_bytes) {
> >  		error = xfs_set_max_atomic_write_opt(mp,
> > -- 
> > 2.47.3
> > 
> > 
> 

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Christoph Hellwig]

On Wed, Jan 28, 2026 at 08:11:42AM -0800, Darrick J. Wong wrote:
> Should we explicitly state here that the errortag=XXX will /not/ be
> echoed back via /proc/mounts?

Sure, I'll add that.

> Seeing as we recently had bug reports
> about scripts encoding /proc/mounts into /etc/fstab.

WTF?

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Darrick J. Wong]

On Wed, Jan 28, 2026 at 05:13:51PM +0100, Christoph Hellwig wrote:
> On Wed, Jan 28, 2026 at 08:11:42AM -0800, Darrick J. Wong wrote:
> > Should we explicitly state here that the errortag=XXX will /not/ be
> > echoed back via /proc/mounts?
> 
> Sure, I'll add that.
> 
> > Seeing as we recently had bug reports
> > about scripts encoding /proc/mounts into /etc/fstab.
> 
> WTF?

I tried to remove ikeep/noikeep/noattr2/attr2 and someone complained
that we broke his initramfs because Gentoo or whatever has this dumb
script that "generates" rootflags= (and maybe fstab too) from
/proc/mounts.

--D

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Carlos Maiolino]

On Wed, Jan 28, 2026 at 08:16:41AM -0800, Darrick J. Wong wrote:
> On Wed, Jan 28, 2026 at 05:13:51PM +0100, Christoph Hellwig wrote:
> > On Wed, Jan 28, 2026 at 08:11:42AM -0800, Darrick J. Wong wrote:
> > > Should we explicitly state here that the errortag=XXX will /not/ be
> > > echoed back via /proc/mounts?
> > 
> > Sure, I'll add that.
> > 
> > > Seeing as we recently had bug reports
> > > about scripts encoding /proc/mounts into /etc/fstab.
> > 
> > WTF?
> 
> I tried to remove ikeep/noikeep/noattr2/attr2 and someone complained
> that we broke his initramfs because Gentoo or whatever has this dumb
> script that "generates" rootflags= (and maybe fstab too) from
> /proc/mounts.

Ooohhhh, that thing...

For context:

https://lore.kernel.org/linux-xfs/176107134044.4152072.18403833729642060548.stgit@frogsfrogsfrogs/

> 
> --D
> 

Re: [PATCH 06/10] xfs: allow setting errortags at mount time

[Hans Holmberg]

On 27/01/2026 17:07, Christoph Hellwig wrote:
> Add an errortag mount option that enables an errortag with the default
> injection frequency.  This allows injecting errors into the mount
> process instead of just on live file systems, and thus test mount
> error handling.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  Documentation/admin-guide/xfs.rst |  6 ++++++
>  fs/xfs/xfs_error.c                | 36 +++++++++++++++++++++++++++++++
>  fs/xfs/xfs_error.h                |  4 ++++
>  fs/xfs/xfs_super.c                |  8 ++++++-
>  4 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
> index c85cd327af28..cb8cd12660d7 100644
> --- a/Documentation/admin-guide/xfs.rst
> +++ b/Documentation/admin-guide/xfs.rst
> @@ -215,6 +215,12 @@ When mounting an XFS filesystem, the following options are accepted.
>  	inconsistent namespace presentation during or after a
>  	failover event.
>  
> +  errortag=tagname
> +	When specified, enables the error inject tag named "tagname" with the
> +	default frequency.  Can be specified multiple times to enable multiple
> +	errortags.  Specifying this option on remount will reset the error tag
> +	to the default value if it was set to any other value before.
> +
>  Deprecation of V4 Format
>  ========================
>  
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 53704f1ed791..d652240a1dca 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -22,6 +22,12 @@
>  static const unsigned int xfs_errortag_random_default[] = { XFS_ERRTAGS };
>  #undef XFS_ERRTAG
>  
> +#define XFS_ERRTAG(_tag, _name, _default) \
> +        [XFS_ERRTAG_##_tag]	=  __stringify(_name),
> +#include "xfs_errortag.h"
> +static const char *xfs_errortag_names[] = { XFS_ERRTAGS };
> +#undef XFS_ERRTAG
> +
>  struct xfs_errortag_attr {
>  	struct attribute	attr;
>  	unsigned int		tag;
> @@ -189,6 +195,36 @@ xfs_errortag_add(
>  	return 0;
>  }
>  
> +int
> +xfs_errortag_add_name(
> +	struct xfs_mount	*mp,
> +	const char		*tag_name)
> +{
> +	unsigned int		i;
> +
> +	for (i = 0; i < XFS_ERRTAG_MAX; i++) {
> +		if (xfs_errortag_names[i] &&
> +		    !strcmp(xfs_errortag_names[i], tag_name))
> +			return xfs_errortag_add(mp, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +void
> +xfs_errortag_copy(
> +	struct xfs_mount	*dst_mp,
> +	struct xfs_mount	*src_mp)
> +{
> +	unsigned int		val, i;
> +
> +	for (i = 0; i < XFS_ERRTAG_MAX; i++) {
> +		val = READ_ONCE(src_mp->m_errortag[i]);
> +		if (val)
> +			WRITE_ONCE(dst_mp->m_errortag[i], val);
> +	}
> +}
> +
>  int
>  xfs_errortag_clearall(
>  	struct xfs_mount	*mp)
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index b40e7c671d2a..05fc1d1cf521 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -45,6 +45,8 @@ void xfs_errortag_delay(struct xfs_mount *mp, const char *file, int line,
>  #define XFS_ERRORTAG_DELAY(mp, tag)		\
>  	xfs_errortag_delay((mp), __FILE__, __LINE__, (tag))
>  int xfs_errortag_add(struct xfs_mount *mp, unsigned int error_tag);
> +int xfs_errortag_add_name(struct xfs_mount *mp, const char *tag_name);
> +void xfs_errortag_copy(struct xfs_mount *dst_mp, struct xfs_mount *src_mp);
>  int xfs_errortag_clearall(struct xfs_mount *mp);
>  #else
>  #define xfs_errortag_init(mp)			(0)
> @@ -52,6 +54,8 @@ int xfs_errortag_clearall(struct xfs_mount *mp);
>  #define XFS_TEST_ERROR(mp, tag)			(false)
>  #define XFS_ERRORTAG_DELAY(mp, tag)		((void)0)
>  #define xfs_errortag_add(mp, tag)		(-ENOSYS)
> +#define xfs_errortag_copy(dst_mp, src_mp)	((void)0)
> +#define xfs_errortag_add_name(mp, tag_name)	(-ENOSYS)
>  #define xfs_errortag_clearall(mp)		(-ENOSYS)
>  #endif /* DEBUG */
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index ee335dbe5811..d5aec07c3a5b 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -40,6 +40,7 @@
>  #include "xfs_defer.h"
>  #include "xfs_attr_item.h"
>  #include "xfs_xattr.h"
> +#include "xfs_error.h"
>  #include "xfs_errortag.h"
>  #include "xfs_iunlink_item.h"
>  #include "xfs_dahash_test.h"
> @@ -112,7 +113,7 @@ enum {
>  	Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
>  	Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
>  	Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum, Opt_max_open_zones,
> -	Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write,
> +	Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write, Opt_errortag,
>  };
>  
>  #define fsparam_dead(NAME) \
> @@ -171,6 +172,7 @@ static const struct fs_parameter_spec xfs_fs_parameters[] = {
>  	fsparam_flag("lifetime",	Opt_lifetime),
>  	fsparam_flag("nolifetime",	Opt_nolifetime),
>  	fsparam_string("max_atomic_write",	Opt_max_atomic_write),
> +	fsparam_string("errortag",	Opt_errortag),
>  	{}
>  };
>  
> @@ -1581,6 +1583,8 @@ xfs_fs_parse_param(
>  			return -EINVAL;
>  		}
>  		return 0;
> +	case Opt_errortag:
> +		return xfs_errortag_add_name(parsing_mp, param->string);
>  	default:
>  		xfs_warn(parsing_mp, "unknown mount option [%s].", param->key);
>  		return -EINVAL;
> @@ -2172,6 +2176,8 @@ xfs_fs_reconfigure(
>  	if (error)
>  		return error;
>  
> +	xfs_errortag_copy(mp, new_mp);
> +
>  	/* Validate new max_atomic_write option before making other changes */
>  	if (mp->m_awu_max_bytes != new_mp->m_awu_max_bytes) {
>  		error = xfs_set_max_atomic_write_opt(mp,


Looks good,

Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>

[PATCH 07/10] xfs: don't mark all discard issued by zoned GC as sync

[Christoph Hellwig]

Discard are not usually sync when issued from zoned garbage collection,
so drop the REQ_SYNC flag.

Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_zone_gc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 7bdc5043cc1a..60964c926f9f 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -905,7 +905,8 @@ xfs_zone_gc_prepare_reset(
 	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
 		if (!bdev_max_discard_sectors(bio->bi_bdev))
 			return false;
-		bio->bi_opf = REQ_OP_DISCARD | REQ_SYNC;
+		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
+		bio->bi_opf |= REQ_OP_DISCARD;
 		bio->bi_iter.bi_size =
 			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
 	}
-- 
2.47.3

Re: [PATCH 07/10] xfs: don't mark all discard issued by zoned GC as sync

[Darrick J. Wong]

On Tue, Jan 27, 2026 at 05:05:47PM +0100, Christoph Hellwig wrote:
> Discard are not usually sync when issued from zoned garbage collection,
> so drop the REQ_SYNC flag.

Cc: <stable@vger.kernel.org> # v6.15

> Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
> Signed-off-by: Christoph Hellwig <hch@lst.de>

What does REQ_SYNC even mean for a discard, anyway?

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_zone_gc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 7bdc5043cc1a..60964c926f9f 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -905,7 +905,8 @@ xfs_zone_gc_prepare_reset(
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
>  		if (!bdev_max_discard_sectors(bio->bi_bdev))
>  			return false;
> -		bio->bi_opf = REQ_OP_DISCARD | REQ_SYNC;
> +		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
> +		bio->bi_opf |= REQ_OP_DISCARD;
>  		bio->bi_iter.bi_size =
>  			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
>  	}
> -- 
> 2.47.3
> 
> 

Re: [PATCH 07/10] xfs: don't mark all discard issued by zoned GC as sync

[Christoph Hellwig]

On Tue, Jan 27, 2026 at 05:38:18PM -0800, Darrick J. Wong wrote:
> On Tue, Jan 27, 2026 at 05:05:47PM +0100, Christoph Hellwig wrote:
> > Discard are not usually sync when issued from zoned garbage collection,
> > so drop the REQ_SYNC flag.
> 
> Cc: <stable@vger.kernel.org> # v6.15
> 
> > Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> What does REQ_SYNC even mean for a discard, anyway?

It expedites a few things in the block submission path basically.
And deep magic in the I/O schedulers, but I'm not sure if that
even applies to discard.

Re: [PATCH 07/10] xfs: don't mark all discard issued by zoned GC as sync

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:05:47PM +0100, Christoph Hellwig wrote:
> Discard are not usually sync when issued from zoned garbage collection,
> so drop the REQ_SYNC flag.
> 

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_zone_gc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 7bdc5043cc1a..60964c926f9f 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -905,7 +905,8 @@ xfs_zone_gc_prepare_reset(
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
>  		if (!bdev_max_discard_sectors(bio->bi_bdev))
>  			return false;
> -		bio->bi_opf = REQ_OP_DISCARD | REQ_SYNC;
> +		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
> +		bio->bi_opf |= REQ_OP_DISCARD;
>  		bio->bi_iter.bi_size =
>  			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
>  	}
> -- 
> 2.47.3
> 
> 

Re: [PATCH 07/10] xfs: don't mark all discard issued by zoned GC as sync

[Hans Holmberg]

On 27/01/2026 17:07, Christoph Hellwig wrote:
> Discard are not usually sync when issued from zoned garbage collection,
> so drop the REQ_SYNC flag.
> 
> Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_zone_gc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 7bdc5043cc1a..60964c926f9f 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -905,7 +905,8 @@ xfs_zone_gc_prepare_reset(
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
>  		if (!bdev_max_discard_sectors(bio->bi_bdev))
>  			return false;
> -		bio->bi_opf = REQ_OP_DISCARD | REQ_SYNC;
> +		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
> +		bio->bi_opf |= REQ_OP_DISCARD;
>  		bio->bi_iter.bi_size =
>  			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
>  	}


Looks good,

Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>

[PATCH 08/10] xfs: refactor zone reset handling

[Christoph Hellwig]

Include the actual bio submission in the common zone reset handler to
share more code and prepare for adding error injection for zone reset.

Note the I plan to refactor the block layer submit_bio_wait and
bio_await_chain code in the next merge window to remove some of the
code duplication added here.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_zone_gc.c | 49 +++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 60964c926f9f..4023448e85d1 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -893,40 +893,55 @@ xfs_zone_gc_finish_reset(
 	bio_put(&chunk->bio);
 }
 
-static bool
-xfs_zone_gc_prepare_reset(
-	struct bio		*bio,
-	struct xfs_rtgroup	*rtg)
+static void
+xfs_submit_zone_reset_bio(
+	struct xfs_rtgroup	*rtg,
+	struct bio		*bio)
 {
 	trace_xfs_zone_reset(rtg);
 
 	ASSERT(rtg_rmap(rtg)->i_used_blocks == 0);
 	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
 	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
-		if (!bdev_max_discard_sectors(bio->bi_bdev))
-			return false;
+		/*
+		 * Also use the bio to drive the state machine when neither
+		 * zone reset nor discard is supported to keep things simple.
+		 */
+		if (!bdev_max_discard_sectors(bio->bi_bdev)) {
+			bio_endio(bio);
+			return;
+		}
 		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
 		bio->bi_opf |= REQ_OP_DISCARD;
 		bio->bi_iter.bi_size =
 			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
 	}
 
-	return true;
+	submit_bio(bio);
+}
+
+static void xfs_bio_wait_endio(struct bio *bio)
+{
+	complete(bio->bi_private);
 }
 
 int
 xfs_zone_gc_reset_sync(
 	struct xfs_rtgroup	*rtg)
 {
-	int			error = 0;
+	DECLARE_COMPLETION_ONSTACK(done);
 	struct bio		bio;
+	int			error;
 
 	bio_init(&bio, rtg_mount(rtg)->m_rtdev_targp->bt_bdev, NULL, 0,
-			REQ_OP_ZONE_RESET);
-	if (xfs_zone_gc_prepare_reset(&bio, rtg))
-		error = submit_bio_wait(&bio);
-	bio_uninit(&bio);
+			REQ_OP_ZONE_RESET | REQ_SYNC);
+	bio.bi_private = &done;
+	bio.bi_end_io = xfs_bio_wait_endio;
+	xfs_submit_zone_reset_bio(rtg, &bio);
+	wait_for_completion_io(&done);
 
+	error = blk_status_to_errno(bio.bi_status);
+	bio_uninit(&bio);
 	return error;
 }
 
@@ -961,15 +976,7 @@ xfs_zone_gc_reset_zones(
 		chunk->data = data;
 		WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
 		list_add_tail(&chunk->entry, &data->resetting);
-
-		/*
-		 * Also use the bio to drive the state machine when neither
-		 * zone reset nor discard is supported to keep things simple.
-		 */
-		if (xfs_zone_gc_prepare_reset(bio, rtg))
-			submit_bio(bio);
-		else
-			bio_endio(bio);
+		xfs_submit_zone_reset_bio(rtg, bio);
 	} while (next);
 }
 
-- 
2.47.3

Re: [PATCH 08/10] xfs: refactor zone reset handling

[Darrick J. Wong]

On Tue, Jan 27, 2026 at 05:05:48PM +0100, Christoph Hellwig wrote:
> Include the actual bio submission in the common zone reset handler to
> share more code and prepare for adding error injection for zone reset.
> 
> Note the I plan to refactor the block layer submit_bio_wait and
> bio_await_chain code in the next merge window to remove some of the
> code duplication added here.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Seems fine to me...
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_zone_gc.c | 49 +++++++++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 60964c926f9f..4023448e85d1 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -893,40 +893,55 @@ xfs_zone_gc_finish_reset(
>  	bio_put(&chunk->bio);
>  }
>  
> -static bool
> -xfs_zone_gc_prepare_reset(
> -	struct bio		*bio,
> -	struct xfs_rtgroup	*rtg)
> +static void
> +xfs_submit_zone_reset_bio(
> +	struct xfs_rtgroup	*rtg,
> +	struct bio		*bio)
>  {
>  	trace_xfs_zone_reset(rtg);
>  
>  	ASSERT(rtg_rmap(rtg)->i_used_blocks == 0);
>  	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
> -		if (!bdev_max_discard_sectors(bio->bi_bdev))
> -			return false;
> +		/*
> +		 * Also use the bio to drive the state machine when neither
> +		 * zone reset nor discard is supported to keep things simple.
> +		 */
> +		if (!bdev_max_discard_sectors(bio->bi_bdev)) {
> +			bio_endio(bio);
> +			return;
> +		}
>  		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
>  		bio->bi_opf |= REQ_OP_DISCARD;
>  		bio->bi_iter.bi_size =
>  			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
>  	}
>  
> -	return true;
> +	submit_bio(bio);
> +}
> +
> +static void xfs_bio_wait_endio(struct bio *bio)
> +{
> +	complete(bio->bi_private);
>  }
>  
>  int
>  xfs_zone_gc_reset_sync(
>  	struct xfs_rtgroup	*rtg)
>  {
> -	int			error = 0;
> +	DECLARE_COMPLETION_ONSTACK(done);
>  	struct bio		bio;
> +	int			error;
>  
>  	bio_init(&bio, rtg_mount(rtg)->m_rtdev_targp->bt_bdev, NULL, 0,
> -			REQ_OP_ZONE_RESET);
> -	if (xfs_zone_gc_prepare_reset(&bio, rtg))
> -		error = submit_bio_wait(&bio);
> -	bio_uninit(&bio);
> +			REQ_OP_ZONE_RESET | REQ_SYNC);
> +	bio.bi_private = &done;
> +	bio.bi_end_io = xfs_bio_wait_endio;
> +	xfs_submit_zone_reset_bio(rtg, &bio);
> +	wait_for_completion_io(&done);
>  
> +	error = blk_status_to_errno(bio.bi_status);
> +	bio_uninit(&bio);
>  	return error;
>  }
>  
> @@ -961,15 +976,7 @@ xfs_zone_gc_reset_zones(
>  		chunk->data = data;
>  		WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
>  		list_add_tail(&chunk->entry, &data->resetting);
> -
> -		/*
> -		 * Also use the bio to drive the state machine when neither
> -		 * zone reset nor discard is supported to keep things simple.
> -		 */
> -		if (xfs_zone_gc_prepare_reset(bio, rtg))
> -			submit_bio(bio);
> -		else
> -			bio_endio(bio);
> +		xfs_submit_zone_reset_bio(rtg, bio);
>  	} while (next);
>  }
>  
> -- 
> 2.47.3
> 
> 

Re: [PATCH 08/10] xfs: refactor zone reset handling

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:05:48PM +0100, Christoph Hellwig wrote:
> Include the actual bio submission in the common zone reset handler to
> share more code and prepare for adding error injection for zone reset.
> 
> Note the I plan to refactor the block layer submit_bio_wait and
> bio_await_chain code in the next merge window to remove some of the
> code duplication added here.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>


> ---
>  fs/xfs/xfs_zone_gc.c | 49 +++++++++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 60964c926f9f..4023448e85d1 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -893,40 +893,55 @@ xfs_zone_gc_finish_reset(
>  	bio_put(&chunk->bio);
>  }
>  
> -static bool
> -xfs_zone_gc_prepare_reset(
> -	struct bio		*bio,
> -	struct xfs_rtgroup	*rtg)
> +static void
> +xfs_submit_zone_reset_bio(
> +	struct xfs_rtgroup	*rtg,
> +	struct bio		*bio)
>  {
>  	trace_xfs_zone_reset(rtg);
>  
>  	ASSERT(rtg_rmap(rtg)->i_used_blocks == 0);
>  	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
> -		if (!bdev_max_discard_sectors(bio->bi_bdev))
> -			return false;
> +		/*
> +		 * Also use the bio to drive the state machine when neither
> +		 * zone reset nor discard is supported to keep things simple.
> +		 */
> +		if (!bdev_max_discard_sectors(bio->bi_bdev)) {
> +			bio_endio(bio);
> +			return;
> +		}
>  		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
>  		bio->bi_opf |= REQ_OP_DISCARD;
>  		bio->bi_iter.bi_size =
>  			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
>  	}
>  
> -	return true;
> +	submit_bio(bio);
> +}
> +
> +static void xfs_bio_wait_endio(struct bio *bio)
> +{
> +	complete(bio->bi_private);
>  }
>  
>  int
>  xfs_zone_gc_reset_sync(
>  	struct xfs_rtgroup	*rtg)
>  {
> -	int			error = 0;
> +	DECLARE_COMPLETION_ONSTACK(done);
>  	struct bio		bio;
> +	int			error;
>  
>  	bio_init(&bio, rtg_mount(rtg)->m_rtdev_targp->bt_bdev, NULL, 0,
> -			REQ_OP_ZONE_RESET);
> -	if (xfs_zone_gc_prepare_reset(&bio, rtg))
> -		error = submit_bio_wait(&bio);
> -	bio_uninit(&bio);
> +			REQ_OP_ZONE_RESET | REQ_SYNC);
> +	bio.bi_private = &done;
> +	bio.bi_end_io = xfs_bio_wait_endio;
> +	xfs_submit_zone_reset_bio(rtg, &bio);
> +	wait_for_completion_io(&done);
>  
> +	error = blk_status_to_errno(bio.bi_status);
> +	bio_uninit(&bio);
>  	return error;
>  }
>  
> @@ -961,15 +976,7 @@ xfs_zone_gc_reset_zones(
>  		chunk->data = data;
>  		WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
>  		list_add_tail(&chunk->entry, &data->resetting);
> -
> -		/*
> -		 * Also use the bio to drive the state machine when neither
> -		 * zone reset nor discard is supported to keep things simple.
> -		 */
> -		if (xfs_zone_gc_prepare_reset(bio, rtg))
> -			submit_bio(bio);
> -		else
> -			bio_endio(bio);
> +		xfs_submit_zone_reset_bio(rtg, bio);
>  	} while (next);
>  }
>  
> -- 
> 2.47.3
> 
> 

Re: [PATCH 08/10] xfs: refactor zone reset handling

[Hans Holmberg]

On 27/01/2026 17:07, Christoph Hellwig wrote:
> Include the actual bio submission in the common zone reset handler to
> share more code and prepare for adding error injection for zone reset.
> 
> Note the I plan to refactor the block layer submit_bio_wait and
> bio_await_chain code in the next merge window to remove some of the
> code duplication added here.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_zone_gc.c | 49 +++++++++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 60964c926f9f..4023448e85d1 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -893,40 +893,55 @@ xfs_zone_gc_finish_reset(
>  	bio_put(&chunk->bio);
>  }
>  
> -static bool
> -xfs_zone_gc_prepare_reset(
> -	struct bio		*bio,
> -	struct xfs_rtgroup	*rtg)
> +static void
> +xfs_submit_zone_reset_bio(
> +	struct xfs_rtgroup	*rtg,
> +	struct bio		*bio)
>  {
>  	trace_xfs_zone_reset(rtg);
>  
>  	ASSERT(rtg_rmap(rtg)->i_used_blocks == 0);
>  	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
> -		if (!bdev_max_discard_sectors(bio->bi_bdev))
> -			return false;
> +		/*
> +		 * Also use the bio to drive the state machine when neither
> +		 * zone reset nor discard is supported to keep things simple.
> +		 */
> +		if (!bdev_max_discard_sectors(bio->bi_bdev)) {
> +			bio_endio(bio);
> +			return;
> +		}
>  		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
>  		bio->bi_opf |= REQ_OP_DISCARD;
>  		bio->bi_iter.bi_size =
>  			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
>  	}
>  
> -	return true;
> +	submit_bio(bio);
> +}
> +
> +static void xfs_bio_wait_endio(struct bio *bio)
> +{
> +	complete(bio->bi_private);
>  }
>  
>  int
>  xfs_zone_gc_reset_sync(
>  	struct xfs_rtgroup	*rtg)
>  {
> -	int			error = 0;
> +	DECLARE_COMPLETION_ONSTACK(done);
>  	struct bio		bio;
> +	int			error;
>  
>  	bio_init(&bio, rtg_mount(rtg)->m_rtdev_targp->bt_bdev, NULL, 0,
> -			REQ_OP_ZONE_RESET);
> -	if (xfs_zone_gc_prepare_reset(&bio, rtg))
> -		error = submit_bio_wait(&bio);
> -	bio_uninit(&bio);
> +			REQ_OP_ZONE_RESET | REQ_SYNC);
> +	bio.bi_private = &done;
> +	bio.bi_end_io = xfs_bio_wait_endio;
> +	xfs_submit_zone_reset_bio(rtg, &bio);
> +	wait_for_completion_io(&done);
>  
> +	error = blk_status_to_errno(bio.bi_status);
> +	bio_uninit(&bio);
>  	return error;
>  }
>  
> @@ -961,15 +976,7 @@ xfs_zone_gc_reset_zones(
>  		chunk->data = data;
>  		WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
>  		list_add_tail(&chunk->entry, &data->resetting);
> -
> -		/*
> -		 * Also use the bio to drive the state machine when neither
> -		 * zone reset nor discard is supported to keep things simple.
> -		 */
> -		if (xfs_zone_gc_prepare_reset(bio, rtg))
> -			submit_bio(bio);
> -		else
> -			bio_endio(bio);
> +		xfs_submit_zone_reset_bio(rtg, bio);
>  	} while (next);
>  }
>  

Looks good,

Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>

[PATCH 09/10] xfs: add zone reset error injection

[Christoph Hellwig]

Add a new errortag to test that zone reset errors are handled correctly.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_errortag.h |  6 ++++--
 fs/xfs/xfs_zone_gc.c         | 13 +++++++++++--
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
index b7d98471684b..6de207fed2d8 100644
--- a/fs/xfs/libxfs/xfs_errortag.h
+++ b/fs/xfs/libxfs/xfs_errortag.h
@@ -74,7 +74,8 @@
 #define XFS_ERRTAG_EXCHMAPS_FINISH_ONE			44
 #define XFS_ERRTAG_METAFILE_RESV_CRITICAL		45
 #define XFS_ERRTAG_FORCE_ZERO_RANGE			46
-#define XFS_ERRTAG_MAX					47
+#define XFS_ERRTAG_ZONE_RESET				47
+#define XFS_ERRTAG_MAX					48
 
 /*
  * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -135,7 +136,8 @@ XFS_ERRTAG(WB_DELAY_MS,		wb_delay_ms,		3000) \
 XFS_ERRTAG(WRITE_DELAY_MS,	write_delay_ms,		3000) \
 XFS_ERRTAG(EXCHMAPS_FINISH_ONE,	exchmaps_finish_one,	1) \
 XFS_ERRTAG(METAFILE_RESV_CRITICAL, metafile_resv_crit,	4) \
-XFS_ERRTAG(FORCE_ZERO_RANGE,	force_zero_range,	4)
+XFS_ERRTAG(FORCE_ZERO_RANGE,	force_zero_range,	4) \
+XFS_ERRTAG(ZONE_RESET,		zone_reset,		1)
 #endif /* XFS_ERRTAG */
 
 #endif /* __XFS_ERRORTAG_H_ */
diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 4023448e85d1..570102184904 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -16,6 +16,8 @@
 #include "xfs_rmap.h"
 #include "xfs_rtbitmap.h"
 #include "xfs_rtrmap_btree.h"
+#include "xfs_errortag.h"
+#include "xfs_error.h"
 #include "xfs_zone_alloc.h"
 #include "xfs_zone_priv.h"
 #include "xfs_zones.h"
@@ -898,9 +900,17 @@ xfs_submit_zone_reset_bio(
 	struct xfs_rtgroup	*rtg,
 	struct bio		*bio)
 {
+	struct xfs_mount	*mp = rtg_mount(rtg);
+
 	trace_xfs_zone_reset(rtg);
 
 	ASSERT(rtg_rmap(rtg)->i_used_blocks == 0);
+
+	if (XFS_TEST_ERROR(mp, XFS_ERRTAG_ZONE_RESET)) {
+		bio_io_error(bio);
+		return;
+	}
+
 	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
 	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
 		/*
@@ -913,8 +923,7 @@ xfs_submit_zone_reset_bio(
 		}
 		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
 		bio->bi_opf |= REQ_OP_DISCARD;
-		bio->bi_iter.bi_size =
-			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
+		bio->bi_iter.bi_size = XFS_FSB_TO_B(mp, rtg_blocks(rtg));
 	}
 
 	submit_bio(bio);
-- 
2.47.3

Re: [PATCH 09/10] xfs: add zone reset error injection

[Darrick J. Wong]

On Tue, Jan 27, 2026 at 05:05:49PM +0100, Christoph Hellwig wrote:
> Add a new errortag to test that zone reset errors are handled correctly.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

This seems pretty straightforward.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_errortag.h |  6 ++++--
>  fs/xfs/xfs_zone_gc.c         | 13 +++++++++++--
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index b7d98471684b..6de207fed2d8 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -74,7 +74,8 @@
>  #define XFS_ERRTAG_EXCHMAPS_FINISH_ONE			44
>  #define XFS_ERRTAG_METAFILE_RESV_CRITICAL		45
>  #define XFS_ERRTAG_FORCE_ZERO_RANGE			46
> -#define XFS_ERRTAG_MAX					47
> +#define XFS_ERRTAG_ZONE_RESET				47
> +#define XFS_ERRTAG_MAX					48
>  
>  /*
>   * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
> @@ -135,7 +136,8 @@ XFS_ERRTAG(WB_DELAY_MS,		wb_delay_ms,		3000) \
>  XFS_ERRTAG(WRITE_DELAY_MS,	write_delay_ms,		3000) \
>  XFS_ERRTAG(EXCHMAPS_FINISH_ONE,	exchmaps_finish_one,	1) \
>  XFS_ERRTAG(METAFILE_RESV_CRITICAL, metafile_resv_crit,	4) \
> -XFS_ERRTAG(FORCE_ZERO_RANGE,	force_zero_range,	4)
> +XFS_ERRTAG(FORCE_ZERO_RANGE,	force_zero_range,	4) \
> +XFS_ERRTAG(ZONE_RESET,		zone_reset,		1)
>  #endif /* XFS_ERRTAG */
>  
>  #endif /* __XFS_ERRORTAG_H_ */
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 4023448e85d1..570102184904 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -16,6 +16,8 @@
>  #include "xfs_rmap.h"
>  #include "xfs_rtbitmap.h"
>  #include "xfs_rtrmap_btree.h"
> +#include "xfs_errortag.h"
> +#include "xfs_error.h"
>  #include "xfs_zone_alloc.h"
>  #include "xfs_zone_priv.h"
>  #include "xfs_zones.h"
> @@ -898,9 +900,17 @@ xfs_submit_zone_reset_bio(
>  	struct xfs_rtgroup	*rtg,
>  	struct bio		*bio)
>  {
> +	struct xfs_mount	*mp = rtg_mount(rtg);
> +
>  	trace_xfs_zone_reset(rtg);
>  
>  	ASSERT(rtg_rmap(rtg)->i_used_blocks == 0);
> +
> +	if (XFS_TEST_ERROR(mp, XFS_ERRTAG_ZONE_RESET)) {
> +		bio_io_error(bio);
> +		return;
> +	}
> +
>  	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
>  		/*
> @@ -913,8 +923,7 @@ xfs_submit_zone_reset_bio(
>  		}
>  		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
>  		bio->bi_opf |= REQ_OP_DISCARD;
> -		bio->bi_iter.bi_size =
> -			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
> +		bio->bi_iter.bi_size = XFS_FSB_TO_B(mp, rtg_blocks(rtg));
>  	}
>  
>  	submit_bio(bio);
> -- 
> 2.47.3
> 
> 

Re: [PATCH 09/10] xfs: add zone reset error injection

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:05:49PM +0100, Christoph Hellwig wrote:
> Add a new errortag to test that zone reset errors are handled correctly.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_errortag.h |  6 ++++--
>  fs/xfs/xfs_zone_gc.c         | 13 +++++++++++--
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index b7d98471684b..6de207fed2d8 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -74,7 +74,8 @@
>  #define XFS_ERRTAG_EXCHMAPS_FINISH_ONE			44
>  #define XFS_ERRTAG_METAFILE_RESV_CRITICAL		45
>  #define XFS_ERRTAG_FORCE_ZERO_RANGE			46
> -#define XFS_ERRTAG_MAX					47
> +#define XFS_ERRTAG_ZONE_RESET				47
> +#define XFS_ERRTAG_MAX					48
>  
>  /*
>   * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
> @@ -135,7 +136,8 @@ XFS_ERRTAG(WB_DELAY_MS,		wb_delay_ms,		3000) \
>  XFS_ERRTAG(WRITE_DELAY_MS,	write_delay_ms,		3000) \
>  XFS_ERRTAG(EXCHMAPS_FINISH_ONE,	exchmaps_finish_one,	1) \
>  XFS_ERRTAG(METAFILE_RESV_CRITICAL, metafile_resv_crit,	4) \
> -XFS_ERRTAG(FORCE_ZERO_RANGE,	force_zero_range,	4)
> +XFS_ERRTAG(FORCE_ZERO_RANGE,	force_zero_range,	4) \
> +XFS_ERRTAG(ZONE_RESET,		zone_reset,		1)
>  #endif /* XFS_ERRTAG */
>  
>  #endif /* __XFS_ERRORTAG_H_ */
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 4023448e85d1..570102184904 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -16,6 +16,8 @@
>  #include "xfs_rmap.h"
>  #include "xfs_rtbitmap.h"
>  #include "xfs_rtrmap_btree.h"
> +#include "xfs_errortag.h"
> +#include "xfs_error.h"
>  #include "xfs_zone_alloc.h"
>  #include "xfs_zone_priv.h"
>  #include "xfs_zones.h"
> @@ -898,9 +900,17 @@ xfs_submit_zone_reset_bio(
>  	struct xfs_rtgroup	*rtg,
>  	struct bio		*bio)
>  {
> +	struct xfs_mount	*mp = rtg_mount(rtg);
> +
>  	trace_xfs_zone_reset(rtg);
>  
>  	ASSERT(rtg_rmap(rtg)->i_used_blocks == 0);
> +
> +	if (XFS_TEST_ERROR(mp, XFS_ERRTAG_ZONE_RESET)) {
> +		bio_io_error(bio);
> +		return;
> +	}
> +
>  	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
>  		/*
> @@ -913,8 +923,7 @@ xfs_submit_zone_reset_bio(
>  		}
>  		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
>  		bio->bi_opf |= REQ_OP_DISCARD;
> -		bio->bi_iter.bi_size =
> -			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
> +		bio->bi_iter.bi_size = XFS_FSB_TO_B(mp, rtg_blocks(rtg));
>  	}
>  
>  	submit_bio(bio);
> -- 

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

Re: [PATCH 09/10] xfs: add zone reset error injection

[Hans Holmberg]

On 27/01/2026 17:07, Christoph Hellwig wrote:
> Add a new errortag to test that zone reset errors are handled correctly.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_errortag.h |  6 ++++--
>  fs/xfs/xfs_zone_gc.c         | 13 +++++++++++--
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index b7d98471684b..6de207fed2d8 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -74,7 +74,8 @@
>  #define XFS_ERRTAG_EXCHMAPS_FINISH_ONE			44
>  #define XFS_ERRTAG_METAFILE_RESV_CRITICAL		45
>  #define XFS_ERRTAG_FORCE_ZERO_RANGE			46
> -#define XFS_ERRTAG_MAX					47
> +#define XFS_ERRTAG_ZONE_RESET				47
> +#define XFS_ERRTAG_MAX					48
>  
>  /*
>   * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
> @@ -135,7 +136,8 @@ XFS_ERRTAG(WB_DELAY_MS,		wb_delay_ms,		3000) \
>  XFS_ERRTAG(WRITE_DELAY_MS,	write_delay_ms,		3000) \
>  XFS_ERRTAG(EXCHMAPS_FINISH_ONE,	exchmaps_finish_one,	1) \
>  XFS_ERRTAG(METAFILE_RESV_CRITICAL, metafile_resv_crit,	4) \
> -XFS_ERRTAG(FORCE_ZERO_RANGE,	force_zero_range,	4)
> +XFS_ERRTAG(FORCE_ZERO_RANGE,	force_zero_range,	4) \
> +XFS_ERRTAG(ZONE_RESET,		zone_reset,		1)
>  #endif /* XFS_ERRTAG */
>  
>  #endif /* __XFS_ERRORTAG_H_ */
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 4023448e85d1..570102184904 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -16,6 +16,8 @@
>  #include "xfs_rmap.h"
>  #include "xfs_rtbitmap.h"
>  #include "xfs_rtrmap_btree.h"
> +#include "xfs_errortag.h"
> +#include "xfs_error.h"
>  #include "xfs_zone_alloc.h"
>  #include "xfs_zone_priv.h"
>  #include "xfs_zones.h"
> @@ -898,9 +900,17 @@ xfs_submit_zone_reset_bio(
>  	struct xfs_rtgroup	*rtg,
>  	struct bio		*bio)
>  {
> +	struct xfs_mount	*mp = rtg_mount(rtg);
> +
>  	trace_xfs_zone_reset(rtg);
>  
>  	ASSERT(rtg_rmap(rtg)->i_used_blocks == 0);
> +
> +	if (XFS_TEST_ERROR(mp, XFS_ERRTAG_ZONE_RESET)) {
> +		bio_io_error(bio);
> +		return;
> +	}
> +
>  	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
>  		/*
> @@ -913,8 +923,7 @@ xfs_submit_zone_reset_bio(
>  		}
>  		bio->bi_opf &= ~REQ_OP_ZONE_RESET;
>  		bio->bi_opf |= REQ_OP_DISCARD;
> -		bio->bi_iter.bi_size =
> -			XFS_FSB_TO_B(rtg_mount(rtg), rtg_blocks(rtg));
> +		bio->bi_iter.bi_size = XFS_FSB_TO_B(mp, rtg_blocks(rtg));
>  	}
>  
>  	submit_bio(bio);

Looks good,

Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>

[PATCH 10/10] xfs: add sysfs stats for zoned GC

[Christoph Hellwig]

Add counters of read, write and zone_reset operations as well as
GC written bytes to sysfs.  This way they can be easily used for
monitoring tools and test cases.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_stats.c   | 7 ++++++-
 fs/xfs/xfs_stats.h   | 6 ++++++
 fs/xfs/xfs_zone_gc.c | 7 +++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index 9781222e0653..6d7f98afa31a 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -24,6 +24,7 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
 	uint64_t	xs_write_bytes = 0;
 	uint64_t	xs_read_bytes = 0;
 	uint64_t	defer_relog = 0;
+	uint64_t	xs_gc_write_bytes = 0;
 
 	static const struct xstats_entry {
 		char	*desc;
@@ -57,7 +58,8 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
 		{ "rtrmapbt_mem",	xfsstats_offset(xs_rtrefcbt_2)	},
 		{ "rtrefcntbt",		xfsstats_offset(xs_qm_dqreclaims)},
 		/* we print both series of quota information together */
-		{ "qm",			xfsstats_offset(xs_xstrat_bytes)},
+		{ "qm",			xfsstats_offset(xs_gc_read_calls)},
+		{ "zoned",		xfsstats_offset(__pad1)},
 	};
 
 	/* Loop over all stats groups */
@@ -77,6 +79,7 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
 		xs_write_bytes += per_cpu_ptr(stats, i)->s.xs_write_bytes;
 		xs_read_bytes += per_cpu_ptr(stats, i)->s.xs_read_bytes;
 		defer_relog += per_cpu_ptr(stats, i)->s.defer_relog;
+		xs_gc_write_bytes += per_cpu_ptr(stats, i)->s.xs_read_bytes;
 	}
 
 	len += scnprintf(buf + len, PATH_MAX-len, "xpc %llu %llu %llu\n",
@@ -89,6 +92,8 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
 #else
 		0);
 #endif
+	len += scnprintf(buf + len, PATH_MAX-len, "gc xpc %llu\n",
+			xs_gc_write_bytes);
 
 	return len;
 }
diff --git a/fs/xfs/xfs_stats.h b/fs/xfs/xfs_stats.h
index 15ba1abcf253..16dbbc0b72db 100644
--- a/fs/xfs/xfs_stats.h
+++ b/fs/xfs/xfs_stats.h
@@ -138,10 +138,16 @@ struct __xfsstats {
 	uint32_t		xs_qm_dqwants;
 	uint32_t		xs_qm_dquot;
 	uint32_t		xs_qm_dquot_unused;
+/* Zone GC counters */
+	uint32_t		xs_gc_read_calls;
+	uint32_t		xs_gc_write_calls;
+	uint32_t		xs_gc_zone_reset_calls;
+	uint32_t		__pad1;
 /* Extra precision counters */
 	uint64_t		xs_xstrat_bytes;
 	uint64_t		xs_write_bytes;
 	uint64_t		xs_read_bytes;
+	uint64_t		xs_gc_write_bytes;
 	uint64_t		defer_relog;
 };
 
diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 570102184904..4bb647d3be41 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -712,6 +712,8 @@ xfs_zone_gc_start_chunk(
 	data->scratch_head = (data->scratch_head + len) % data->scratch_size;
 	data->scratch_available -= len;
 
+	XFS_STATS_INC(mp, xs_gc_read_calls);
+
 	WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
 	list_add_tail(&chunk->entry, &data->reading);
 	xfs_zone_gc_iter_advance(iter, irec.rm_blockcount);
@@ -815,6 +817,9 @@ xfs_zone_gc_write_chunk(
 		return;
 	}
 
+	XFS_STATS_INC(mp, xs_gc_write_calls);
+	XFS_STATS_ADD(mp, xs_gc_write_bytes, chunk->len);
+
 	WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
 	list_move_tail(&chunk->entry, &data->writing);
 
@@ -911,6 +916,8 @@ xfs_submit_zone_reset_bio(
 		return;
 	}
 
+	XFS_STATS_INC(mp, xs_gc_zone_reset_calls);
+
 	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
 	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
 		/*
-- 
2.47.3

Re: [PATCH 10/10] xfs: add sysfs stats for zoned GC

[Darrick J. Wong]

On Tue, Jan 27, 2026 at 05:05:50PM +0100, Christoph Hellwig wrote:
> Add counters of read, write and zone_reset operations as well as
> GC written bytes to sysfs.  This way they can be easily used for
> monitoring tools and test cases.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

+1 for observability
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_stats.c   | 7 ++++++-
>  fs/xfs/xfs_stats.h   | 6 ++++++
>  fs/xfs/xfs_zone_gc.c | 7 +++++++
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> index 9781222e0653..6d7f98afa31a 100644
> --- a/fs/xfs/xfs_stats.c
> +++ b/fs/xfs/xfs_stats.c
> @@ -24,6 +24,7 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  	uint64_t	xs_write_bytes = 0;
>  	uint64_t	xs_read_bytes = 0;
>  	uint64_t	defer_relog = 0;
> +	uint64_t	xs_gc_write_bytes = 0;
>  
>  	static const struct xstats_entry {
>  		char	*desc;
> @@ -57,7 +58,8 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  		{ "rtrmapbt_mem",	xfsstats_offset(xs_rtrefcbt_2)	},
>  		{ "rtrefcntbt",		xfsstats_offset(xs_qm_dqreclaims)},
>  		/* we print both series of quota information together */
> -		{ "qm",			xfsstats_offset(xs_xstrat_bytes)},
> +		{ "qm",			xfsstats_offset(xs_gc_read_calls)},
> +		{ "zoned",		xfsstats_offset(__pad1)},
>  	};
>  
>  	/* Loop over all stats groups */
> @@ -77,6 +79,7 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  		xs_write_bytes += per_cpu_ptr(stats, i)->s.xs_write_bytes;
>  		xs_read_bytes += per_cpu_ptr(stats, i)->s.xs_read_bytes;
>  		defer_relog += per_cpu_ptr(stats, i)->s.defer_relog;
> +		xs_gc_write_bytes += per_cpu_ptr(stats, i)->s.xs_read_bytes;
>  	}
>  
>  	len += scnprintf(buf + len, PATH_MAX-len, "xpc %llu %llu %llu\n",
> @@ -89,6 +92,8 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  #else
>  		0);
>  #endif
> +	len += scnprintf(buf + len, PATH_MAX-len, "gc xpc %llu\n",
> +			xs_gc_write_bytes);
>  
>  	return len;
>  }
> diff --git a/fs/xfs/xfs_stats.h b/fs/xfs/xfs_stats.h
> index 15ba1abcf253..16dbbc0b72db 100644
> --- a/fs/xfs/xfs_stats.h
> +++ b/fs/xfs/xfs_stats.h
> @@ -138,10 +138,16 @@ struct __xfsstats {
>  	uint32_t		xs_qm_dqwants;
>  	uint32_t		xs_qm_dquot;
>  	uint32_t		xs_qm_dquot_unused;
> +/* Zone GC counters */
> +	uint32_t		xs_gc_read_calls;
> +	uint32_t		xs_gc_write_calls;
> +	uint32_t		xs_gc_zone_reset_calls;
> +	uint32_t		__pad1;
>  /* Extra precision counters */
>  	uint64_t		xs_xstrat_bytes;
>  	uint64_t		xs_write_bytes;
>  	uint64_t		xs_read_bytes;
> +	uint64_t		xs_gc_write_bytes;
>  	uint64_t		defer_relog;
>  };
>  
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 570102184904..4bb647d3be41 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -712,6 +712,8 @@ xfs_zone_gc_start_chunk(
>  	data->scratch_head = (data->scratch_head + len) % data->scratch_size;
>  	data->scratch_available -= len;
>  
> +	XFS_STATS_INC(mp, xs_gc_read_calls);
> +
>  	WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
>  	list_add_tail(&chunk->entry, &data->reading);
>  	xfs_zone_gc_iter_advance(iter, irec.rm_blockcount);
> @@ -815,6 +817,9 @@ xfs_zone_gc_write_chunk(
>  		return;
>  	}
>  
> +	XFS_STATS_INC(mp, xs_gc_write_calls);
> +	XFS_STATS_ADD(mp, xs_gc_write_bytes, chunk->len);
> +
>  	WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
>  	list_move_tail(&chunk->entry, &data->writing);
>  
> @@ -911,6 +916,8 @@ xfs_submit_zone_reset_bio(
>  		return;
>  	}
>  
> +	XFS_STATS_INC(mp, xs_gc_zone_reset_calls);
> +
>  	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
>  		/*
> -- 
> 2.47.3
> 
> 

Re: [PATCH 10/10] xfs: add sysfs stats for zoned GC

[Carlos Maiolino]

On Tue, Jan 27, 2026 at 05:05:50PM +0100, Christoph Hellwig wrote:
> Add counters of read, write and zone_reset operations as well as
> GC written bytes to sysfs.  This way they can be easily used for
> monitoring tools and test cases.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>


Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/xfs_stats.c   | 7 ++++++-
>  fs/xfs/xfs_stats.h   | 6 ++++++
>  fs/xfs/xfs_zone_gc.c | 7 +++++++
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> index 9781222e0653..6d7f98afa31a 100644
> --- a/fs/xfs/xfs_stats.c
> +++ b/fs/xfs/xfs_stats.c
> @@ -24,6 +24,7 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  	uint64_t	xs_write_bytes = 0;
>  	uint64_t	xs_read_bytes = 0;
>  	uint64_t	defer_relog = 0;
> +	uint64_t	xs_gc_write_bytes = 0;
>  
>  	static const struct xstats_entry {
>  		char	*desc;
> @@ -57,7 +58,8 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  		{ "rtrmapbt_mem",	xfsstats_offset(xs_rtrefcbt_2)	},
>  		{ "rtrefcntbt",		xfsstats_offset(xs_qm_dqreclaims)},
>  		/* we print both series of quota information together */
> -		{ "qm",			xfsstats_offset(xs_xstrat_bytes)},
> +		{ "qm",			xfsstats_offset(xs_gc_read_calls)},
> +		{ "zoned",		xfsstats_offset(__pad1)},
>  	};
>  
>  	/* Loop over all stats groups */
> @@ -77,6 +79,7 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  		xs_write_bytes += per_cpu_ptr(stats, i)->s.xs_write_bytes;
>  		xs_read_bytes += per_cpu_ptr(stats, i)->s.xs_read_bytes;
>  		defer_relog += per_cpu_ptr(stats, i)->s.defer_relog;
> +		xs_gc_write_bytes += per_cpu_ptr(stats, i)->s.xs_read_bytes;
>  	}
>  
>  	len += scnprintf(buf + len, PATH_MAX-len, "xpc %llu %llu %llu\n",
> @@ -89,6 +92,8 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  #else
>  		0);
>  #endif
> +	len += scnprintf(buf + len, PATH_MAX-len, "gc xpc %llu\n",
> +			xs_gc_write_bytes);
>  
>  	return len;
>  }
> diff --git a/fs/xfs/xfs_stats.h b/fs/xfs/xfs_stats.h
> index 15ba1abcf253..16dbbc0b72db 100644
> --- a/fs/xfs/xfs_stats.h
> +++ b/fs/xfs/xfs_stats.h
> @@ -138,10 +138,16 @@ struct __xfsstats {
>  	uint32_t		xs_qm_dqwants;
>  	uint32_t		xs_qm_dquot;
>  	uint32_t		xs_qm_dquot_unused;
> +/* Zone GC counters */
> +	uint32_t		xs_gc_read_calls;
> +	uint32_t		xs_gc_write_calls;
> +	uint32_t		xs_gc_zone_reset_calls;
> +	uint32_t		__pad1;
>  /* Extra precision counters */
>  	uint64_t		xs_xstrat_bytes;
>  	uint64_t		xs_write_bytes;
>  	uint64_t		xs_read_bytes;
> +	uint64_t		xs_gc_write_bytes;
>  	uint64_t		defer_relog;
>  };
>  
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 570102184904..4bb647d3be41 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -712,6 +712,8 @@ xfs_zone_gc_start_chunk(
>  	data->scratch_head = (data->scratch_head + len) % data->scratch_size;
>  	data->scratch_available -= len;
>  
> +	XFS_STATS_INC(mp, xs_gc_read_calls);
> +
>  	WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
>  	list_add_tail(&chunk->entry, &data->reading);
>  	xfs_zone_gc_iter_advance(iter, irec.rm_blockcount);
> @@ -815,6 +817,9 @@ xfs_zone_gc_write_chunk(
>  		return;
>  	}
>  
> +	XFS_STATS_INC(mp, xs_gc_write_calls);
> +	XFS_STATS_ADD(mp, xs_gc_write_bytes, chunk->len);
> +
>  	WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
>  	list_move_tail(&chunk->entry, &data->writing);
>  
> @@ -911,6 +916,8 @@ xfs_submit_zone_reset_bio(
>  		return;
>  	}
>  
> +	XFS_STATS_INC(mp, xs_gc_zone_reset_calls);
> +
>  	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
>  		/*
> -- 
> 2.47.3
> 
> 

Re: [PATCH 10/10] xfs: add sysfs stats for zoned GC

[Hans Holmberg]

On 27/01/2026 17:07, Christoph Hellwig wrote:
> Add counters of read, write and zone_reset operations as well as
> GC written bytes to sysfs.  This way they can be easily used for
> monitoring tools and test cases.

This is great, but I read this as "add all of these counters to sysfs", so clarifying
that it is only gc bytes written that is added to sysfs would be good.

You did not add a counter for gc bytes read, because that is equal to gc bytes written?

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_stats.c   | 7 ++++++-
>  fs/xfs/xfs_stats.h   | 6 ++++++
>  fs/xfs/xfs_zone_gc.c | 7 +++++++
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> index 9781222e0653..6d7f98afa31a 100644
> --- a/fs/xfs/xfs_stats.c
> +++ b/fs/xfs/xfs_stats.c
> @@ -24,6 +24,7 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  	uint64_t	xs_write_bytes = 0;
>  	uint64_t	xs_read_bytes = 0;
>  	uint64_t	defer_relog = 0;
> +	uint64_t	xs_gc_write_bytes = 0;
>  
>  	static const struct xstats_entry {
>  		char	*desc;
> @@ -57,7 +58,8 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  		{ "rtrmapbt_mem",	xfsstats_offset(xs_rtrefcbt_2)	},
>  		{ "rtrefcntbt",		xfsstats_offset(xs_qm_dqreclaims)},
>  		/* we print both series of quota information together */
> -		{ "qm",			xfsstats_offset(xs_xstrat_bytes)},
> +		{ "qm",			xfsstats_offset(xs_gc_read_calls)},
> +		{ "zoned",		xfsstats_offset(__pad1)},
>  	};
>  
>  	/* Loop over all stats groups */
> @@ -77,6 +79,7 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  		xs_write_bytes += per_cpu_ptr(stats, i)->s.xs_write_bytes;
>  		xs_read_bytes += per_cpu_ptr(stats, i)->s.xs_read_bytes;
>  		defer_relog += per_cpu_ptr(stats, i)->s.defer_relog;
> +		xs_gc_write_bytes += per_cpu_ptr(stats, i)->s.xs_read_bytes;

oh! this should be:

xs_gc_write_bytes += per_cpu_ptr(stats, i)->s.xs_gc_write_bytes;

right?


>  	}
>  
>  	len += scnprintf(buf + len, PATH_MAX-len, "xpc %llu %llu %llu\n",
> @@ -89,6 +92,8 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
>  #else
>  		0);
>  #endif
> +	len += scnprintf(buf + len, PATH_MAX-len, "gc xpc %llu\n",
> +			xs_gc_write_bytes);
>  
>  	return len;
>  }
> diff --git a/fs/xfs/xfs_stats.h b/fs/xfs/xfs_stats.h
> index 15ba1abcf253..16dbbc0b72db 100644
> --- a/fs/xfs/xfs_stats.h
> +++ b/fs/xfs/xfs_stats.h
> @@ -138,10 +138,16 @@ struct __xfsstats {
>  	uint32_t		xs_qm_dqwants;
>  	uint32_t		xs_qm_dquot;
>  	uint32_t		xs_qm_dquot_unused;
> +/* Zone GC counters */
> +	uint32_t		xs_gc_read_calls;
> +	uint32_t		xs_gc_write_calls;
> +	uint32_t		xs_gc_zone_reset_calls;
> +	uint32_t		__pad1;
>  /* Extra precision counters */
>  	uint64_t		xs_xstrat_bytes;
>  	uint64_t		xs_write_bytes;
>  	uint64_t		xs_read_bytes;
> +	uint64_t		xs_gc_write_bytes;
>  	uint64_t		defer_relog;
>  };
>  
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 570102184904..4bb647d3be41 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -712,6 +712,8 @@ xfs_zone_gc_start_chunk(
>  	data->scratch_head = (data->scratch_head + len) % data->scratch_size;
>  	data->scratch_available -= len;
>  
> +	XFS_STATS_INC(mp, xs_gc_read_calls);
> +
>  	WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
>  	list_add_tail(&chunk->entry, &data->reading);
>  	xfs_zone_gc_iter_advance(iter, irec.rm_blockcount);
> @@ -815,6 +817,9 @@ xfs_zone_gc_write_chunk(
>  		return;
>  	}
>  
> +	XFS_STATS_INC(mp, xs_gc_write_calls);
> +	XFS_STATS_ADD(mp, xs_gc_write_bytes, chunk->len);
> +
>  	WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
>  	list_move_tail(&chunk->entry, &data->writing);
>  
> @@ -911,6 +916,8 @@ xfs_submit_zone_reset_bio(
>  		return;
>  	}
>  
> +	XFS_STATS_INC(mp, xs_gc_zone_reset_calls);
> +
>  	bio->bi_iter.bi_sector = xfs_gbno_to_daddr(&rtg->rtg_group, 0);
>  	if (!bdev_zone_is_seq(bio->bi_bdev, bio->bi_iter.bi_sector)) {
>  		/*

Re: [PATCH 10/10] xfs: add sysfs stats for zoned GC

[hch]

On Wed, Jan 28, 2026 at 12:53:24PM +0000, Hans Holmberg wrote:
> On 27/01/2026 17:07, Christoph Hellwig wrote:
> > Add counters of read, write and zone_reset operations as well as
> > GC written bytes to sysfs.  This way they can be easily used for
> > monitoring tools and test cases.
> 
> This is great, but I read this as "add all of these counters to sysfs", so clarifying
> that it is only gc bytes written that is added to sysfs would be good.

All of them are added.  But the others are 32-bit counters and
handled by the existing code just by adding the new group to the
array.

> You did not add a counter for gc bytes read, because that is equal to gc bytes written?

Yes.

Re: [PATCH 10/10] xfs: add sysfs stats for zoned GC

[Hans Holmberg]

On 28/01/2026 15:12, hch wrote:
> On Wed, Jan 28, 2026 at 12:53:24PM +0000, Hans Holmberg wrote:
>> On 27/01/2026 17:07, Christoph Hellwig wrote:
>>> Add counters of read, write and zone_reset operations as well as
>>> GC written bytes to sysfs.  This way they can be easily used for
>>> monitoring tools and test cases.
>>
>> This is great, but I read this as "add all of these counters to sysfs", so clarifying
>> that it is only gc bytes written that is added to sysfs would be good.
> 
> All of them are added.  But the others are 32-bit counters and
> handled by the existing code just by adding the new group to the
> array.

AAh, now I see.

> 
>> You did not add a counter for gc bytes read, because that is equal to gc bytes written?
> 
> Yes.
> 

nit:

xs_gc_write_bytes is fine by me as (it is not wrong) but maybe xs_gc_bytes would be a better name?
(as it is the amount of data moved)

and you saw my comment on the xs_gc_write_bytes typo?

Re: [PATCH 10/10] xfs: add sysfs stats for zoned GC

[hch]

On Wed, Jan 28, 2026 at 03:11:01PM +0000, Hans Holmberg wrote:
> xs_gc_write_bytes is fine by me as (it is not wrong) but maybe xs_gc_bytes would be a better name?

Fine with me.

> (as it is the amount of data moved)
> 
> and you saw my comment on the xs_gc_write_bytes typo?

No, must have missed it, I'll take another look.