Re: [PATCH 03/12] xfs: split out a lower-level xfs_buf_get_map helper from xfs_find_get_buf

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

On Wed, Jul 15, 2026 at 04:50:56PM +0200, Christoph Hellwig wrote:
> xfs_buf_get_map is currently reused to implement xfs_buf_read_map and
> xfs_buf_readahead_map.  This causes double accounting of buf_get stat
> and leads to some ugly overload of the flags.
> 
> Split out a slightly lower-level xfs_find_get_buf helper and use that to
> implement xfs_buf_get_map, xfs_buf_read_map and xfs_buf_readahead_map.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 41 +++++++++++++++++++++++++++++------------
>  1 file changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index e56d4b8b0771..2cf359b4c446 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -514,8 +514,8 @@ xfs_buf_find_insert(
>   * cache hits, as metadata intensive workloads will see 3 orders of magnitude
>   * more hits than misses.
>   */
> -int
> -xfs_buf_get_map(
> +static int
> +xfs_find_get_buf(

I like the idea of factoring this out, but I can't tell from the names
what's the difference between xfs_find_get_buf and xfs_buf_get_map.

I might have called the inner function __xfs_buf_get or something.
Dunno, don't care to bikeshed this.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  	struct xfs_buftarg	*btp,
>  	struct xfs_buf_map	*map,
>  	int			nmaps,
> @@ -552,16 +552,33 @@ xfs_buf_get_map(
>  		return error;
>  	}
>  
> +	*bpp = bp;
> +	return 0;
> +}
> +
> +int
> +xfs_buf_get_map(
> +	struct xfs_buftarg	*btp,
> +	struct xfs_buf_map	*map,
> +	int			nmaps,
> +	xfs_buf_flags_t		flags,
> +	struct xfs_buf		**bpp)
> +{
> +	int			error;
> +
> +	ASSERT(!(flags & ~(XBF_TRYLOCK | XBF_INCORE | XBF_LIVESCAN)));
> +	ASSERT(!(flags & XBF_LIVESCAN) || (flags & XBF_INCORE));
> +
>  	/*
> -	 * Clear b_error if this is a lookup from a caller that doesn't expect
> -	 * valid data to be found in the buffer.
> +	 * Zero the buffer and clear b_error as xfs_buf_get_map callers don't
> +	 * expect valid data to be found in the buffer.
>  	 */
> -	if (!(flags & XBF_READ))
> -		xfs_buf_ioerror(bp, 0);
> -
> +	error = xfs_find_get_buf(btp, map, nmaps, flags, bpp);
> +	if (error)
> +		return error;
>  	XFS_STATS_INC(btp->bt_mount, xb_get);
> -	trace_xfs_buf_get(bp, flags, _RET_IP_);
> -	*bpp = bp;
> +	trace_xfs_buf_get(*bpp, flags, _RET_IP_);
> +	xfs_buf_ioerror(*bpp, 0);
>  	return 0;
>  }
>  
> @@ -625,12 +642,12 @@ xfs_buf_read_map(
>  	struct xfs_buf		*bp;
>  	int			error;
>  
> -	ASSERT(!(flags & (XBF_WRITE | XBF_ASYNC | XBF_READ_AHEAD)));
> +	ASSERT(!(flags & ~XBF_TRYLOCK));
>  
>  	flags |= XBF_READ;
>  	*bpp = NULL;
>  
> -	error = xfs_buf_get_map(target, map, nmaps, flags, &bp);
> +	error = xfs_find_get_buf(target, map, nmaps, flags, &bp);
>  	if (error)
>  		return error;
>  
> @@ -706,7 +723,7 @@ xfs_buf_readahead_map(
>  	if (xfs_buftarg_is_mem(target))
>  		return;
>  
> -	if (xfs_buf_get_map(target, map, nmaps, flags | XBF_TRYLOCK, &bp))
> +	if (xfs_find_get_buf(target, map, nmaps, flags | XBF_TRYLOCK, &bp))
>  		return;
>  	trace_xfs_buf_readahead(bp, 0, _RET_IP_);
>  
> -- 
> 2.53.0
> 
>