[PATCH 0/3] xfs: support on by default errortags

[PATCH 0/3] xfs: support on by default errortags

[Brian Foster]

Hi all,

This is a quick side quest based on the discussion on the iomap folio
batch series here[1]. I could see this going a number of different ways
and/or being enhanced in the future. For example, we could have per-tag
warning state instead of the global op state, dynamic default
configuration, put this behind a kconfig option, etc. My goal here is
really just to implement enough to keep things simple and pretty close
to existing XFS_DEBUG behavior.

Ultimately if people are Ok with something like this, I'll change the
force zero tag in the iomap series to on by default. If not, I'll just
leave it as is and go with a custom test. Otherwise, this survives a
first run through fstests without any explosions or spurious failures.

Thoughts, reviews, flames appreciated.

Brian

[1] https://lore.kernel.org/linux-fsdevel/20250605173357.579720-8-bfoster@redhat.com/

Brian Foster (3):
  xfs: support on by default errortags
  xfs: convert extent alloc debug fallback to errortag
  xfs: convert sparse inode alloc debug fallback to errortag

 fs/xfs/libxfs/xfs_alloc.c    |  5 +---
 fs/xfs/libxfs/xfs_errortag.h |  6 ++++-
 fs/xfs/libxfs/xfs_ialloc.c   | 14 ++++-------
 fs/xfs/xfs_error.c           | 48 ++++++++++++++++++++++++++++++++++--
 fs/xfs/xfs_mount.h           |  3 +++
 5 files changed, 60 insertions(+), 16 deletions(-)

-- 
2.49.0

[PATCH 1/3] xfs: support on by default errortags

[Brian Foster]

XFS has a couple places where atypical behavior is randomized in
DEBUG mode in order to facilitate testing and code coverage. For
example, DEBUG kernels randomly drop into different block and inode
allocation algorithms that on production kernels may only occur
under certain circumstances.

These hooks are essentially hardcoded errortags. Rather than add
more of such logic for similar things in the future, introduce the
ability to define errortags that are on by default. Since errortags
are somewhat noisy, also introduce a quiet mode opstate flag to
suppress logging warnings for such tags. Quiet mode is enabled when
at least one tag is enabled by default at mount time and then
disabled upon the first errortag configuration change from
userspace.

This generally mimics current XFS_DEBUG behavior with the exception
that logging is enabled for all tags once any other tag is
configured. This can be enhanced to support per-tag log state in the
future if needed, but for now is probably unnecessary as only a
handful of default enabled tags are expected.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_error.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 fs/xfs/xfs_mount.h |  3 +++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index dbd87e137694..62ac6debcb5e 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -69,6 +69,7 @@ static unsigned int xfs_errortag_random_default[] = {
 struct xfs_errortag_attr {
 	struct attribute	attr;
 	unsigned int		tag;
+	bool			enable_default;
 };
 
 static inline struct xfs_errortag_attr *
@@ -129,12 +130,15 @@ static const struct sysfs_ops xfs_errortag_sysfs_ops = {
 	.store = xfs_errortag_attr_store,
 };
 
-#define XFS_ERRORTAG_ATTR_RW(_name, _tag) \
+#define __XFS_ERRORTAG_ATTR_RW(_name, _tag, enable) \
 static struct xfs_errortag_attr xfs_errortag_attr_##_name = {		\
 	.attr = {.name = __stringify(_name),				\
 		 .mode = VERIFY_OCTAL_PERMISSIONS(S_IWUSR | S_IRUGO) },	\
 	.tag	= (_tag),						\
+	.enable_default = enable,					\
 }
+#define XFS_ERRORTAG_ATTR_RW(_name, _tag) \
+	__XFS_ERRORTAG_ATTR_RW(_name, _tag, false)
 
 #define XFS_ERRORTAG_ATTR_LIST(_name) &xfs_errortag_attr_##_name.attr
 
@@ -240,6 +244,35 @@ static const struct kobj_type xfs_errortag_ktype = {
 	.default_groups = xfs_errortag_groups,
 };
 
+/*
+ * Enable tags that are defined to be on by default. This is typically limited
+ * to tags that don't necessarily inject errors, but rather modify control paths
+ * for improved code coverage testing on DEBUG kernels.
+ */
+static void
+xfs_errortag_enable_defaults(
+	struct xfs_mount	*mp)
+{
+	int i;
+
+	for (i = 0; xfs_errortag_attrs[i]; i++) {
+		struct xfs_errortag_attr *xfs_attr =
+				to_attr(xfs_errortag_attrs[i]);
+
+		if (!xfs_attr->enable_default)
+			continue;
+
+		/*
+		 * Suppress log noise unless userspace makes configuration
+		 * changes. Open code the assignment to avoid clearing the quiet
+		 * flag.
+		 */
+		xfs_set_quiet_errtag(mp);
+		mp->m_errortag[xfs_attr->tag] =
+			xfs_errortag_random_default[xfs_attr->tag];
+	}
+}
+
 int
 xfs_errortag_init(
 	struct xfs_mount	*mp)
@@ -251,6 +284,8 @@ xfs_errortag_init(
 	if (!mp->m_errortag)
 		return -ENOMEM;
 
+	xfs_errortag_enable_defaults(mp);
+
 	ret = xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype,
 				&mp->m_kobj, "errortag");
 	if (ret)
@@ -320,9 +355,11 @@ xfs_errortag_test(
 	if (!randfactor || get_random_u32_below(randfactor))
 		return false;
 
-	xfs_warn_ratelimited(mp,
+	if (!xfs_is_quiet_errtag(mp)) {
+		xfs_warn_ratelimited(mp,
 "Injecting error (%s) at file %s, line %d, on filesystem \"%s\"",
 			expression, file, line, mp->m_super->s_id);
+	}
 	return true;
 }
 
@@ -346,6 +383,7 @@ xfs_errortag_set(
 	if (!xfs_errortag_valid(error_tag))
 		return -EINVAL;
 
+	xfs_clear_quiet_errtag(mp);
 	mp->m_errortag[error_tag] = tag_value;
 	return 0;
 }
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index d85084f9f317..44b02728056f 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -558,6 +558,8 @@ __XFS_HAS_FEAT(nouuid, NOUUID)
  */
 #define XFS_OPSTATE_BLOCKGC_ENABLED	6
 
+/* Debug kernel skips warning on errtag event triggers */
+#define XFS_OPSTATE_QUIET_ERRTAG	7
 /* Kernel has logged a warning about shrink being used on this fs. */
 #define XFS_OPSTATE_WARNED_SHRINK	9
 /* Kernel has logged a warning about logged xattr updates being used. */
@@ -600,6 +602,7 @@ __XFS_IS_OPSTATE(inode32, INODE32)
 __XFS_IS_OPSTATE(readonly, READONLY)
 __XFS_IS_OPSTATE(inodegc_enabled, INODEGC_ENABLED)
 __XFS_IS_OPSTATE(blockgc_enabled, BLOCKGC_ENABLED)
+__XFS_IS_OPSTATE(quiet_errtag, QUIET_ERRTAG)
 #ifdef CONFIG_XFS_QUOTA
 __XFS_IS_OPSTATE(quotacheck_running, QUOTACHECK_RUNNING)
 __XFS_IS_OPSTATE(resuming_quotaon, RESUMING_QUOTAON)
-- 
2.49.0

[PATCH 2/3] xfs: convert extent alloc debug fallback to errortag

[Brian Foster]

Extent allocation includes multiple algorithms depending on on-disk
state to maintain performance and efficiency. The likely first
candidate is to simply use the rightmost block in the cntbt if
that's where initial lookup landed.

Since this is a common path for larger allocations on freshly
created filesystems, we include some DEBUG mode logic to randomly
fall out of the this algorithm and exercise fallback behavior. Now
that errortags can be enabled by default, convert this logic to an
errortag and drop the unnecessary DEBUG ifdef.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_alloc.c    | 5 +----
 fs/xfs/libxfs/xfs_errortag.h | 4 +++-
 fs/xfs/xfs_error.c           | 3 +++
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 7839efe050bf..129c9f690afc 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -1615,11 +1615,8 @@ xfs_alloc_ag_vextent_lastblock(
 	int			error;
 	int			i;
 
-#ifdef DEBUG
-	/* Randomly don't execute the first algorithm. */
-	if (get_random_u32_below(2))
+	if (XFS_TEST_ERROR(false, args->mp, XFS_ERRTAG_AG_ALLOC_SKIP))
 		return 0;
-#endif
 
 	/*
 	 * Start from the entry that lookup found, sequence through all larger
diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
index a53c5d40e084..c57d26619817 100644
--- a/fs/xfs/libxfs/xfs_errortag.h
+++ b/fs/xfs/libxfs/xfs_errortag.h
@@ -65,7 +65,8 @@
 #define XFS_ERRTAG_WRITE_DELAY_MS			43
 #define XFS_ERRTAG_EXCHMAPS_FINISH_ONE			44
 #define XFS_ERRTAG_METAFILE_RESV_CRITICAL		45
-#define XFS_ERRTAG_MAX					46
+#define XFS_ERRTAG_AG_ALLOC_SKIP			46
+#define XFS_ERRTAG_MAX					47
 
 /*
  * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -115,5 +116,6 @@
 #define XFS_RANDOM_WRITE_DELAY_MS			3000
 #define XFS_RANDOM_EXCHMAPS_FINISH_ONE			1
 #define XFS_RANDOM_METAFILE_RESV_CRITICAL		4
+#define XFS_RANDOM_AG_ALLOC_SKIP			2
 
 #endif /* __XFS_ERRORTAG_H_ */
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 62ac6debcb5e..f1222e4e8c5f 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -64,6 +64,7 @@ static unsigned int xfs_errortag_random_default[] = {
 	XFS_RANDOM_WRITE_DELAY_MS,
 	XFS_RANDOM_EXCHMAPS_FINISH_ONE,
 	XFS_RANDOM_METAFILE_RESV_CRITICAL,
+	XFS_RANDOM_AG_ALLOC_SKIP,
 };
 
 struct xfs_errortag_attr {
@@ -187,6 +188,7 @@ XFS_ERRORTAG_ATTR_RW(wb_delay_ms,	XFS_ERRTAG_WB_DELAY_MS);
 XFS_ERRORTAG_ATTR_RW(write_delay_ms,	XFS_ERRTAG_WRITE_DELAY_MS);
 XFS_ERRORTAG_ATTR_RW(exchmaps_finish_one, XFS_ERRTAG_EXCHMAPS_FINISH_ONE);
 XFS_ERRORTAG_ATTR_RW(metafile_resv_crit, XFS_ERRTAG_METAFILE_RESV_CRITICAL);
+__XFS_ERRORTAG_ATTR_RW(ag_alloc_skip,	XFS_ERRTAG_AG_ALLOC_SKIP,	true);
 
 static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(noerror),
@@ -234,6 +236,7 @@ static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(write_delay_ms),
 	XFS_ERRORTAG_ATTR_LIST(exchmaps_finish_one),
 	XFS_ERRORTAG_ATTR_LIST(metafile_resv_crit),
+	XFS_ERRORTAG_ATTR_LIST(ag_alloc_skip),
 	NULL,
 };
 ATTRIBUTE_GROUPS(xfs_errortag);
-- 
2.49.0

[PATCH 3/3] xfs: convert sparse inode alloc debug fallback to errortag

[Brian Foster]

XFS includes DEBUG logic to randomly fall into sparse inode chunk
allocation. This is to facilitate code coverage testing as sparse
allocation is really only required once an AG has insufficiently
large free extents to allocate a full inode chunk.

Similar to for the extent allocation path, convert this into an
errortag that is enabled by default to maintain current DEBUG mode
behavior.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_errortag.h |  4 +++-
 fs/xfs/libxfs/xfs_ialloc.c   | 14 +++++---------
 fs/xfs/xfs_error.c           |  3 +++
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
index c57d26619817..3a1404a24584 100644
--- a/fs/xfs/libxfs/xfs_errortag.h
+++ b/fs/xfs/libxfs/xfs_errortag.h
@@ -66,7 +66,8 @@
 #define XFS_ERRTAG_EXCHMAPS_FINISH_ONE			44
 #define XFS_ERRTAG_METAFILE_RESV_CRITICAL		45
 #define XFS_ERRTAG_AG_ALLOC_SKIP			46
-#define XFS_ERRTAG_MAX					47
+#define XFS_ERRTAG_SPARSE_IALLOC			47
+#define XFS_ERRTAG_MAX					48
 
 /*
  * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -117,5 +118,6 @@
 #define XFS_RANDOM_EXCHMAPS_FINISH_ONE			1
 #define XFS_RANDOM_METAFILE_RESV_CRITICAL		4
 #define XFS_RANDOM_AG_ALLOC_SKIP			2
+#define XFS_RANDOM_SPARSE_IALLOC			2
 
 #endif /* __XFS_ERRORTAG_H_ */
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 0c47b5c6ca7d..9901e09e7a5d 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -707,7 +707,6 @@ xfs_ialloc_ag_alloc(
 	struct xfs_inobt_rec_incore rec;
 	struct xfs_ino_geometry	*igeo = M_IGEO(tp->t_mountp);
 	uint16_t		allocmask = (uint16_t) -1;
-	int			do_sparse = 0;
 
 	memset(&args, 0, sizeof(args));
 	args.tp = tp;
@@ -716,13 +715,6 @@ xfs_ialloc_ag_alloc(
 	args.oinfo = XFS_RMAP_OINFO_INODES;
 	args.pag = pag;
 
-#ifdef DEBUG
-	/* randomly do sparse inode allocations */
-	if (xfs_has_sparseinodes(tp->t_mountp) &&
-	    igeo->ialloc_min_blks < igeo->ialloc_blks)
-		do_sparse = get_random_u32_below(2);
-#endif
-
 	/*
 	 * Locking will ensure that we don't have two callers in here
 	 * at one time.
@@ -742,8 +734,12 @@ xfs_ialloc_ag_alloc(
 	newino = be32_to_cpu(agi->agi_newino);
 	args.agbno = XFS_AGINO_TO_AGBNO(args.mp, newino) +
 		     igeo->ialloc_blks;
-	if (do_sparse)
+
+	if (xfs_has_sparseinodes(args.mp) &&
+	    igeo->ialloc_min_blks < igeo->ialloc_blks &&
+	    XFS_TEST_ERROR(false, args.mp, XFS_ERRTAG_SPARSE_IALLOC))
 		goto sparse_alloc;
+
 	if (likely(newino != NULLAGINO &&
 		  (args.agbno < be32_to_cpu(agi->agi_length)))) {
 		args.prod = 1;
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index f1222e4e8c5f..480fabfadf39 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -65,6 +65,7 @@ static unsigned int xfs_errortag_random_default[] = {
 	XFS_RANDOM_EXCHMAPS_FINISH_ONE,
 	XFS_RANDOM_METAFILE_RESV_CRITICAL,
 	XFS_RANDOM_AG_ALLOC_SKIP,
+	XFS_RANDOM_SPARSE_IALLOC,
 };
 
 struct xfs_errortag_attr {
@@ -189,6 +190,7 @@ XFS_ERRORTAG_ATTR_RW(write_delay_ms,	XFS_ERRTAG_WRITE_DELAY_MS);
 XFS_ERRORTAG_ATTR_RW(exchmaps_finish_one, XFS_ERRTAG_EXCHMAPS_FINISH_ONE);
 XFS_ERRORTAG_ATTR_RW(metafile_resv_crit, XFS_ERRTAG_METAFILE_RESV_CRITICAL);
 __XFS_ERRORTAG_ATTR_RW(ag_alloc_skip,	XFS_ERRTAG_AG_ALLOC_SKIP,	true);
+__XFS_ERRORTAG_ATTR_RW(sparse_ialloc,	XFS_ERRTAG_SPARSE_IALLOC,	true);
 
 static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(noerror),
@@ -237,6 +239,7 @@ static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(exchmaps_finish_one),
 	XFS_ERRORTAG_ATTR_LIST(metafile_resv_crit),
 	XFS_ERRORTAG_ATTR_LIST(ag_alloc_skip),
+	XFS_ERRORTAG_ATTR_LIST(sparse_ialloc),
 	NULL,
 };
 ATTRIBUTE_GROUPS(xfs_errortag);
-- 
2.49.0