Re: [PATCH 10/12] xfs: merge xfs_buf_reverify into xfs_buf_read_map

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

On Wed, Jul 15, 2026 at 04:51:03PM +0200, Christoph Hellwig wrote:
> xfs_buf_read_map is the only caller of xfs_buf_reverify that is left.
> Merge it into that so that the comments can be moved closer to the
> logic, and redundant asserts can be removed.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 79 ++++++++++++++++++++++--------------------------
>  1 file changed, 36 insertions(+), 43 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 21c21491d72e..be3b25ccbea6 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -626,41 +626,6 @@ _xfs_buf_read(
>  	return xfs_buf_iowait(bp);
>  }
>  
> -/*
> - * Reverify a buffer found in cache without an attached ->b_ops.
> - *
> - * If the caller passed an ops structure and the buffer doesn't have ops
> - * assigned, set the ops and use it to verify the contents. If verification
> - * fails, clear XBF_DONE. We assume the buffer has no recorded errors and is
> - * already in XBF_DONE state on entry.
> - *
> - * Under normal operations, every in-core buffer is verified on read I/O
> - * completion. There are two scenarios that can lead to in-core buffers without
> - * an assigned ->b_ops. The first is during log recovery of buffers on a V4
> - * filesystem, though these buffers are purged at the end of recovery. The
> - * other is online repair, which intentionally reads with a NULL buffer ops to
> - * run several verifiers across an in-core buffer in order to establish buffer
> - * type.  If repair can't establish that, the buffer will be left in memory
> - * with NULL buffer ops.
> - */
> -static int
> -xfs_buf_reverify(
> -	struct xfs_buf		*bp,
> -	const struct xfs_buf_ops *ops)
> -{
> -	ASSERT(bp->b_flags & XBF_DONE);
> -	ASSERT(bp->b_error == 0);
> -
> -	if (!ops || bp->b_ops)
> -		return 0;
> -
> -	bp->b_ops = ops;
> -	bp->b_ops->verify_read(bp);
> -	if (bp->b_error)
> -		xfs_buf_clear_flags(bp, XBF_DONE);
> -	return bp->b_error;
> -}
> -
>  int
>  xfs_buf_read_map(
>  	struct xfs_buftarg	*target,
> @@ -685,18 +650,46 @@ xfs_buf_read_map(
>  
>  	trace_xfs_buf_read(bp, flags, _RET_IP_);
>  
> -	if (!(bp->b_flags & XBF_DONE)) {
> +	if (bp->b_flags & XBF_DONE) {
> +		ASSERT(bp->b_error == 0);
> +
> +		/*
> +		 * If the caller passed an ops structure and the buffer doesn't
> +		 * have ops assigned yet, set the ops and use them to verify the
> +		 * buffer contents.
> +		 *
> +		 * Under normal operations, every in-core buffer is verified on
> +		 * read I/O completion, but there are two scenarios that can
> +		 * lead to in-core buffers without an assigned ->b_ops:
> +		 *
> +		 *   1) During log recovery of buffers on a V4 filesystem.
> +		 *	These buffers are purged at the end of recovery, though.
> +		 *   2) Oonline repair intentionally reads with a NULL buffer

s/Oonline/Online/ like bfoster said.

With that fixed,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> +		 *	ops to run several verifiers across an in-core buffer in
> +		 *	order to establish buffer type.  If repair can't
> +		 *	establish that, the buffer will be left in memory with
> +		 *	NULL buffer ops.
> +		 */
> +		if (ops && !bp->b_ops) {
> +			bp->b_ops = ops;
> +			bp->b_ops->verify_read(bp);
> +			/*
> +			 * If verification failed, clear XBF_DONE as we assume
> +			 * that buffers have no recorded errors when in XBF_DONE
> +			 * state.
> +			 */
> +			error = bp->b_error;
> +			if (error)
> +				xfs_buf_clear_flags(bp, XBF_DONE);
> +		}
> +
> +		/* We do not want read in the flags */
> +		xfs_buf_clear_flags(bp, XBF_READ);
> +	} else {
>  		/* Initiate the buffer read and wait. */
>  		XFS_STATS_INC(target->bt_mount, xb_get_read);
>  		bp->b_ops = ops;
>  		error = _xfs_buf_read(bp);
> -	} else {
> -		/* Buffer already read; all we need to do is check it. */
> -		error = xfs_buf_reverify(bp, ops);
> -
> -		/* We do not want read in the flags */
> -		xfs_buf_clear_flags(bp, XBF_READ);
> -		ASSERT(bp->b_ops != NULL || ops == NULL);
>  	}
>  
>  	if (error)
> -- 
> 2.53.0
> 
>