From af3e3a5259e35d7056fbe568a0ffcbd1420e1742 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 11 Mar 2016 17:34:50 +0100 Subject: block: don't unecessarily clobber bi_error for chained bios Only overwrite the parents bi_error if it was zero. That way a successful bio completion doesn't reset the error pointer. Reported-by: Brian Foster Signed-off-by: Christoph Hellwig Signed-off-by: Jens Axboe --- block/bio.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'block/bio.c') diff --git a/block/bio.c b/block/bio.c index dbabd48b1934..282ca2e5aaf2 100644 --- a/block/bio.c +++ b/block/bio.c @@ -300,7 +300,8 @@ static void bio_chain_endio(struct bio *bio) { struct bio *parent = bio->bi_private; - parent->bi_error = bio->bi_error; + if (!parent->bi_error) + parent->bi_error = bio->bi_error; bio_endio(parent); bio_put(bio); } @@ -1753,7 +1754,9 @@ void bio_endio(struct bio *bio) */ if (bio->bi_end_io == bio_chain_endio) { struct bio *parent = bio->bi_private; - parent->bi_error = bio->bi_error; + + if (!parent->bi_error) + parent->bi_error = bio->bi_error; bio_put(bio); bio = parent; } else { -- cgit v1.2.3 From 38f8baae890561203ba6093f76b14576ce9b271b Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 11 Mar 2016 17:34:51 +0100 Subject: block: factor out chained bio completion Factor common code between bio_chain_endio and bio_endio into a common helper. Signed-off-by: Christoph Hellwig Signed-off-by: Jens Axboe --- block/bio.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'block/bio.c') diff --git a/block/bio.c b/block/bio.c index 282ca2e5aaf2..67e51ace1b77 100644 --- a/block/bio.c +++ b/block/bio.c @@ -296,14 +296,19 @@ void bio_reset(struct bio *bio) } EXPORT_SYMBOL(bio_reset); -static void bio_chain_endio(struct bio *bio) +static struct bio *__bio_chain_endio(struct bio *bio) { struct bio *parent = bio->bi_private; if (!parent->bi_error) parent->bi_error = bio->bi_error; - bio_endio(parent); bio_put(bio); + return parent; +} + +static void bio_chain_endio(struct bio *bio) +{ + bio_endio(__bio_chain_endio(bio)); } /* @@ -1753,12 +1758,7 @@ void bio_endio(struct bio *bio) * pointers also disables gcc's sibling call optimization. */ if (bio->bi_end_io == bio_chain_endio) { - struct bio *parent = bio->bi_private; - - if (!parent->bi_error) - parent->bi_error = bio->bi_error; - bio_put(bio); - bio = parent; + bio = __bio_chain_endio(bio); } else { if (bio->bi_end_io) bio->bi_end_io(bio); -- cgit v1.2.3 From ba8c6967b7391aab8fa562611fe637a57850b4aa Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 11 Mar 2016 17:34:52 +0100 Subject: block: cleanup bio_endio Replace the while loop that unecessarily checks for a NULL bio in the fast path with a simple goto loop. Signed-off-by: Christoph Hellwig Signed-off-by: Jens Axboe --- block/bio.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) (limited to 'block/bio.c') diff --git a/block/bio.c b/block/bio.c index 67e51ace1b77..e4682ec11fcd 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); -- cgit v1.2.3 From 2b885517110cbe8724fef30363778b6284d0a428 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 11 Mar 2016 17:34:53 +0100 Subject: block: bio_remaining_done() isn't unlikely We use bio chaining during most I/Os these days due to the delayed bio splitting. Additionally XFS will start using it, and there is a pending direct I/O rewrite also making heavy use for it. Don't pretend it's always unlikely, and let the branch predictor do it's job instead. Signed-off-by: Christoph Hellwig Signed-off-by: Jens Axboe --- block/bio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'block/bio.c') diff --git a/block/bio.c b/block/bio.c index e4682ec11fcd..0fde6e0e81f2 100644 --- a/block/bio.c +++ b/block/bio.c @@ -1746,7 +1746,7 @@ static inline bool bio_remaining_done(struct bio *bio) void bio_endio(struct bio *bio) { again: - if (unlikely(!bio_remaining_done(bio))) + if (!bio_remaining_done(bio)) return; /* -- cgit v1.2.3