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

["Darrick J. Wong" <djwong@kernel.org>]

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
> 
>