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

[Brian Foster <bfoster@redhat.com>]

On Wed, Nov 05, 2025 at 02:37:15PM -0800, Darrick J. Wong wrote:
> 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.
> 

Yeah.. modulo a potential clean up to have this actually report the COW
mapping which might be incrementally cleaner. I'll take a closer look at
that when I get back to this..

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

The batch stuff is already in -next I believe. It might be good to get
the first patch in sooner, but I don't think the rest should be too bad
provided it stays isolated to XFS and zero range.

Brian

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