diff mbox

[3/4] block: cleanup bio_endio

Message ID 1457714093-21926-4-git-send-email-hch@lst.de (mailing list archive)
State Accepted, archived
Delegated to: Jens Axboe
Headers show

Commit Message

Christoph Hellwig March 11, 2016, 4:34 p.m. UTC
Replace the while loop that unecessarily checks for a NULL bio in the fast
path witch a simple goto loop.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

Comments

Johannes Thumshirn March 14, 2016, 8:24 a.m. UTC | #1
On Fri, Mar 11, 2016 at 05:34:52PM +0100, Christoph Hellwig wrote:
> Replace the while loop that unecessarily checks for a NULL bio in the fast
> path witch a simple goto loop.

s/witch/with/ ?

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/bio.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index 67e51ac..e4682ec 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1745,26 +1745,25 @@ static inline bool bio_remaining_done(struct bio *bio)
>   **/
>  void bio_endio(struct bio *bio)
>  {
> -	while (bio) {
> -		if (unlikely(!bio_remaining_done(bio)))
> -			break;
> +again:
> +	if (unlikely(!bio_remaining_done(bio)))
> +		return;
>  
> -		/*
> -		 * Need to have a real endio function for chained bios,
> -		 * otherwise various corner cases will break (like stacking
> -		 * block devices that save/restore bi_end_io) - however, we want
> -		 * to avoid unbounded recursion and blowing the stack. Tail call
> -		 * optimization would handle this, but compiling with frame
> -		 * pointers also disables gcc's sibling call optimization.
> -		 */
> -		if (bio->bi_end_io == bio_chain_endio) {
> -			bio = __bio_chain_endio(bio);
> -		} else {
> -			if (bio->bi_end_io)
> -				bio->bi_end_io(bio);
> -			bio = NULL;
> -		}
> +	/*
> +	 * Need to have a real endio function for chained bios, otherwise
> +	 * various corner cases will break (like stacking block devices that
> +	 * save/restore bi_end_io) - however, we want to avoid unbounded
> +	 * recursion and blowing the stack. Tail call optimization would
> +	 * handle this, but compiling with frame pointers also disables
> +	 * gcc's sibling call optimization.
> +	 */
> +	if (bio->bi_end_io == bio_chain_endio) {
> +		bio = __bio_chain_endio(bio);
> +		goto again;
>  	}
> +
> +	if (bio->bi_end_io)
> +		bio->bi_end_io(bio);
>  }
>  EXPORT_SYMBOL(bio_endio);
>  
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Just out of curiosity, do you have any numbers for this change?
Sagi Grimberg March 14, 2016, 2:55 p.m. UTC | #2
Looks fine,

Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/block/bio.c b/block/bio.c
index 67e51ac..e4682ec 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1745,26 +1745,25 @@  static inline bool bio_remaining_done(struct bio *bio)
  **/
 void bio_endio(struct bio *bio)
 {
-	while (bio) {
-		if (unlikely(!bio_remaining_done(bio)))
-			break;
+again:
+	if (unlikely(!bio_remaining_done(bio)))
+		return;
 
-		/*
-		 * Need to have a real endio function for chained bios,
-		 * otherwise various corner cases will break (like stacking
-		 * block devices that save/restore bi_end_io) - however, we want
-		 * to avoid unbounded recursion and blowing the stack. Tail call
-		 * optimization would handle this, but compiling with frame
-		 * pointers also disables gcc's sibling call optimization.
-		 */
-		if (bio->bi_end_io == bio_chain_endio) {
-			bio = __bio_chain_endio(bio);
-		} else {
-			if (bio->bi_end_io)
-				bio->bi_end_io(bio);
-			bio = NULL;
-		}
+	/*
+	 * Need to have a real endio function for chained bios, otherwise
+	 * various corner cases will break (like stacking block devices that
+	 * save/restore bi_end_io) - however, we want to avoid unbounded
+	 * recursion and blowing the stack. Tail call optimization would
+	 * handle this, but compiling with frame pointers also disables
+	 * gcc's sibling call optimization.
+	 */
+	if (bio->bi_end_io == bio_chain_endio) {
+		bio = __bio_chain_endio(bio);
+		goto again;
 	}
+
+	if (bio->bi_end_io)
+		bio->bi_end_io(bio);
 }
 EXPORT_SYMBOL(bio_endio);