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

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

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?

> +
> +		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()? 
--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;
>  	}
>  
>  	/*