Re: [PATCH v3 1/8] xfs: fix iomap hole map reporting for zoned zero range

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

On Mon, Mar 09, 2026 at 02:18:56PM -0400, Brian Foster wrote:
> On Mon, Mar 09, 2026 at 10:11:00AM -0700, Darrick J. Wong wrote:
> > On Mon, Mar 09, 2026 at 09:44:59AM -0400, Brian Foster wrote:
> > > The hole mapping logic for zero range in zoned mode is not quite
> > > correct. It currently reports a hole whenever one exists in the data
> > > fork. If the first write to a sparse range has completed and not yet
> > > written back, the blocks exist in the COW fork as delalloc until
> > > writeback completes, at which point they are allocated and mapped
> > > into the data fork. If a zero range occurs on a range that has not
> > > yet populated the data fork, we will incorrectly report it as a
> > > hole.
> > > 
> > > Note that this currently functions correctly because we are bailed
> > > out by the pagecache flush in iomap_zero_range(). If a hole or
> > > unwritten mapping is reported with dirty pagecache, it assumes there
> > > is pending data, flushes to induce any pending block
> > > allocations/remaps, and retries the lookup. We want to remove this
> > > hack from iomap, however, so update iomap_begin() to only report a
> > > hole for zeroing when one exists in both forks.
> > > 
> > > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > > ---
> > >  fs/xfs/xfs_iomap.c | 18 ++++++++++--------
> > >  1 file changed, 10 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> > > index be86d43044df..8c3469d2c73e 100644
> > > --- a/fs/xfs/xfs_iomap.c
> > > +++ b/fs/xfs/xfs_iomap.c
> > > @@ -1651,14 +1651,6 @@ xfs_zoned_buffered_write_iomap_begin(
> > >  				&smap))
> > >  			smap.br_startoff = end_fsb; /* fake hole until EOF */
> > >  		if (smap.br_startoff > offset_fsb) {
> > > -			/*
> > > -			 * We never need to allocate blocks for zeroing a hole.
> > > -			 */
> > > -			if (flags & IOMAP_ZERO) {
> > > -				xfs_hole_to_iomap(ip, iomap, offset_fsb,
> > > -						smap.br_startoff);
> > > -				goto out_unlock;
> > > -			}
> > >  			end_fsb = min(end_fsb, smap.br_startoff);
> > >  		} else {
> > >  			end_fsb = min(end_fsb,
> > > @@ -1690,6 +1682,16 @@ xfs_zoned_buffered_write_iomap_begin(
> > >  	count_fsb = min3(end_fsb - offset_fsb, XFS_MAX_BMBT_EXTLEN,
> > >  			 XFS_B_TO_FSB(mp, 1024 * PAGE_SIZE));
> > >  
> > > +	/*
> > > +	 * When zeroing, don't allocate blocks for holes as they are already
> > > +	 * zeroes, but we need to ensure that no extents exist in both the data
> > > +	 * and COW fork to ensure this really is a hole.
> > 
> > But where is the cow fork check?  iomap_iter initializes srcmap to
> > IOMAP_HOLE, so if we end up here on an IOMAP_ZERO then we've scanned the
> > data fork and found no mapping.  But then we jump to out_unlock, which
> > means we don't actually look at ip->cowfp.
> > 
> 
> The COW fork is checked a few lines up from where this is added. If it
> finds blocks it exits out, if not it falls into this codepath for
> allocation into the fork (implying a hole).

Ah silly me falling off the end of the (diff) context window
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> 
> Brian
> 
> > <confused>
> > 
> > --D
> > 
> > > +	 */
> > > +	if ((flags & IOMAP_ZERO) && srcmap->type == IOMAP_HOLE) {
> > > +		xfs_hole_to_iomap(ip, iomap, offset_fsb, end_fsb);
> > > +		goto out_unlock;
> > > +	}
> > > +
> > >  	/*
> > >  	 * The block reservation is supposed to cover all blocks that the
> > >  	 * operation could possible write, but there is a nasty corner case
> > > -- 
> > > 2.52.0
> > > 
> > > 
> > 
> 
>