diff mbox series

[09/13] xfs: simplify the xfs_buf_ioend_disposition calling convention

Message ID 20200830061512.1148591-10-hch@lst.de (mailing list archive)
State Superseded
Headers show
Series None | expand

Commit Message

Christoph Hellwig Aug. 30, 2020, 6:15 a.m. UTC
Now that all the actual error handling is in a single place,
xfs_buf_ioend_disposition just needs to return true if took ownership of
the buffer, or false if not instead of the tristate.  Also move the
error check back in the caller to optimize for the fast path, and give
the function a better fitting name.

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

Comments

Darrick J. Wong Aug. 31, 2020, 8:26 p.m. UTC | #1
On Sun, Aug 30, 2020 at 08:15:08AM +0200, Christoph Hellwig wrote:
> Now that all the actual error handling is in a single place,
> xfs_buf_ioend_disposition just needs to return true if took ownership of
> the buffer, or false if not instead of the tristate.  Also move the
> error check back in the caller to optimize for the fast path, and give
> the function a better fitting name.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_buf.c | 36 +++++++++---------------------------
>  1 file changed, 9 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 951d9c35b3170c..13435cce2699e4 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1223,29 +1223,17 @@ xfs_buf_ioerror_permanent(
>   * If we get repeated async write failures, then we take action according to the
>   * error configuration we have been set up to use.
>   *
> - * Multi-state return value:
> - *
> - * XBF_IOEND_FINISH: run callback completions
> - * XBF_IOEND_DONE: resubmitted immediately, do not run any completions
> - * XBF_IOEND_FAIL: transient error, run failure callback completions and then
> - *    release the buffer
> + * Returns true if this function took care of error handling and the caller must
> + * not touch the buffer again.  Return false if the caller should proceed with
> + * normal I/O completion handling.
>   */
> -enum xfs_buf_ioend_disposition {
> -	XBF_IOEND_FINISH,
> -	XBF_IOEND_DONE,
> -	XBF_IOEND_FAIL,
> -};
> -
> -static enum xfs_buf_ioend_disposition
> -xfs_buf_ioend_disposition(
> +static bool
> +xfs_buf_ioend_handle_error(
>  	struct xfs_buf		*bp)
>  {
>  	struct xfs_mount	*mp = bp->b_mount;
>  	struct xfs_error_cfg	*cfg;
>  
> -	if (likely(!bp->b_error))
> -		return XBF_IOEND_FINISH;
> -
>  	/*
>  	 * If we've already decided to shutdown the filesystem because of I/O
>  	 * errors, there's no point in giving this a retry.
> @@ -1291,18 +1279,18 @@ xfs_buf_ioend_disposition(
>  		ASSERT(list_empty(&bp->b_li_list));
>  	xfs_buf_ioerror(bp, 0);
>  	xfs_buf_relse(bp);
> -	return XBF_IOEND_FAIL;
> +	return true;
>  
>  resubmit:
>  	xfs_buf_ioerror(bp, 0);
>  	bp->b_flags |= (XBF_WRITE | XBF_DONE | XBF_WRITE_FAIL);
>  	xfs_buf_submit(bp);
> -	return XBF_IOEND_DONE;
> +	return true;
>  out_stale:
>  	xfs_buf_stale(bp);
>  	bp->b_flags |= XBF_DONE;
>  	trace_xfs_buf_error_relse(bp, _RET_IP_);
> -	return XBF_IOEND_FINISH;
> +	return false;
>  }
>  
>  static void
> @@ -1340,14 +1328,8 @@ xfs_buf_ioend(
>  			bp->b_flags |= XBF_DONE;
>  		}
>  
> -		switch (xfs_buf_ioend_disposition(bp)) {
> -		case XBF_IOEND_DONE:
> -			return;
> -		case XBF_IOEND_FAIL:
> +		if (unlikely(bp->b_error) && xfs_buf_ioend_handle_error(bp))
>  			return;
> -		default:
> -			break;
> -		}
>  
>  		/* clear the retry state */
>  		bp->b_last_error = 0;
> -- 
> 2.28.0
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 951d9c35b3170c..13435cce2699e4 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1223,29 +1223,17 @@  xfs_buf_ioerror_permanent(
  * If we get repeated async write failures, then we take action according to the
  * error configuration we have been set up to use.
  *
- * Multi-state return value:
- *
- * XBF_IOEND_FINISH: run callback completions
- * XBF_IOEND_DONE: resubmitted immediately, do not run any completions
- * XBF_IOEND_FAIL: transient error, run failure callback completions and then
- *    release the buffer
+ * Returns true if this function took care of error handling and the caller must
+ * not touch the buffer again.  Return false if the caller should proceed with
+ * normal I/O completion handling.
  */
-enum xfs_buf_ioend_disposition {
-	XBF_IOEND_FINISH,
-	XBF_IOEND_DONE,
-	XBF_IOEND_FAIL,
-};
-
-static enum xfs_buf_ioend_disposition
-xfs_buf_ioend_disposition(
+static bool
+xfs_buf_ioend_handle_error(
 	struct xfs_buf		*bp)
 {
 	struct xfs_mount	*mp = bp->b_mount;
 	struct xfs_error_cfg	*cfg;
 
-	if (likely(!bp->b_error))
-		return XBF_IOEND_FINISH;
-
 	/*
 	 * If we've already decided to shutdown the filesystem because of I/O
 	 * errors, there's no point in giving this a retry.
@@ -1291,18 +1279,18 @@  xfs_buf_ioend_disposition(
 		ASSERT(list_empty(&bp->b_li_list));
 	xfs_buf_ioerror(bp, 0);
 	xfs_buf_relse(bp);
-	return XBF_IOEND_FAIL;
+	return true;
 
 resubmit:
 	xfs_buf_ioerror(bp, 0);
 	bp->b_flags |= (XBF_WRITE | XBF_DONE | XBF_WRITE_FAIL);
 	xfs_buf_submit(bp);
-	return XBF_IOEND_DONE;
+	return true;
 out_stale:
 	xfs_buf_stale(bp);
 	bp->b_flags |= XBF_DONE;
 	trace_xfs_buf_error_relse(bp, _RET_IP_);
-	return XBF_IOEND_FINISH;
+	return false;
 }
 
 static void
@@ -1340,14 +1328,8 @@  xfs_buf_ioend(
 			bp->b_flags |= XBF_DONE;
 		}
 
-		switch (xfs_buf_ioend_disposition(bp)) {
-		case XBF_IOEND_DONE:
-			return;
-		case XBF_IOEND_FAIL:
+		if (unlikely(bp->b_error) && xfs_buf_ioend_handle_error(bp))
 			return;
-		default:
-			break;
-		}
 
 		/* clear the retry state */
 		bp->b_last_error = 0;