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

[Brian Foster <bfoster@redhat.com>]

On Mon, Jul 14, 2025 at 10:24:44PM -0700, Darrick J. Wong wrote:
> 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/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 ?
> 

~/linux$ git grep FALLOC_FL_WRITE_ZEROES
~/linux$ 

IIRC write zeroes is intended to expose fast hardware (physical) zeroing
(i.e. zeroed written extents)..? If so, I suppose you could consider
this a fallback of sorts. I'm not sure what write zeroes is expected to
do in the unwritten extent case, whereas iomap zero range is happy to
skip those mappings unless they're already dirty in pagecache.

Regardless, the purpose of this patch is not to add support for physical
zeroing, but rather to increase test coverage for the additional code on
debug kernels because the production use case tends to be more limited.
This could easily be moved/applied to write zeroes if it makes sense in
the future and test infra grows support for it.

Brian

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