xfs_buf_submit error handling fix

xfs_buf_submit error handling fix

[Christoph Hellwig]

Hi all,

currently xfs_buf_submit can execute the endio handling twice when failing
for a synchronous submission.  This series refactors the surrounding code
a bit to allow for a non-hacky fix and then applies that.

Diffstat:
 xfs_buf.c      |   86 ++++++++++++++++++++++++++++-----------------------------
 xfs_buf.h      |    2 -
 xfs_buf_item.c |    3 -
 xfs_inode.c    |    3 -
 4 files changed, 46 insertions(+), 48 deletions(-)

[PATCH 1/6] xfs: open code xfs_buf_ioend_fail in xfs_buf_submit

[Christoph Hellwig]

This better integrates with the other failure handling in xfs_buf_submit,
and prepares for a better API in xfs_buf_ioend_fail.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 1ed9f01b3275..9aaa8b87fc33 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1383,10 +1383,8 @@ xfs_buf_submit(
 	 * state here rather than mount state to avoid corrupting the log tail
 	 * on shutdown.
 	 */
-	if (bp->b_mount->m_log && xlog_is_shutdown(bp->b_mount->m_log)) {
-		xfs_buf_ioend_fail(bp);
-		return;
-	}
+	if (bp->b_mount->m_log && xlog_is_shutdown(bp->b_mount->m_log))
+		goto ioerror;
 
 	if (bp->b_flags & XBF_WRITE)
 		xfs_buf_wait_unpin(bp);
@@ -1399,17 +1397,22 @@ xfs_buf_submit(
 
 	if ((bp->b_flags & XBF_WRITE) && !xfs_buf_verify_write(bp)) {
 		xfs_force_shutdown(bp->b_mount, SHUTDOWN_CORRUPT_INCORE);
-		xfs_buf_ioend(bp);
-		return;
+		goto end_io;
 	}
 
 	/* In-memory targets are directly mapped, no I/O required. */
-	if (xfs_buftarg_is_mem(bp->b_target)) {
-		xfs_buf_ioend(bp);
-		return;
-	}
+	if (xfs_buftarg_is_mem(bp->b_target))
+		goto end_io;
 
 	xfs_buf_submit_bio(bp);
+	return;
+
+ioerror:
+	bp->b_flags &= ~XBF_DONE;
+	xfs_buf_stale(bp);
+	xfs_buf_ioerror(bp, -EIO);
+end_io:
+	xfs_buf_ioend(bp);
 }
 
 /*
-- 
2.53.0

Re: [PATCH 1/6] xfs: open code xfs_buf_ioend_fail in xfs_buf_submit

[Carlos Maiolino]

On Mon, Jun 22, 2026 at 09:30:00AM +0200, Christoph Hellwig wrote:
> This better integrates with the other failure handling in xfs_buf_submit,
> and prepares for a better API in xfs_buf_ioend_fail.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/xfs_buf.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 1ed9f01b3275..9aaa8b87fc33 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1383,10 +1383,8 @@ xfs_buf_submit(
>  	 * state here rather than mount state to avoid corrupting the log tail
>  	 * on shutdown.
>  	 */
> -	if (bp->b_mount->m_log && xlog_is_shutdown(bp->b_mount->m_log)) {
> -		xfs_buf_ioend_fail(bp);
> -		return;
> -	}
> +	if (bp->b_mount->m_log && xlog_is_shutdown(bp->b_mount->m_log))
> +		goto ioerror;
>  
>  	if (bp->b_flags & XBF_WRITE)
>  		xfs_buf_wait_unpin(bp);
> @@ -1399,17 +1397,22 @@ xfs_buf_submit(
>  
>  	if ((bp->b_flags & XBF_WRITE) && !xfs_buf_verify_write(bp)) {
>  		xfs_force_shutdown(bp->b_mount, SHUTDOWN_CORRUPT_INCORE);
> -		xfs_buf_ioend(bp);
> -		return;
> +		goto end_io;
>  	}
>  
>  	/* In-memory targets are directly mapped, no I/O required. */
> -	if (xfs_buftarg_is_mem(bp->b_target)) {
> -		xfs_buf_ioend(bp);
> -		return;
> -	}
> +	if (xfs_buftarg_is_mem(bp->b_target))
> +		goto end_io;
>  
>  	xfs_buf_submit_bio(bp);
> +	return;
> +
> +ioerror:
> +	bp->b_flags &= ~XBF_DONE;
> +	xfs_buf_stale(bp);
> +	xfs_buf_ioerror(bp, -EIO);
> +end_io:
> +	xfs_buf_ioend(bp);
>  }
>  
>  /*
> -- 
> 2.53.0
> 
> 

[PATCH 2/6] xfs: also mark the buffer stale on verifier failure in xfs_buf_submit

[Christoph Hellwig]

We should treat the buffer that caused a shutdown the same as handling
buffers after a shutdown, so use the same stale && !DONE logic here.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 9aaa8b87fc33..60ef53acebd2 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1383,8 +1383,10 @@ xfs_buf_submit(
 	 * state here rather than mount state to avoid corrupting the log tail
 	 * on shutdown.
 	 */
-	if (bp->b_mount->m_log && xlog_is_shutdown(bp->b_mount->m_log))
+	if (bp->b_mount->m_log && xlog_is_shutdown(bp->b_mount->m_log)) {
+		xfs_buf_ioerror(bp, -EIO);
 		goto ioerror;
+	}
 
 	if (bp->b_flags & XBF_WRITE)
 		xfs_buf_wait_unpin(bp);
@@ -1397,7 +1399,7 @@ xfs_buf_submit(
 
 	if ((bp->b_flags & XBF_WRITE) && !xfs_buf_verify_write(bp)) {
 		xfs_force_shutdown(bp->b_mount, SHUTDOWN_CORRUPT_INCORE);
-		goto end_io;
+		goto ioerror;
 	}
 
 	/* In-memory targets are directly mapped, no I/O required. */
@@ -1410,7 +1412,6 @@ xfs_buf_submit(
 ioerror:
 	bp->b_flags &= ~XBF_DONE;
 	xfs_buf_stale(bp);
-	xfs_buf_ioerror(bp, -EIO);
 end_io:
 	xfs_buf_ioend(bp);
 }
-- 
2.53.0

Re: [PATCH 2/6] xfs: also mark the buffer stale on verifier failure in xfs_buf_submit

[Carlos Maiolino]

On Mon, Jun 22, 2026 at 09:30:01AM +0200, Christoph Hellwig wrote:
> We should treat the buffer that caused a shutdown the same as handling
> buffers after a shutdown, so use the same stale && !DONE logic here.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/xfs_buf.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 9aaa8b87fc33..60ef53acebd2 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1383,8 +1383,10 @@ xfs_buf_submit(
>  	 * state here rather than mount state to avoid corrupting the log tail
>  	 * on shutdown.
>  	 */
> -	if (bp->b_mount->m_log && xlog_is_shutdown(bp->b_mount->m_log))
> +	if (bp->b_mount->m_log && xlog_is_shutdown(bp->b_mount->m_log)) {
> +		xfs_buf_ioerror(bp, -EIO);
>  		goto ioerror;
> +	}
>  
>  	if (bp->b_flags & XBF_WRITE)
>  		xfs_buf_wait_unpin(bp);
> @@ -1397,7 +1399,7 @@ xfs_buf_submit(
>  
>  	if ((bp->b_flags & XBF_WRITE) && !xfs_buf_verify_write(bp)) {
>  		xfs_force_shutdown(bp->b_mount, SHUTDOWN_CORRUPT_INCORE);
> -		goto end_io;
> +		goto ioerror;
>  	}
>  
>  	/* In-memory targets are directly mapped, no I/O required. */
> @@ -1410,7 +1412,6 @@ xfs_buf_submit(
>  ioerror:
>  	bp->b_flags &= ~XBF_DONE;
>  	xfs_buf_stale(bp);
> -	xfs_buf_ioerror(bp, -EIO);
>  end_io:
>  	xfs_buf_ioend(bp);
>  }
> -- 
> 2.53.0
> 
> 

[PATCH 3/6] xfs: improve the xfs_buf_ioend_fail calling convention

[Christoph Hellwig]

Move setting the ASYNC flag into xfs_buf_ioend_fail_async, assert that
the buffer is locked as expected, and drop the confusing _ioend in the
name.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c      | 13 +++++++------
 fs/xfs/xfs_buf.h      |  2 +-
 fs/xfs/xfs_buf_item.c |  3 +--
 fs/xfs/xfs_inode.c    |  3 +--
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 60ef53acebd2..ef2ea15a97bb 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1192,17 +1192,18 @@ xfs_buf_ioerror_alert(
 }
 
 /*
- * To simulate an I/O failure, the buffer must be locked and held with at least
- * two references.
+ * Fail a locked and referenced buffer outside the I/O path.
  *
- * The buf item reference is dropped via ioend processing. The second reference
- * is owned by the caller and is dropped on I/O completion if the buffer is
- * XBF_ASYNC.
+ * The caller transfers a reference which will be released after processing the
+ * error.
  */
 void
-xfs_buf_ioend_fail(
+xfs_buf_fail(
 	struct xfs_buf	*bp)
 {
+	ASSERT(xfs_buf_islocked(bp));
+
+	bp->b_flags |= XBF_ASYNC;
 	bp->b_flags &= ~XBF_DONE;
 	xfs_buf_stale(bp);
 	xfs_buf_ioerror(bp, -EIO);
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index bf39d89f0f6d..690002d3dd2b 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -290,7 +290,7 @@ extern void __xfs_buf_ioerror(struct xfs_buf *bp, int error,
 		xfs_failaddr_t failaddr);
 #define xfs_buf_ioerror(bp, err) __xfs_buf_ioerror((bp), (err), __this_address)
 extern void xfs_buf_ioerror_alert(struct xfs_buf *bp, xfs_failaddr_t fa);
-void xfs_buf_ioend_fail(struct xfs_buf *);
+void xfs_buf_fail(struct xfs_buf *bp);
 void __xfs_buf_mark_corrupt(struct xfs_buf *bp, xfs_failaddr_t fa);
 #define xfs_buf_mark_corrupt(bp) __xfs_buf_mark_corrupt((bp), __this_address)
 
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 8487635579e5..1f055cd6732e 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -549,8 +549,7 @@ xfs_buf_item_unpin(
 		 * wait for the lock and then run the IO failure completion.
 		 */
 		xfs_buf_lock(bp);
-		bp->b_flags |= XBF_ASYNC;
-		xfs_buf_ioend_fail(bp);
+		xfs_buf_fail(bp);
 		return;
 	}
 
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 9978ac1422fc..15facb0631d1 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2634,8 +2634,7 @@ xfs_iflush_cluster(
 		 * inode cluster buffers.
 		 */
 		xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
-		bp->b_flags |= XBF_ASYNC;
-		xfs_buf_ioend_fail(bp);
+		xfs_buf_fail(bp);
 		return error;
 	}
 
-- 
2.53.0

Re: [PATCH 3/6] xfs: improve the xfs_buf_ioend_fail calling convention

[Carlos Maiolino]

On Mon, Jun 22, 2026 at 09:30:02AM +0200, Christoph Hellwig wrote:
> Move setting the ASYNC flag into xfs_buf_ioend_fail_async, assert that
				  Is this a typo? I can't find this
				  definition anywhere and the code also
				  moves this into xfs_buf_ioend_fail(),
				  not into a _async variation.


> the buffer is locked as expected, and drop the confusing _ioend in the
> name.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c      | 13 +++++++------
>  fs/xfs/xfs_buf.h      |  2 +-
>  fs/xfs/xfs_buf_item.c |  3 +--
>  fs/xfs/xfs_inode.c    |  3 +--
>  4 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 60ef53acebd2..ef2ea15a97bb 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1192,17 +1192,18 @@ xfs_buf_ioerror_alert(
>  }
>  
>  /*
> - * To simulate an I/O failure, the buffer must be locked and held with at least
> - * two references.
> + * Fail a locked and referenced buffer outside the I/O path.
>   *
> - * The buf item reference is dropped via ioend processing. The second reference
> - * is owned by the caller and is dropped on I/O completion if the buffer is
> - * XBF_ASYNC.
> + * The caller transfers a reference which will be released after processing the
> + * error.
>   */
>  void
> -xfs_buf_ioend_fail(
> +xfs_buf_fail(
>  	struct xfs_buf	*bp)
>  {
> +	ASSERT(xfs_buf_islocked(bp));
> +
> +	bp->b_flags |= XBF_ASYNC;
>  	bp->b_flags &= ~XBF_DONE;
>  	xfs_buf_stale(bp);
>  	xfs_buf_ioerror(bp, -EIO);
> diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
> index bf39d89f0f6d..690002d3dd2b 100644
> --- a/fs/xfs/xfs_buf.h
> +++ b/fs/xfs/xfs_buf.h
> @@ -290,7 +290,7 @@ extern void __xfs_buf_ioerror(struct xfs_buf *bp, int error,
>  		xfs_failaddr_t failaddr);
>  #define xfs_buf_ioerror(bp, err) __xfs_buf_ioerror((bp), (err), __this_address)
>  extern void xfs_buf_ioerror_alert(struct xfs_buf *bp, xfs_failaddr_t fa);
> -void xfs_buf_ioend_fail(struct xfs_buf *);
> +void xfs_buf_fail(struct xfs_buf *bp);
>  void __xfs_buf_mark_corrupt(struct xfs_buf *bp, xfs_failaddr_t fa);
>  #define xfs_buf_mark_corrupt(bp) __xfs_buf_mark_corrupt((bp), __this_address)
>  
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index 8487635579e5..1f055cd6732e 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -549,8 +549,7 @@ xfs_buf_item_unpin(
>  		 * wait for the lock and then run the IO failure completion.
>  		 */
>  		xfs_buf_lock(bp);
> -		bp->b_flags |= XBF_ASYNC;
> -		xfs_buf_ioend_fail(bp);
> +		xfs_buf_fail(bp);
>  		return;
>  	}
>  
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 9978ac1422fc..15facb0631d1 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2634,8 +2634,7 @@ xfs_iflush_cluster(
>  		 * inode cluster buffers.
>  		 */
>  		xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> -		bp->b_flags |= XBF_ASYNC;
> -		xfs_buf_ioend_fail(bp);
> +		xfs_buf_fail(bp);
>  		return error;
>  	}
>  
> -- 
> 2.53.0
> 

Re: [PATCH 3/6] xfs: improve the xfs_buf_ioend_fail calling convention

[Christoph Hellwig]

On Mon, Jun 22, 2026 at 02:59:05PM +0200, Carlos Maiolino wrote:
> On Mon, Jun 22, 2026 at 09:30:02AM +0200, Christoph Hellwig wrote:
> > Move setting the ASYNC flag into xfs_buf_ioend_fail_async, assert that
> 				  Is this a typo? I can't find this
> 				  definition anywhere and the code also
> 				  moves this into xfs_buf_ioend_fail(),
> 				  not into a _async variation.

Yeah, this could use a little update.

Re: [PATCH 3/6] xfs: improve the xfs_buf_ioend_fail calling convention

[Carlos Maiolino]

On Wed, Jun 24, 2026 at 09:42:06AM +0200, Christoph Hellwig wrote:
> On Mon, Jun 22, 2026 at 02:59:05PM +0200, Carlos Maiolino wrote:
> > On Mon, Jun 22, 2026 at 09:30:02AM +0200, Christoph Hellwig wrote:
> > > Move setting the ASYNC flag into xfs_buf_ioend_fail_async, assert that
> > 				  Is this a typo? I can't find this
> > 				  definition anywhere and the code also
> > 				  moves this into xfs_buf_ioend_fail(),
> > 				  not into a _async variation.
> 
> Yeah, this could use a little update.
> 
> 

If you'll be re-sending the whole series again, feel free to add

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

At the end everything looks good with these caveats

[PATCH 4/6] xfs: remove xfs_buf_ioend

[Christoph Hellwig]

There are two callers of xfs_buf_ioend, one of which always has the
XBF_ASYNC flag set.  Open code the logic in both callers to prepare for a
bug fix.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index ef2ea15a97bb..651add22deea 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1146,18 +1146,6 @@ __xfs_buf_ioend(
 	return true;
 }
 
-static void
-xfs_buf_ioend(
-	struct xfs_buf	*bp)
-{
-	if (!__xfs_buf_ioend(bp))
-		return;
-	if (bp->b_flags & XBF_ASYNC)
-		xfs_buf_relse(bp);
-	else
-		complete(&bp->b_iowait);
-}
-
 static void
 xfs_buf_ioend_work(
 	struct work_struct	*work)
@@ -1207,7 +1195,8 @@ xfs_buf_fail(
 	bp->b_flags &= ~XBF_DONE;
 	xfs_buf_stale(bp);
 	xfs_buf_ioerror(bp, -EIO);
-	xfs_buf_ioend(bp);
+	if (__xfs_buf_ioend(bp))
+		xfs_buf_relse(bp);
 }
 
 int
@@ -1414,7 +1403,12 @@ xfs_buf_submit(
 	bp->b_flags &= ~XBF_DONE;
 	xfs_buf_stale(bp);
 end_io:
-	xfs_buf_ioend(bp);
+	if (!__xfs_buf_ioend(bp))
+		return;
+	if (bp->b_flags & XBF_ASYNC)
+		xfs_buf_relse(bp);
+	else
+		complete(&bp->b_iowait);
 }
 
 /*
-- 
2.53.0

Re: [PATCH 4/6] xfs: remove xfs_buf_ioend

[Carlos Maiolino]

On Mon, Jun 22, 2026 at 09:30:03AM +0200, Christoph Hellwig wrote:
> There are two callers of xfs_buf_ioend, one of which always has the
> XBF_ASYNC flag set.  Open code the logic in both callers to prepare for a
> bug fix.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Independent of the bugfix I didn't see yet, I'm happy to see this
function gone...

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/xfs_buf.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index ef2ea15a97bb..651add22deea 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1146,18 +1146,6 @@ __xfs_buf_ioend(
>  	return true;
>  }
>  
> -static void
> -xfs_buf_ioend(
> -	struct xfs_buf	*bp)
> -{
> -	if (!__xfs_buf_ioend(bp))
> -		return;
> -	if (bp->b_flags & XBF_ASYNC)
> -		xfs_buf_relse(bp);
> -	else
> -		complete(&bp->b_iowait);
> -}
> -
>  static void
>  xfs_buf_ioend_work(
>  	struct work_struct	*work)
> @@ -1207,7 +1195,8 @@ xfs_buf_fail(
>  	bp->b_flags &= ~XBF_DONE;
>  	xfs_buf_stale(bp);
>  	xfs_buf_ioerror(bp, -EIO);
> -	xfs_buf_ioend(bp);
> +	if (__xfs_buf_ioend(bp))
> +		xfs_buf_relse(bp);
>  }
>  
>  int
> @@ -1414,7 +1403,12 @@ xfs_buf_submit(
>  	bp->b_flags &= ~XBF_DONE;
>  	xfs_buf_stale(bp);
>  end_io:
> -	xfs_buf_ioend(bp);
> +	if (!__xfs_buf_ioend(bp))
> +		return;
> +	if (bp->b_flags & XBF_ASYNC)
> +		xfs_buf_relse(bp);
> +	else
> +		complete(&bp->b_iowait);
>  }
>  
>  /*
> -- 
> 2.53.0
> 

[PATCH 5/6] xfs: fix handling of synchronous errors in xfs_buf_submit

[Christoph Hellwig]

Synchronous readers and writers already run __xfs_buf_ioend from
xfs_buf_iowait after being woken through bp->b_iowait, so we
should not call it here, which can lead to double completions.

Fixes: 4b90de5bc0f5 ("xfs: reduce context switches for synchronous buffered I/O")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 651add22deea..ffaab0feb230 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1403,12 +1403,12 @@ xfs_buf_submit(
 	bp->b_flags &= ~XBF_DONE;
 	xfs_buf_stale(bp);
 end_io:
-	if (!__xfs_buf_ioend(bp))
-		return;
-	if (bp->b_flags & XBF_ASYNC)
-		xfs_buf_relse(bp);
-	else
+	if (bp->b_flags & XBF_ASYNC) {
+		if (__xfs_buf_ioend(bp))
+			xfs_buf_relse(bp);
+	} else {
 		complete(&bp->b_iowait);
+	}
 }
 
 /*
-- 
2.53.0

Re: [PATCH 5/6] xfs: fix handling of synchronous errors in xfs_buf_submit

[Carlos Maiolino]

On Mon, Jun 22, 2026 at 09:30:04AM +0200, Christoph Hellwig wrote:
> Synchronous readers and writers already run __xfs_buf_ioend from
> xfs_buf_iowait after being woken through bp->b_iowait, so we
> should not call it here, which can lead to double completions.

Makes sense

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>


> 
> Fixes: 4b90de5bc0f5 ("xfs: reduce context switches for synchronous buffered I/O")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 651add22deea..ffaab0feb230 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1403,12 +1403,12 @@ xfs_buf_submit(
>  	bp->b_flags &= ~XBF_DONE;
>  	xfs_buf_stale(bp);
>  end_io:
> -	if (!__xfs_buf_ioend(bp))
> -		return;
> -	if (bp->b_flags & XBF_ASYNC)
> -		xfs_buf_relse(bp);
> -	else
> +	if (bp->b_flags & XBF_ASYNC) {
> +		if (__xfs_buf_ioend(bp))
> +			xfs_buf_relse(bp);
> +	} else {
>  		complete(&bp->b_iowait);
> +	}
>  }
>  
>  /*
> -- 
> 2.53.0
> 
> 

[PATCH 6/6] xfs: simplify __xfs_buf_ioend

[Christoph Hellwig]

__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>
---
 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 ffaab0feb230..b66bbcab91aa 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

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

[Carlos Maiolino]

On Mon, Jun 22, 2026 at 09:30:05AM +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>

Looks good.

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>


> ---
>  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 ffaab0feb230..b66bbcab91aa 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
>