Re: [PATCH 6/6] xfs: replace zero range flush with folio batch

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

On Thu, Oct 16, 2025 at 03:03:03PM -0400, Brian Foster wrote:
> Now that the zero range pagecache flush is purely isolated to
> providing zeroing correctness in this case, we can remove it and
> replace it with the folio batch mechanism that is used for handling
> unwritten extents.
> 
> This is still slightly odd in that XFS reports a hole vs. a mapping
> that reflects the COW fork extents, but that has always been the
> case in this situation and so a separate issue. We drop the iomap
> warning that assumes the folio batch is always associated with
> unwritten mappings, but this is mainly a development assertion as
> otherwise the core iomap fbatch code doesn't care much about the
> mapping type if it's handed the set of folios to process.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/iomap/buffered-io.c |  4 ----
>  fs/xfs/xfs_iomap.c     | 16 ++++------------
>  2 files changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index d6de689374c3..7bc4b8d090ee 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1534,10 +1534,6 @@ iomap_zero_range(struct inode *inode, loff_t pos, loff_t len, bool *did_zero,
>  	while ((ret = iomap_iter(&iter, ops)) > 0) {
>  		const struct iomap *srcmap = iomap_iter_srcmap(&iter);
>  
> -		if (WARN_ON_ONCE((iter.iomap.flags & IOMAP_F_FOLIO_BATCH) &&
> -				 srcmap->type != IOMAP_UNWRITTEN))
> -			return -EIO;
> -
>  		if (!(iter.iomap.flags & IOMAP_F_FOLIO_BATCH) &&
>  		    (srcmap->type == IOMAP_HOLE ||
>  		     srcmap->type == IOMAP_UNWRITTEN)) {
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 29f1462819fa..5a845a0ded79 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1704,7 +1704,6 @@ 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);
> @@ -1736,7 +1735,6 @@ xfs_buffered_write_iomap_begin(
>  	if (error)
>  		return error;
>  
> -restart:
>  	error = xfs_ilock_for_iomap(ip, flags, &lockmode);
>  	if (error)
>  		return error;
> @@ -1812,16 +1810,10 @@ xfs_buffered_write_iomap_begin(
>  		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;
> -		if (filemap_range_needs_writeback(mapping, start, end)) {
> -			xfs_iunlock(ip, lockmode);
> -			error = filemap_write_and_wait_range(mapping, start,
> -							     end);
> -			if (error)
> -				return error;
> -			goto restart;
> -		}
> +		end = XFS_FSB_TO_B(mp, imap.br_startoff + imap.br_blockcount);
> +		iomap_flags |= iomap_fill_dirty_folios(iter, &start, end);
> +		xfs_trim_extent(&imap, offset_fsb,
> +				XFS_B_TO_FSB(mp, start) - offset_fsb);

Hrm, ok.  This replaces the pagecache flush with passing in folios and
letting iomap zero the folios regardless of whatever's in the mapping.
That seems to me like a reasonable way to solve the immediate problem
without the huge reengineering ->iomap_begin project.

The changes here mostly look ok to me, though I wonder how well this all
meshes with all the other iomap work headed to 6.19...

--D

>  
>  		goto found_imap;
>  	}
> -- 
> 2.51.0
> 
>