Re: [PATCH v2 4/5] xfs: only flush when COW fork blocks overlap data fork holes

[Brian Foster <bfoster@redhat.com>]

On Tue, Feb 17, 2026 at 08:36:50PM +0530, Nirjhar Roy (IBM) wrote:
> On Thu, 2026-01-29 at 10:50 -0500, Brian Foster wrote:
> > The zero range hole mapping flush case has been lifted from iomap
> > into XFS. Now that we have more mapping context available from the
> > ->iomap_begin() handler, we can isolate the flush further to when we
> > know a hole is fronted by COW blocks.
> > 
> > Rather than purely rely on pagecache dirty state, explicitly check
> > for the case where a range is a hole in both forks. Otherwise trim
> > to the range where there does happen to be overlap and use that for
> > the pagecache writeback check. This might prevent some spurious
> > zeroing, but more importantly makes it easier to remove the flush
> > entirely.
> > 
> > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > ---
> >  fs/xfs/xfs_iomap.c | 36 ++++++++++++++++++++++++++++++------
> >  1 file changed, 30 insertions(+), 6 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> > index 0edab7af4a10..0e82b4ec8264 100644
> > --- a/fs/xfs/xfs_iomap.c
> > +++ b/fs/xfs/xfs_iomap.c
> > @@ -1760,10 +1760,12 @@ xfs_buffered_write_iomap_begin(
> >  {
> >  	struct iomap_iter	*iter = container_of(iomap, struct iomap_iter,
> >  						     iomap);
> > +	struct address_space	*mapping = inode->i_mapping;
> >  	struct xfs_inode	*ip = XFS_I(inode);
> >  	struct xfs_mount	*mp = ip->i_mount;
> >  	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
> >  	xfs_fileoff_t		end_fsb = xfs_iomap_end_fsb(mp, offset, count);
> > +	xfs_fileoff_t		cow_fsb = NULLFILEOFF;
> >  	struct xfs_bmbt_irec	imap, cmap;
> >  	struct xfs_iext_cursor	icur, ccur;
> >  	xfs_fsblock_t		prealloc_blocks = 0;
> > @@ -1831,6 +1833,8 @@ xfs_buffered_write_iomap_begin(
> >  		}
> >  		cow_eof = !xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb,
> >  				&ccur, &cmap);
> > +		if (!cow_eof)
> > +			cow_fsb = cmap.br_startoff;
> >  	}
> >  
> >  	/* We never need to allocate blocks for unsharing a hole. */
> > @@ -1845,17 +1849,37 @@ xfs_buffered_write_iomap_begin(
> >  	 * writeback to remap pending blocks and restart the lookup.
> >  	 */
> >  	if ((flags & IOMAP_ZERO) && imap.br_startoff > offset_fsb) {
> > -		if (filemap_range_needs_writeback(inode->i_mapping, offset,
> > -						  offset + count - 1)) {
> > +		loff_t start, end;
> 
> Nit: Tab between data type and identifier?
> 

Sure.

> > +
> > +		imap.br_blockcount = imap.br_startoff - offset_fsb;
> > +		imap.br_startoff = offset_fsb;
> > +		imap.br_startblock = HOLESTARTBLOCK;
> > +		imap.br_state = XFS_EXT_NORM;
> > +
> > +		if (cow_fsb == NULLFILEOFF) {
> > +			goto found_imap;
> > +		} else if (cow_fsb > offset_fsb) {
> > +			xfs_trim_extent(&imap, offset_fsb,
> > +					cow_fsb - offset_fsb);
> > +			goto found_imap;
> > +		}
> > +
> > +		/* COW fork blocks overlap the hole */
> > +		xfs_trim_extent(&imap, offset_fsb,
> > +			    cmap.br_startoff + cmap.br_blockcount - offset_fsb);
> > +		start = XFS_FSB_TO_B(mp, imap.br_startoff);
> > +		end = XFS_FSB_TO_B(mp,
> > +				   imap.br_startoff + imap.br_blockcount) - 1;
> 
> So, we are including the bytes in the block number (imap.br_startoff + imap.br_blockcount - 1)th,
> right? That is why a -1 after XFS_FSB_TO_B()? 

Not sure I follow what you mean by the "bytes in the block number"
phrasing..? Anyways, the XFS_FSB_TO_B() here should return the starting
byte offset of the first block beyond the range (exclusive). The -1
changes that to the last byte offset of the range we're interested in
(inclusive), which I believe is what the filemap api wants..

Brian

> --NR
> > +		if (filemap_range_needs_writeback(mapping, start, end)) {
> >  			xfs_iunlock(ip, lockmode);
> > -			error = filemap_write_and_wait_range(inode->i_mapping,
> > -						offset, offset + count - 1);
> > +			error = filemap_write_and_wait_range(mapping, start,
> > +							     end);
> >  			if (error)
> >  				return error;
> >  			goto restart;
> >  		}
> > -		xfs_hole_to_iomap(ip, iomap, offset_fsb, imap.br_startoff);
> > -		goto out_unlock;
> > +
> > +		goto found_imap;
> >  	}
> >  
> >  	/*
>