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

Darrick J. Wong
2025-11-05T22:37:16Z

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