linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: linux-fsdevel@vger.kernel.org
Cc: linux-xfs@vger.kernel.org, linux-mm@kvack.org
Subject: [PATCH RFC 7/7] xfs: error tag to force zeroing on debug kernels
Date: Thu,  5 Jun 2025 13:33:57 -0400	[thread overview]
Message-ID: <20250605173357.579720-8-bfoster@redhat.com> (raw)
In-Reply-To: <20250605173357.579720-1-bfoster@redhat.com>

iomap_zero_range() has to cover various corner cases that are
difficult to test on production kernels because it is used in fairly
limited use cases. For example, it is currently only used by XFS and
mostly only in partial block zeroing cases.

While it's possible to test most of these functional cases, we can
provide more robust test coverage by co-opting fallocate zero range
to invoke zeroing of the entire range instead of the more efficient
block punch/allocate sequence. Add an errortag to occasionally
invoke forced zeroing.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---

Hi all,

My original test approach involved a hack that redirected fallocate zero
range to iomap. The easiest way to incorporate something like that in
XFS would probably be via a randomized debug branch in the same path
(i.e. similar to what is done in the block/inode allocation paths), but
I find it a little annoying that there's currently no great way to run a
full fstests run with certain error tags enabled.

So for that I was playing around with some fstests hacks to hook into
the device mount paths and enable certain errortags immediately on each
mount. Obviously this implicitly depends on XFS_DEBUG plus the specific
errortag support, but otherwise I'd probably propose something simple
like to enable or disable the whole thing with an XFS_ERRORS_ON=1 type
setting or some such.

So that's the context for this patch. I'm curious if there are any
thoughts..? Another simple option might be to enable certain errortags
by default on XFS_DEBUG, or with XFS_DEBUG plus some new kernel config
option (i.e. XFS_DEBUG_ERRORTAGS). That would be less code in fstests
and perhaps facilitate external testing, but we'd still need to track
which tags get enabled by default in that mode. OTOH, if you consider
the couple or so open coded get_random_u32_below(2) branches we do have,
these are in theory just errortags that happen to be on by default on
XFS_DEBUG kernels (without the log noise). Thoughts?

Brian

 fs/xfs/libxfs/xfs_errortag.h |  4 +++-
 fs/xfs/xfs_error.c           |  3 +++
 fs/xfs/xfs_file.c            | 21 +++++++++++++++------
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
index a53c5d40e084..33ca3fc2ca88 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_FORCE_ZERO_RANGE			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_FORCE_ZERO_RANGE			4
 
 #endif /* __XFS_ERRORTAG_H_ */
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index dbd87e137694..00c0c391c329 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_FORCE_ZERO_RANGE,
 };
 
 struct xfs_errortag_attr {
@@ -183,6 +184,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(force_zero_range, XFS_ERRTAG_FORCE_ZERO_RANGE);
 
 static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(noerror),
@@ -230,6 +232,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(force_zero_range),
 	NULL,
 };
 ATTRIBUTE_GROUPS(xfs_errortag);
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 48254a72071b..88ce754ef226 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -27,6 +27,8 @@
 #include "xfs_file.h"
 #include "xfs_aops.h"
 #include "xfs_zone_alloc.h"
+#include "xfs_error.h"
+#include "xfs_errortag.h"
 
 #include <linux/dax.h>
 #include <linux/falloc.h>
@@ -1269,13 +1271,20 @@ xfs_falloc_zero_range(
 	if (error)
 		return error;
 
-	error = xfs_free_file_space(XFS_I(inode), offset, len, ac);
-	if (error)
-		return error;
+	/* randomly force zeroing to exercise zero range */
+	if (XFS_TEST_ERROR(false, XFS_I(inode)->i_mount,
+			   XFS_ERRTAG_FORCE_ZERO_RANGE)) {
+		error = xfs_zero_range(XFS_I(inode), offset, len, ac, NULL);
+	} else {
+		error = xfs_free_file_space(XFS_I(inode), offset, len, ac);
+		if (error)
+			return error;
 
-	len = round_up(offset + len, blksize) - round_down(offset, blksize);
-	offset = round_down(offset, blksize);
-	error = xfs_alloc_file_space(XFS_I(inode), offset, len);
+		len = round_up(offset + len, blksize) -
+			round_down(offset, blksize);
+		offset = round_down(offset, blksize);
+		error = xfs_alloc_file_space(XFS_I(inode), offset, len);
+	}
 	if (error)
 		return error;
 	return xfs_falloc_setsize(file, new_size);
-- 
2.49.0


  parent reply	other threads:[~2025-06-05 17:30 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-05 17:33 [PATCH 0/7] iomap: zero range folio batch support Brian Foster
2025-06-05 17:33 ` [PATCH 1/7] iomap: move pos+len BUG_ON() to after folio lookup Brian Foster
2025-06-09 16:16   ` Darrick J. Wong
2025-06-10  4:20     ` Christoph Hellwig
2025-06-10 12:16       ` Brian Foster
2025-06-05 17:33 ` [PATCH 2/7] filemap: add helper to look up dirty folios in a range Brian Foster
2025-06-09 15:48   ` Darrick J. Wong
2025-06-10  4:21     ` Christoph Hellwig
2025-06-10 12:17     ` Brian Foster
2025-06-10  4:22   ` Christoph Hellwig
2025-06-05 17:33 ` [PATCH 3/7] iomap: optional zero range dirty folio processing Brian Foster
2025-06-09 16:04   ` Darrick J. Wong
2025-06-10  4:27     ` Christoph Hellwig
2025-06-10 12:21       ` Brian Foster
2025-06-10 12:21     ` Brian Foster
2025-06-10 13:29       ` Christoph Hellwig
2025-06-10 14:19         ` Brian Foster
2025-06-11  3:54           ` Christoph Hellwig
2025-06-10 14:55       ` Darrick J. Wong
2025-06-11  3:55         ` Christoph Hellwig
2025-06-12  4:06           ` Darrick J. Wong
2025-06-10  4:27   ` Christoph Hellwig
2025-06-05 17:33 ` [PATCH 4/7] xfs: always trim mapping to requested range for zero range Brian Foster
2025-06-09 16:07   ` Darrick J. Wong
2025-06-05 17:33 ` [PATCH 5/7] xfs: fill dirty folios on zero range of unwritten mappings Brian Foster
2025-06-06  2:02   ` kernel test robot
2025-06-06 15:20     ` Brian Foster
2025-06-09 16:12   ` Darrick J. Wong
2025-06-10  4:31     ` Christoph Hellwig
2025-06-10 12:24     ` Brian Foster
2025-07-02 18:50       ` Darrick J. Wong
2025-06-05 17:33 ` [PATCH 6/7] iomap: remove old partial eof zeroing optimization Brian Foster
2025-06-10  4:32   ` Christoph Hellwig
2025-06-05 17:33 ` Brian Foster [this message]
2025-06-10  4:33   ` [PATCH RFC 7/7] xfs: error tag to force zeroing on debug kernels Christoph Hellwig
2025-06-10 12:26     ` Brian Foster
2025-06-10 13:30       ` Christoph Hellwig
2025-06-10 14:20         ` Brian Foster
2025-06-10 19:12           ` Brian Foster
2025-06-11  3:56             ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250605173357.579720-8-bfoster@redhat.com \
    --to=bfoster@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).