Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1315463 > unrolled thread

[PATCH v1] block: fix bio splitting on max sectors

Back to article view | Back to linux.kernel

Contents

  [PATCH v1] block: fix bio splitting on max sectors Ming Lei <tom.leiming@gmail.com> - 2016-01-23 01:10 +0100
    Re: [PATCH v1] block: fix bio splitting on max sectors Jens Axboe <axboe@kernel.dk> - 2016-01-23 04:50 +0100
      Re: [PATCH v1] block: fix bio splitting on max sectors Stefan Haberland <sth@linux.vnet.ibm.com> - 2016-01-25 15:20 +0100
        Re: [PATCH v1] block: fix bio splitting on max sectors Jens Axboe <axboe@kernel.dk> - 2016-01-25 19:30 +0100

#1315463 — [PATCH v1] block: fix bio splitting on max sectors

After commit e36f62042880(block: split bios to maxpossible length),
bio can be splitted in the middle of a vector entry, then it
is easy to split out one bio which size isn't aligned with block
size, especially when the block size is bigger than 512.

This patch fixes the issue by making the max io size aligned
to logical block size.

Fixes: e36f62042880(block: split bios to maxpossible length)
Reported-by: Stefan Haberland <sth@linux.vnet.ibm.com>
Cc: Keith Busch <keith.busch@intel.com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
V1:
	- avoid double shift as suggested by Linus
	- compute 'max_sectors' once as suggested by Keith

 block/blk-merge.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 1699df5..888a7fe 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -70,6 +70,18 @@ static struct bio *blk_bio_write_same_split(struct request_queue *q,
 	return bio_split(bio, q->limits.max_write_same_sectors, GFP_NOIO, bs);
 }
 
+static inline unsigned get_max_io_size(struct request_queue *q,
+				       struct bio *bio)
+{
+	unsigned sectors = blk_max_size_offset(q, bio->bi_iter.bi_sector);
+	unsigned mask = queue_logical_block_size(q) - 1;
+
+	/* aligned to logical block size */
+	sectors &= ~(mask >> 9);
+
+	return sectors;
+}
+
 static struct bio *blk_bio_segment_split(struct request_queue *q,
 					 struct bio *bio,
 					 struct bio_set *bs,
@@ -81,6 +93,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
 	unsigned front_seg_size = bio->bi_seg_front_size;
 	bool do_split = true;
 	struct bio *new = NULL;
+	const unsigned max_sectors = get_max_io_size(q, bio);
 
 	bio_for_each_segment(bv, bio, iter) {
 		/*
@@ -90,20 +103,19 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
 		if (bvprvp && bvec_gap_to_prev(q, bvprvp, bv.bv_offset))
 			goto split;
 
-		if (sectors + (bv.bv_len >> 9) >
-				blk_max_size_offset(q, bio->bi_iter.bi_sector)) {
+		if (sectors + (bv.bv_len >> 9) > max_sectors) {
 			/*
 			 * Consider this a new segment if we're splitting in
 			 * the middle of this vector.
 			 */
 			if (nsegs < queue_max_segments(q) &&
-			    sectors < blk_max_size_offset(q,
-						bio->bi_iter.bi_sector)) {
+			    sectors < max_sectors) {
 				nsegs++;
-				sectors = blk_max_size_offset(q,
-						bio->bi_iter.bi_sector);
+				sectors = max_sectors;
 			}
-			goto split;
+			if (sectors)
+				goto split;
+			/* Make this single bvec as the 1st segment */
 		}
 
 		if (bvprvp && blk_queue_cluster(q)) {
-- 
1.9.1

[toc] | [next] | [standalone]

#1315511

On 01/22/2016 05:05 PM, Ming Lei wrote:
> After commit e36f62042880(block: split bios to maxpossible length),
> bio can be splitted in the middle of a vector entry, then it
> is easy to split out one bio which size isn't aligned with block
> size, especially when the block size is bigger than 512.
>
> This patch fixes the issue by making the max io size aligned
> to logical block size.
>
> Fixes: e36f62042880(block: split bios to maxpossible length)
> Reported-by: Stefan Haberland <sth@linux.vnet.ibm.com>
> Cc: Keith Busch <keith.busch@intel.com>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
> V1:
> 	- avoid double shift as suggested by Linus
> 	- compute 'max_sectors' once as suggested by Keith

This looks good to me, I'll apply and run a bit of local testing.

-- 
Jens Axboe

[toc] | [prev] | [next] | [standalone]

#1316745

Am 23.01.2016 um 04:44 schrieb Jens Axboe:
> On 01/22/2016 05:05 PM, Ming Lei wrote:
>> After commit e36f62042880(block: split bios to maxpossible length),
>> bio can be splitted in the middle of a vector entry, then it
>> is easy to split out one bio which size isn't aligned with block
>> size, especially when the block size is bigger than 512.
>>
>> This patch fixes the issue by making the max io size aligned
>> to logical block size.
>>
>> Fixes: e36f62042880(block: split bios to maxpossible length)
>> Reported-by: Stefan Haberland <sth@linux.vnet.ibm.com>
>> Cc: Keith Busch <keith.busch@intel.com>
>> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>> ---
>> V1:
>>     - avoid double shift as suggested by Linus
>>     - compute 'max_sectors' once as suggested by Keith
>
> This looks good to me, I'll apply and run a bit of local testing.
>

I applied the patch and it seems to work for DASD devices. I will do 
some more testing but for me it looks OK.
Thanks!

Regards,
Stefan

[toc] | [prev] | [next] | [standalone]

#1317151

On 01/25/2016 07:13 AM, Stefan Haberland wrote:
> Am 23.01.2016 um 04:44 schrieb Jens Axboe:
>> On 01/22/2016 05:05 PM, Ming Lei wrote:
>>> After commit e36f62042880(block: split bios to maxpossible length),
>>> bio can be splitted in the middle of a vector entry, then it
>>> is easy to split out one bio which size isn't aligned with block
>>> size, especially when the block size is bigger than 512.
>>>
>>> This patch fixes the issue by making the max io size aligned
>>> to logical block size.
>>>
>>> Fixes: e36f62042880(block: split bios to maxpossible length)
>>> Reported-by: Stefan Haberland <sth@linux.vnet.ibm.com>
>>> Cc: Keith Busch <keith.busch@intel.com>
>>> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
>>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>>> ---
>>> V1:
>>>     - avoid double shift as suggested by Linus
>>>     - compute 'max_sectors' once as suggested by Keith
>>
>> This looks good to me, I'll apply and run a bit of local testing.
>>
>
> I applied the patch and it seems to work for DASD devices. I will do
> some more testing but for me it looks OK.

Thanks for testing, Stefan. It'll go into mainline pretty soon.

-- 
Jens Axboe

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web