Re: [PATCH v3 7/7] xfs: error tag to force zeroing on debug kernels

Darrick J. Wong
2025-07-15T05:24:48Z

On Mon, Jul 14, 2025 at 04:41:22PM -0400, Brian Foster wrote:
> 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>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_errortag.h | 4 +++-
> fs/xfs/xfs_error.c | 3 +++
> fs/xfs/xfs_file.c | 26 ++++++++++++++++++++------
> 3 files changed, 26 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 0b41b18debf3..c865f9555b77 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,25 @@ xfs_falloc_zero_range(
> if (error)
> return error;
>
> - error = xfs_free_file_space(XFS_I(inode), offset, len, ac);
> - if (error)
> - return error;
> + /*
> + * Zero range implements a full zeroing mechanism but is only used in
> + * limited situations. It is more efficient to allocate unwritten
> + * extents than to perform zeroing here, so use an errortag to randomly
> + * force zeroing on DEBUG kernels for added test coverage.
> + */
> + 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);

Isn't this basically the ultra slow version fallback version of
FALLOC_FL_WRITE_ZEROES ?

--D

> + } 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.50.0
>
>