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

Brian Foster
2025-11-06T15:49:00Z

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