* [PATCH] block: blk-merge: fast-clone bio when splitting rw bios
@ 2015-09-17 15:13 Ming Lei
2015-09-17 15:19 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Ming Lei @ 2015-09-17 15:13 UTC (permalink / raw)
To: Jens Axboe, linux-kernel
Cc: Ming Lei, Christoph Hellwig, Kent Overstreet, Ming Lin,
Dongsu Park
biovecs has become immutable since v3.13, so it isn't necessary
to allocate biovecs for the new cloned bios, then we can save
one extra biovecs allocation/copy, and the allocation is often
not fixed-length and a bit more expensive.
For example, if the 'max_sectors_kb' of null blk's queue is set
as 16(32 sectors) via sysfs just for making more splits, this patch
can increase throught about ~70% in the sequential read test over
null_blk(direct io, bs: 1M).
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Kent Overstreet <kent.overstreet@gmail.com>
Cc: Ming Lin <ming.l@ssi.samsung.com>
Cc: Dongsu Park <dpark@posteo.net>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
block/blk-merge.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index d088cff..be2493f 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -66,16 +66,13 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
struct bio *bio,
struct bio_set *bs)
{
- struct bio *split;
struct bio_vec bv, bvprv;
struct bvec_iter iter;
unsigned seg_size = 0, nsegs = 0, sectors = 0;
int prev = 0;
bio_for_each_segment(bv, bio, iter) {
- sectors += bv.bv_len >> 9;
-
- if (sectors > queue_max_sectors(q))
+ if (sectors + (bv.bv_len >> 9) > queue_max_sectors(q))
goto split;
/*
@@ -96,6 +93,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
seg_size += bv.bv_len;
bvprv = bv;
prev = 1;
+ sectors += bv.bv_len >> 9;
continue;
}
new_segment:
@@ -106,21 +104,12 @@ new_segment:
bvprv = bv;
prev = 1;
seg_size = bv.bv_len;
+ sectors += bv.bv_len >> 9;
}
return NULL;
split:
- split = bio_clone_bioset(bio, GFP_NOIO, bs);
-
- split->bi_iter.bi_size -= iter.bi_size;
- bio->bi_iter = iter;
-
- if (bio_integrity(bio)) {
- bio_integrity_advance(bio, split->bi_iter.bi_size);
- bio_integrity_trim(split, 0, bio_sectors(split));
- }
-
- return split;
+ return bio_split(bio, sectors, GFP_NOIO, bs);
}
void blk_queue_split(struct request_queue *q, struct bio **bio,
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] block: blk-merge: fast-clone bio when splitting rw bios
2015-09-17 15:13 [PATCH] block: blk-merge: fast-clone bio when splitting rw bios Ming Lei
@ 2015-09-17 15:19 ` Jens Axboe
2015-09-17 15:50 ` Ming Lei
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2015-09-17 15:19 UTC (permalink / raw)
To: Ming Lei, linux-kernel
Cc: Christoph Hellwig, Kent Overstreet, Ming Lin, Dongsu Park
On 09/17/2015 09:13 AM, Ming Lei wrote:
> biovecs has become immutable since v3.13, so it isn't necessary
> to allocate biovecs for the new cloned bios, then we can save
> one extra biovecs allocation/copy, and the allocation is often
> not fixed-length and a bit more expensive.
>
> For example, if the 'max_sectors_kb' of null blk's queue is set
> as 16(32 sectors) via sysfs just for making more splits, this patch
> can increase throught about ~70% in the sequential read test over
> null_blk(direct io, bs: 1M).
I'd be curious how this compares to before we did the splitting, not
exceeding the limits through bio_add_page() instead?
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] block: blk-merge: fast-clone bio when splitting rw bios
2015-09-17 15:19 ` Jens Axboe
@ 2015-09-17 15:50 ` Ming Lei
2015-09-17 15:55 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Ming Lei @ 2015-09-17 15:50 UTC (permalink / raw)
To: Jens Axboe
Cc: Linux Kernel Mailing List, Christoph Hellwig, Kent Overstreet,
Ming Lin, Dongsu Park
On Thu, Sep 17, 2015 at 11:19 PM, Jens Axboe <axboe@kernel.dk> wrote:
> On 09/17/2015 09:13 AM, Ming Lei wrote:
>>
>> biovecs has become immutable since v3.13, so it isn't necessary
>> to allocate biovecs for the new cloned bios, then we can save
>> one extra biovecs allocation/copy, and the allocation is often
>> not fixed-length and a bit more expensive.
>>
>> For example, if the 'max_sectors_kb' of null blk's queue is set
>> as 16(32 sectors) via sysfs just for making more splits, this patch
>> can increase throught about ~70% in the sequential read test over
>> null_blk(direct io, bs: 1M).
>
>
> I'd be curious how this compares to before we did the splitting, not
> exceeding the limits through bio_add_page() instead?
Let me show these test results:
----------------------------------------------------------------------------------
kernel | throught
----------------------------------------------------------------------------------
4.3.0-rc1-next-20150916 | bw=12227MB/s, iops=12227
----------------------------------------------------------------------------------
4.3.0-rc1-next-20150916 with patch | bw=21011MB/s, iops=21011
----------------------------------------------------------------------------------
v4.2 |
bw=18959MB/s, iops=18958
----------------------------------------------------------------------------------
So from the above, looks this patch is kind of fix for performance regression
introduced by 54efd50bfd(block: make generic_make_request handle
arbitrarily sized bios), :-)
Thanks,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] block: blk-merge: fast-clone bio when splitting rw bios
2015-09-17 15:50 ` Ming Lei
@ 2015-09-17 15:55 ` Jens Axboe
2015-09-17 16:01 ` Jens Axboe
2015-09-17 16:08 ` Ming Lei
0 siblings, 2 replies; 6+ messages in thread
From: Jens Axboe @ 2015-09-17 15:55 UTC (permalink / raw)
To: Ming Lei
Cc: Linux Kernel Mailing List, Christoph Hellwig, Kent Overstreet,
Ming Lin, Dongsu Park
On 09/17/2015 09:50 AM, Ming Lei wrote:
> On Thu, Sep 17, 2015 at 11:19 PM, Jens Axboe <axboe@kernel.dk> wrote:
>> On 09/17/2015 09:13 AM, Ming Lei wrote:
>>>
>>> biovecs has become immutable since v3.13, so it isn't necessary
>>> to allocate biovecs for the new cloned bios, then we can save
>>> one extra biovecs allocation/copy, and the allocation is often
>>> not fixed-length and a bit more expensive.
>>>
>>> For example, if the 'max_sectors_kb' of null blk's queue is set
>>> as 16(32 sectors) via sysfs just for making more splits, this patch
>>> can increase throught about ~70% in the sequential read test over
>>> null_blk(direct io, bs: 1M).
>>
>>
>> I'd be curious how this compares to before we did the splitting, not
>> exceeding the limits through bio_add_page() instead?
>
> Let me show these test results:
>
> ----------------------------------------------------------------------------------
> kernel | throught
> ----------------------------------------------------------------------------------
> 4.3.0-rc1-next-20150916 | bw=12227MB/s, iops=12227
> ----------------------------------------------------------------------------------
> 4.3.0-rc1-next-20150916 with patch | bw=21011MB/s, iops=21011
> ----------------------------------------------------------------------------------
> v4.2 |
> bw=18959MB/s, iops=18958
> ----------------------------------------------------------------------------------
>
> So from the above, looks this patch is kind of fix for performance regression
> introduced by 54efd50bfd(block: make generic_make_request handle
> arbitrarily sized bios), :-)
So that's 1MB user IO, and 16KB device limit, correct? If that is the
case, then the results make sense. And looks like we're still ahead of
the older bio_add_page() approach, which is what I mostly cared about.
Thanks! I'll apply this for -rc2.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] block: blk-merge: fast-clone bio when splitting rw bios
2015-09-17 15:55 ` Jens Axboe
@ 2015-09-17 16:01 ` Jens Axboe
2015-09-17 16:08 ` Ming Lei
1 sibling, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2015-09-17 16:01 UTC (permalink / raw)
To: Ming Lei
Cc: Linux Kernel Mailing List, Christoph Hellwig, Kent Overstreet,
Ming Lin, Dongsu Park
On 09/17/2015 09:55 AM, Jens Axboe wrote:
> On 09/17/2015 09:50 AM, Ming Lei wrote:
>> On Thu, Sep 17, 2015 at 11:19 PM, Jens Axboe <axboe@kernel.dk> wrote:
>>> On 09/17/2015 09:13 AM, Ming Lei wrote:
>>>>
>>>> biovecs has become immutable since v3.13, so it isn't necessary
>>>> to allocate biovecs for the new cloned bios, then we can save
>>>> one extra biovecs allocation/copy, and the allocation is often
>>>> not fixed-length and a bit more expensive.
>>>>
>>>> For example, if the 'max_sectors_kb' of null blk's queue is set
>>>> as 16(32 sectors) via sysfs just for making more splits, this patch
>>>> can increase throught about ~70% in the sequential read test over
>>>> null_blk(direct io, bs: 1M).
>>>
>>>
>>> I'd be curious how this compares to before we did the splitting, not
>>> exceeding the limits through bio_add_page() instead?
>>
>> Let me show these test results:
>>
>> ----------------------------------------------------------------------------------
>>
>> kernel | throught
>> ----------------------------------------------------------------------------------
>>
>> 4.3.0-rc1-next-20150916 | bw=12227MB/s, iops=12227
>> ----------------------------------------------------------------------------------
>>
>> 4.3.0-rc1-next-20150916 with patch | bw=21011MB/s, iops=21011
>> ----------------------------------------------------------------------------------
>>
>> v4.2 |
>> bw=18959MB/s, iops=18958
>> ----------------------------------------------------------------------------------
>>
>>
>> So from the above, looks this patch is kind of fix for performance
>> regression
>> introduced by 54efd50bfd(block: make generic_make_request handle
>> arbitrarily sized bios), :-)
>
> So that's 1MB user IO, and 16KB device limit, correct? If that is the
> case, then the results make sense. And looks like we're still ahead of
> the older bio_add_page() approach, which is what I mostly cared about.
> Thanks! I'll apply this for -rc2.
Hand applied, as it didn't apply with the blk-merge.c warning fix at all
(against for-linus). Please double check:
http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus&id=52cc6eead9095e2faf2ec7afc013aa3af1f01ac5
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] block: blk-merge: fast-clone bio when splitting rw bios
2015-09-17 15:55 ` Jens Axboe
2015-09-17 16:01 ` Jens Axboe
@ 2015-09-17 16:08 ` Ming Lei
1 sibling, 0 replies; 6+ messages in thread
From: Ming Lei @ 2015-09-17 16:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Linux Kernel Mailing List, Christoph Hellwig, Kent Overstreet,
Ming Lin, Dongsu Park
On Thu, Sep 17, 2015 at 11:55 PM, Jens Axboe <axboe@kernel.dk> wrote:
> On 09/17/2015 09:50 AM, Ming Lei wrote:
>>
>> On Thu, Sep 17, 2015 at 11:19 PM, Jens Axboe <axboe@kernel.dk> wrote:
>>>
>>> On 09/17/2015 09:13 AM, Ming Lei wrote:
>>>>
>>>>
>>>> biovecs has become immutable since v3.13, so it isn't necessary
>>>> to allocate biovecs for the new cloned bios, then we can save
>>>> one extra biovecs allocation/copy, and the allocation is often
>>>> not fixed-length and a bit more expensive.
>>>>
>>>> For example, if the 'max_sectors_kb' of null blk's queue is set
>>>> as 16(32 sectors) via sysfs just for making more splits, this patch
>>>> can increase throught about ~70% in the sequential read test over
>>>> null_blk(direct io, bs: 1M).
>>>
>>>
>>>
>>> I'd be curious how this compares to before we did the splitting, not
>>> exceeding the limits through bio_add_page() instead?
>>
>>
>> Let me show these test results:
>>
>>
>> ----------------------------------------------------------------------------------
>> kernel | throught
>>
>> ----------------------------------------------------------------------------------
>> 4.3.0-rc1-next-20150916 | bw=12227MB/s, iops=12227
>>
>> ----------------------------------------------------------------------------------
>> 4.3.0-rc1-next-20150916 with patch | bw=21011MB/s, iops=21011
>>
>> ----------------------------------------------------------------------------------
>> v4.2 |
>> bw=18959MB/s, iops=18958
>>
>> ----------------------------------------------------------------------------------
>>
>> So from the above, looks this patch is kind of fix for performance
>> regression
>> introduced by 54efd50bfd(block: make generic_make_request handle
>> arbitrarily sized bios), :-)
>
>
> So that's 1MB user IO, and 16KB device limit, correct? If that is the case,
Yes, exactly, just for showing 'improvement' from the patch by setting
the limit, ;-)
> then the results make sense. And looks like we're still ahead of the older
> bio_add_page() approach, which is what I mostly cared about. Thanks! I'll
> apply this for -rc2.
>
> --
> Jens Axboe
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-17 16:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-17 15:13 [PATCH] block: blk-merge: fast-clone bio when splitting rw bios Ming Lei
2015-09-17 15:19 ` Jens Axboe
2015-09-17 15:50 ` Ming Lei
2015-09-17 15:55 ` Jens Axboe
2015-09-17 16:01 ` Jens Axboe
2015-09-17 16:08 ` Ming Lei
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.