Re: [PATCH 4/6] xfs: look up cow fork extent earlier for buffered iomap_begin

Darrick J. Wong
2025-11-05T22:26:08Z

On Thu, Oct 16, 2025 at 03:03:01PM -0400, Brian Foster wrote:
> To further isolate the need for flushing for zero range, we need to
> know whether a hole in the data fork is fronted by blocks in the COW
> fork or not. COW fork lookup currently occurs further down in the
> function, after the zero range case is handled.
>
> As a preparation step, lift the COW fork extent lookup to earlier in
> the function, at the same time as the data fork lookup. Only the
> lookup logic is lifted. The COW fork branch/reporting logic remains
> as is to avoid any observable behavior change from an iomap
> reporting perspective.
>
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Looks fine to me,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> fs/xfs/xfs_iomap.c | 46 +++++++++++++++++++++++++---------------------
> 1 file changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index b84c94558cc9..ba5697d8b8fd 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1753,14 +1753,29 @@ xfs_buffered_write_iomap_begin(
> goto out_unlock;
>
> /*
> - * Search the data fork first to look up our source mapping. We
> - * always need the data fork map, as we have to return it to the
> - * iomap code so that the higher level write code can read data in to
> - * perform read-modify-write cycles for unaligned writes.
> + * Search the data fork first to look up our source mapping. We always
> + * need the data fork map, as we have to return it to the iomap code so
> + * that the higher level write code can read data in to perform
> + * read-modify-write cycles for unaligned writes.
> + *
> + * Then search the COW fork extent list even if we did not find a data
> + * fork extent. This serves two purposes: first this implements the
> + * speculative preallocation using cowextsize, so that we also unshare
> + * block adjacent to shared blocks instead of just the shared blocks
> + * themselves. Second the lookup in the extent list is generally faster
> + * than going out to the shared extent tree.
> */
> eof = !xfs_iext_lookup_extent(ip, &ip->i_df, offset_fsb, &icur, &imap);
> if (eof)
> imap.br_startoff = end_fsb; /* fake hole until the end */
> + if (xfs_is_cow_inode(ip)) {
> + if (!ip->i_cowfp) {
> + ASSERT(!xfs_is_reflink_inode(ip));
> + xfs_ifork_init_cow(ip);
> + }
> + cow_eof = !xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb,
> + &ccur, &cmap);
> + }
>
> /* We never need to allocate blocks for unsharing a hole. */
> if ((flags & IOMAP_UNSHARE) && imap.br_startoff > offset_fsb) {
> @@ -1827,24 +1842,13 @@ xfs_buffered_write_iomap_begin(
> }
>
> /*
> - * Search the COW fork extent list even if we did not find a data fork
> - * extent. This serves two purposes: first this implements the
> - * speculative preallocation using cowextsize, so that we also unshare
> - * block adjacent to shared blocks instead of just the shared blocks
> - * themselves. Second the lookup in the extent list is generally faster
> - * than going out to the shared extent tree.
> + * Now that we've handled any operation specific special cases, at this
> + * point we can report a COW mapping if found.
> */
> - if (xfs_is_cow_inode(ip)) {
> - if (!ip->i_cowfp) {
> - ASSERT(!xfs_is_reflink_inode(ip));
> - xfs_ifork_init_cow(ip);
> - }
> - cow_eof = !xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb,
> - &ccur, &cmap);
> - if (!cow_eof && cmap.br_startoff <= offset_fsb) {
> - trace_xfs_reflink_cow_found(ip, &cmap);
> - goto found_cow;
> - }
> + if (xfs_is_cow_inode(ip) &&
> + !cow_eof && cmap.br_startoff <= offset_fsb) {
> + trace_xfs_reflink_cow_found(ip, &cmap);
> + goto found_cow;
> }
>
> if (imap.br_startoff <= offset_fsb) {
> --
> 2.51.0
>
>