Re: [PATCH 6/6] xfs: simplify __xfs_buf_ioend

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

On Thu, Jun 25, 2026 at 03:58:36PM +0200, Christoph Hellwig wrote:
> __xfs_buf_ioend can only resubmit the buffer for asynchronous
> writes, which means the retry handling xfs_buf_iowait is not needed.
> 
> Because of this can stop returning a value from __xfs_buf_ioend and
> just release the buffer for async I/O that does not require retries.
> 
> Also drop the __-prefix now that the semantics are straight forward.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

This makes sense to me -- only async writes (e.g. logged buffers written
by the ail) get to go through the write retry mechanism; the synchronous
writers are either regular threads and can report errors, or they're
dquot/inode items and a cluster flush just marks the log item as failed.

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

--D

> ---
>  fs/xfs/xfs_buf.c | 51 ++++++++++++++++++++++++------------------------
>  1 file changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 83549573e2cc..f8511c11e017 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1098,11 +1098,17 @@ xfs_buf_ioend_handle_error(
>  	return false;
>  }
>  
> -/* returns false if the caller needs to resubmit the I/O, else true */
> -static bool
> -__xfs_buf_ioend(
> +/*
> + * Complete a buffer read or write.
> + *
> + * Releases the buffer if the I/O was asynchronous.
> + */
> +static void
> +xfs_buf_ioend(
>  	struct xfs_buf	*bp)
>  {
> +	bool		async = bp->b_flags & XBF_ASYNC;
> +
>  	trace_xfs_buf_iodone(bp, _RET_IP_);
>  
>  	if (bp->b_flags & XBF_READ) {
> @@ -1116,14 +1122,16 @@ __xfs_buf_ioend(
>  		if (bp->b_flags & XBF_READ_AHEAD)
>  			percpu_counter_dec(&bp->b_target->bt_readahead_count);
>  	} else {
> -		if (!bp->b_error) {
> +		if (unlikely(bp->b_error)) {
> +			if (xfs_buf_ioend_handle_error(bp)) {
> +				ASSERT(async);
> +				return;
> +			}
> +		} else {
>  			bp->b_flags &= ~XBF_WRITE_FAIL;
>  			bp->b_flags |= XBF_DONE;
>  		}
>  
> -		if (unlikely(bp->b_error) && xfs_buf_ioend_handle_error(bp))
> -			return false;
> -
>  		/* clear the retry state */
>  		bp->b_last_error = 0;
>  		bp->b_retries = 0;
> @@ -1143,18 +1151,15 @@ __xfs_buf_ioend(
>  
>  	bp->b_flags &= ~(XBF_READ | XBF_WRITE | XBF_READ_AHEAD |
>  			 _XBF_LOGRECOVERY);
> -	return true;
> +	if (async)
> +		xfs_buf_relse(bp);
>  }
>  
>  static void
>  xfs_buf_ioend_work(
>  	struct work_struct	*work)
>  {
> -	struct xfs_buf		*bp =
> -		container_of(work, struct xfs_buf, b_ioend_work);
> -
> -	if (__xfs_buf_ioend(bp))
> -		xfs_buf_relse(bp);
> +	xfs_buf_ioend(container_of(work, struct xfs_buf, b_ioend_work));
>  }
>  
>  void
> @@ -1195,8 +1200,7 @@ xfs_buf_fail(
>  	bp->b_flags &= ~XBF_DONE;
>  	xfs_buf_stale(bp);
>  	xfs_buf_ioerror(bp, -EIO);
> -	if (__xfs_buf_ioend(bp))
> -		xfs_buf_relse(bp);
> +	xfs_buf_ioend(bp);
>  }
>  
>  int
> @@ -1305,12 +1309,11 @@ xfs_buf_iowait(
>  {
>  	ASSERT(!(bp->b_flags & XBF_ASYNC));
>  
> -	do {
> -		trace_xfs_buf_iowait(bp, _RET_IP_);
> -		wait_for_completion(&bp->b_iowait);
> -		trace_xfs_buf_iowait_done(bp, _RET_IP_);
> -	} while (!__xfs_buf_ioend(bp));
> +	trace_xfs_buf_iowait(bp, _RET_IP_);
> +	wait_for_completion(&bp->b_iowait);
> +	trace_xfs_buf_iowait_done(bp, _RET_IP_);
>  
> +	xfs_buf_ioend(bp);
>  	return bp->b_error;
>  }
>  
> @@ -1403,12 +1406,10 @@ xfs_buf_submit(
>  	bp->b_flags &= ~XBF_DONE;
>  	xfs_buf_stale(bp);
>  end_io:
> -	if (bp->b_flags & XBF_ASYNC) {
> -		if (__xfs_buf_ioend(bp))
> -			xfs_buf_relse(bp);
> -	} else {
> +	if (bp->b_flags & XBF_ASYNC)
> +		xfs_buf_ioend(bp);
> +	else
>  		complete(&bp->b_iowait);
> -	}
>  }
>  
>  /*
> -- 
> 2.53.0
> 
>