2016-03-12 14:56:35

by Ming Lei

Subject: [PATCH] block: don't optimize for non-cloned bio in bio_get_last_bvec()

For !BIO_CLONED bio, we can use .bi_vcnt safely, but it
doesn't mean we can just simply return .bi_io_vec[.bi_vcnt - 1]
because the start postion may have been moved in the middle of
the bvec, such as splitting in the middle of bvec.

Fixes: 7bcd79ac50d9(block: bio: introduce helpers to get the 1st and last bvec)
Cc: [email protected]
Reported-by: Kent Overstreet <[email protected]>
Signed-off-by: Ming Lei <[email protected]>
---
Jens, I am fine with either reverting previous patchset or
applying this one to v4.5, and you decide it.

Thanks Kent for reporting the issue!

include/linux/bio.h | 5 -----
1 file changed, 5 deletions(-)

diff --git a/include/linux/bio.h b/include/linux/bio.h
index 1e7248f..4abc129 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -267,11 +267,6 @@ static inline void bio_get_last_bvec(struct bio *bio, struct bio_vec *bv)
struct bvec_iter iter = bio->bi_iter;
int idx;

- if (!bio_flagged(bio, BIO_CLONED)) {
- *bv = bio->bi_io_vec[bio->bi_vcnt - 1];
- return;
- }
-
if (unlikely(!bio_multiple_segments(bio))) {
*bv = bio_iovec(bio);
return;
--
1.9.1

2016-03-13 03:58:52

by Jens Axboe

Subject: Re: [PATCH] block: don't optimize for non-cloned bio in bio_get_last_bvec()

On 03/12/2016 07:56 AM, Ming Lei wrote:
> For !BIO_CLONED bio, we can use .bi_vcnt safely, but it
> doesn't mean we can just simply return .bi_io_vec[.bi_vcnt - 1]
> because the start postion may have been moved in the middle of
> the bvec, such as splitting in the middle of bvec.

Added and tested.

--
Jens Axboe

2016-03-13 04:20:51

by Jens Axboe

Subject: Re: [PATCH] block: don't optimize for non-cloned bio in bio_get_last_bvec()

On 03/12/2016 08:58 PM, Jens Axboe wrote:
> On 03/12/2016 07:56 AM, Ming Lei wrote:
>> For !BIO_CLONED bio, we can use .bi_vcnt safely, but it
>> doesn't mean we can just simply return .bi_io_vec[.bi_vcnt - 1]
>> because the start postion may have been moved in the middle of
>> the bvec, such as splitting in the middle of bvec.
>
> Added and tested.

And Ming, let's not get a repeat of this situation. For critical fixes,
it has to be spot on for post -rc1, especially for code that is
notoriously difficult to get right. Fixes are obviously fine, but it has
to be just enough to fix the regression, no more. This has been too lax,
I expect this to improve.

--
Jens Axboe