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

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

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