* [PATCH] bcache: fix unmatched generic_end_io_acct() & generic_start_io_acct()
@ 2018-01-03 4:39 Zhai Zhaoxuan
2018-01-03 5:45 ` Coly Li
2018-01-03 16:57 ` Michael Lyle
0 siblings, 2 replies; 6+ messages in thread
From: Zhai Zhaoxuan @ 2018-01-03 4:39 UTC (permalink / raw)
To: linux-bcache, linux-block; +Cc: tang.junhui, Michael Lyle, Zhai Zhaoxuan
The function cached_dev_make_request() and flash_dev_make_request() call
generic_start_io_acct() with (struct bcache_device)->disk when they start a
closure. Then the function bio_complete() calls generic_end_io_acct() with
(struct search)->orig_bio->bi_disk when the closure has done.
Since the `bi_disk` is not the bcache device, the generic_end_io_acct() is
called with a wrong device queue.
It causes the "inflight" (in struct hd_struct) counter keep increasing
without decreasing.
This patch fix the problem by calling generic_end_io_acct() with
(struct bcache_device)->disk.
Signed-off-by: Zhai Zhaoxuan <kxuanobj@gmail.com>
---
drivers/md/bcache/request.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 643c3021624f..83de85fe4542 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -611,8 +611,8 @@ static void request_endio(struct bio *bio)
static void bio_complete(struct search *s)
{
if (s->orig_bio) {
- struct request_queue *q = s->orig_bio->bi_disk->queue;
- generic_end_io_acct(q, bio_data_dir(s->orig_bio),
+ generic_end_io_acct(s->d->disk->queue,
+ bio_data_dir(s->orig_bio),
&s->d->disk->part0, s->start_time);
trace_bcache_request_end(s->d, s->orig_bio);
--
2.15.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] bcache: fix unmatched generic_end_io_acct() & generic_start_io_acct()
2018-01-03 4:39 [PATCH] bcache: fix unmatched generic_end_io_acct() & generic_start_io_acct() Zhai Zhaoxuan
@ 2018-01-03 5:45 ` Coly Li
2018-01-03 7:05 ` Zhaoxuan
2018-01-03 16:57 ` Michael Lyle
1 sibling, 1 reply; 6+ messages in thread
From: Coly Li @ 2018-01-03 5:45 UTC (permalink / raw)
To: Zhai Zhaoxuan, linux-bcache, linux-block; +Cc: tang.junhui, Michael Lyle
On 03/01/2018 12:39 PM, Zhai Zhaoxuan wrote:
> The function cached_dev_make_request() and flash_dev_make_request() call
> generic_start_io_acct() with (struct bcache_device)->disk when they start a
> closure. Then the function bio_complete() calls generic_end_io_acct() with
> (struct search)->orig_bio->bi_disk when the closure has done.
> Since the `bi_disk` is not the bcache device, the generic_end_io_acct() is
> called with a wrong device queue.
>
> It causes the "inflight" (in struct hd_struct) counter keep increasing
> without decreasing.
>
> This patch fix the problem by calling generic_end_io_acct() with
> (struct bcache_device)->disk.
>
> Signed-off-by: Zhai Zhaoxuan <kxuanobj@gmail.com>
Hi Zhaoxuan,
Nice catch, it makes sense. Thanks.
Reviewed-by: Coly Li <colyli@suse.de>
One more question, do you experience any problem when inflight counter
increases only ? I just wondering.
Coly
> ---
> drivers/md/bcache/request.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index 643c3021624f..83de85fe4542 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -611,8 +611,8 @@ static void request_endio(struct bio *bio)
> static void bio_complete(struct search *s)
> {
> if (s->orig_bio) {
> - struct request_queue *q = s->orig_bio->bi_disk->queue;
> - generic_end_io_acct(q, bio_data_dir(s->orig_bio),
> + generic_end_io_acct(s->d->disk->queue,
> + bio_data_dir(s->orig_bio),
> &s->d->disk->part0, s->start_time);
>
> trace_bcache_request_end(s->d, s->orig_bio);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] bcache: fix unmatched generic_end_io_acct() & generic_start_io_acct()
2018-01-03 5:45 ` Coly Li
@ 2018-01-03 7:05 ` Zhaoxuan
2018-01-03 7:21 ` Coly Li
0 siblings, 1 reply; 6+ messages in thread
From: Zhaoxuan @ 2018-01-03 7:05 UTC (permalink / raw)
To: Coly Li, linux-bcache, linux-block; +Cc: tang.junhui, Michael Lyle
On 03/01/2018 13:45 PM, Coly Li wrote:
> On 03/01/2018 12:39 PM, Zhai Zhaoxuan wrote:
>> The function cached_dev_make_request() and flash_dev_make_request() call
>> generic_start_io_acct() with (struct bcache_device)->disk when they start a
>> closure. Then the function bio_complete() calls generic_end_io_acct() with
>> (struct search)->orig_bio->bi_disk when the closure has done.
>> Since the `bi_disk` is not the bcache device, the generic_end_io_acct() is
>> called with a wrong device queue.
>>
>> It causes the "inflight" (in struct hd_struct) counter keep increasing
>> without decreasing.
>>
>> This patch fix the problem by calling generic_end_io_acct() with
>> (struct bcache_device)->disk.
>>
>> Signed-off-by: Zhai Zhaoxuan <kxuanobj@gmail.com>
> Hi Zhaoxuan,
>
> Nice catch, it makes sense. Thanks.
>
> Reviewed-by: Coly Li <colyli@suse.de>
>
> One more question, do you experience any problem when inflight counter
> increases only ? I just wondering.
Hi Coly,
Thanks for your review.
There is a problem in `iostat -x`. `iostat` shows the bcache device keep
100.00%
busy (100.00 in the "%util" column), even if the bcache is idle.
[root@base ~]# iostat -x -d 1
Linux 4.15.0-rc6-ARCH (base) 01/03/18 _x86_64_ (2 CPU)
Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
avgrq-sz avgqu-sz await r_await w_await svctm %util
vda 7.85 1.07 69.98 13.20 1818.60 158.20
47.54 0.04 1.08 0.67 3.28 0.30 2.47
vdb 0.00 0.00 2.61 0.00 75.80 0.00
58.09 0.00 0.75 0.75 0.00 0.00 0.00
bcache0 0.00 0.00 1.98 0.00 52.00 0.00
52.64 77.73 1.06 1.06 0.00 485.93 96.00
Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
avgrq-sz avgqu-sz await r_await w_await svctm %util
vda 0.00 0.00 0.00 0.00 0.00 0.00
0.00 0.00 0.00 0.00 0.00 0.00 0.00
vdb 0.00 0.00 0.00 0.00 0.00 0.00
0.00 0.00 0.00 0.00 0.00 0.00 0.00
bcache0 0.00 0.00 0.00 0.00 0.00 0.00
0.00 81.00 0.00 0.00 0.00 0.00 100.00
Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
avgrq-sz avgqu-sz await r_await w_await svctm %util
vda 0.00 9.00 0.00 2.00 0.00 44.00
44.00 0.00 10.00 0.00 10.00 0.00 0.00
vdb 0.00 0.00 0.00 0.00 0.00 0.00
0.00 0.00 0.00 0.00 0.00 0.00 0.00
bcache0 0.00 0.00 0.00 0.00 0.00 0.00
0.00 81.00 0.00 0.00 0.00 0.00 100.00
Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
avgrq-sz avgqu-sz await r_await w_await svctm %util
vda 0.00 0.00 0.00 0.00 0.00 0.00
0.00 0.00 0.00 0.00 0.00 0.00 0.00
vdb 0.00 0.00 0.00 0.00 0.00 0.00
0.00 0.00 0.00 0.00 0.00 0.00 0.00
bcache0 0.00 0.00 0.00 0.00 0.00 0.00
0.00 81.00 0.00 0.00 0.00 0.00 100.00
^C
[root@base ~]# bcache-super-show /dev/vdb
sb.magic ok
sb.first_sector 8 [match]
sb.csum 54824DA53D2DC101 [match]
sb.version 1 [backing device]
dev.label (empty)
dev.uuid c5e43e66-4b28-48d7-bd46-73fb6e1efdbb
dev.sectors_per_block 1
dev.sectors_per_bucket 1024
dev.data.first_sector 16
dev.data.cache_mode 0 [writethrough]
dev.data.cache_state 1 [clean]
cset.uuid a79f8422-ed64-4b92-8688-692f48cd6d97
[root@base ~]# bcache-super-show /dev/vda2
sb.magic ok
sb.first_sector 8 [match]
sb.csum EF7AB1A8928862E6 [match]
sb.version 3 [cache device]
dev.label (empty)
dev.uuid 5762b745-77f8-4831-81ce-9ee7866b8d84
dev.sectors_per_block 1
dev.sectors_per_bucket 1024
dev.cache.first_sector 1024
dev.cache.cache_sectors 2096128
dev.cache.total_sectors 2097152
dev.cache.ordered yes
dev.cache.discard no
dev.cache.pos 0
dev.cache.replacement 0 [lru]
cset.uuid a79f8422-ed64-4b92-8688-692f48cd6d97
> Coly
>
>> ---
>> drivers/md/bcache/request.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
>> index 643c3021624f..83de85fe4542 100644
>> --- a/drivers/md/bcache/request.c
>> +++ b/drivers/md/bcache/request.c
>> @@ -611,8 +611,8 @@ static void request_endio(struct bio *bio)
>> static void bio_complete(struct search *s)
>> {
>> if (s->orig_bio) {
>> - struct request_queue *q = s->orig_bio->bi_disk->queue;
>> - generic_end_io_acct(q, bio_data_dir(s->orig_bio),
>> + generic_end_io_acct(s->d->disk->queue,
>> + bio_data_dir(s->orig_bio),
>> &s->d->disk->part0, s->start_time);
>>
>> trace_bcache_request_end(s->d, s->orig_bio);
>>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] bcache: fix unmatched generic_end_io_acct() & generic_start_io_acct()
2018-01-03 7:05 ` Zhaoxuan
@ 2018-01-03 7:21 ` Coly Li
0 siblings, 0 replies; 6+ messages in thread
From: Coly Li @ 2018-01-03 7:21 UTC (permalink / raw)
To: Zhaoxuan, linux-bcache, linux-block; +Cc: tang.junhui, Michael Lyle
On 03/01/2018 3:05 PM, Zhaoxuan wrote:
>
> On 03/01/2018 13:45 PM, Coly Li wrote:
>> On 03/01/2018 12:39 PM, Zhai Zhaoxuan wrote:
>>> The function cached_dev_make_request() and flash_dev_make_request() call
>>> generic_start_io_acct() with (struct bcache_device)->disk when they
>>> start a
>>> closure. Then the function bio_complete() calls generic_end_io_acct()
>>> with
>>> (struct search)->orig_bio->bi_disk when the closure has done.
>>> Since the `bi_disk` is not the bcache device, the
>>> generic_end_io_acct() is
>>> called with a wrong device queue.
>>>
>>> It causes the "inflight" (in struct hd_struct) counter keep increasing
>>> without decreasing.
>>>
>>> This patch fix the problem by calling generic_end_io_acct() with
>>> (struct bcache_device)->disk.
>>>
>>> Signed-off-by: Zhai Zhaoxuan <kxuanobj@gmail.com>
>> Hi Zhaoxuan,
>>
>> Nice catch, it makes sense. Thanks.
>>
>> Reviewed-by: Coly Li <colyli@suse.de>
>>
>> One more question, do you experience any problem when inflight counter
>> increases only ? I just wondering.
>
> Hi Coly,
>
> Thanks for your review.
>
> There is a problem in `iostat -x`. `iostat` shows the bcache device keep
> 100.00%
> busy (100.00 in the "%util" column), even if the bcache is idle.
>
> [root@base ~]# iostat -x -d 1
> Linux 4.15.0-rc6-ARCH (base) 01/03/18 _x86_64_ (2 CPU)
>
> Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
> avgrq-sz avgqu-sz await r_await w_await svctm %util
> vda 7.85 1.07 69.98 13.20 1818.60 158.20
> 47.54 0.04 1.08 0.67 3.28 0.30 2.47
> vdb 0.00 0.00 2.61 0.00 75.80 0.00
> 58.09 0.00 0.75 0.75 0.00 0.00 0.00
> bcache0 0.00 0.00 1.98 0.00 52.00 0.00
> 52.64 77.73 1.06 1.06 0.00 485.93 96.00
>
> Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
> avgrq-sz avgqu-sz await r_await w_await svctm %util
> vda 0.00 0.00 0.00 0.00 0.00 0.00
> 0.00 0.00 0.00 0.00 0.00 0.00 0.00
> vdb 0.00 0.00 0.00 0.00 0.00 0.00
> 0.00 0.00 0.00 0.00 0.00 0.00 0.00
> bcache0 0.00 0.00 0.00 0.00 0.00 0.00
> 0.00 81.00 0.00 0.00 0.00 0.00 100.00
>
> Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
> avgrq-sz avgqu-sz await r_await w_await svctm %util
> vda 0.00 9.00 0.00 2.00 0.00 44.00
> 44.00 0.00 10.00 0.00 10.00 0.00 0.00
> vdb 0.00 0.00 0.00 0.00 0.00 0.00
> 0.00 0.00 0.00 0.00 0.00 0.00 0.00
> bcache0 0.00 0.00 0.00 0.00 0.00 0.00
> 0.00 81.00 0.00 0.00 0.00 0.00 100.00
>
> Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
> avgrq-sz avgqu-sz await r_await w_await svctm %util
> vda 0.00 0.00 0.00 0.00 0.00 0.00
> 0.00 0.00 0.00 0.00 0.00 0.00 0.00
> vdb 0.00 0.00 0.00 0.00 0.00 0.00
> 0.00 0.00 0.00 0.00 0.00 0.00 0.00
> bcache0 0.00 0.00 0.00 0.00 0.00 0.00
> 0.00 81.00 0.00 0.00 0.00 0.00 100.00
>
> ^C
> [root@base ~]# bcache-super-show /dev/vdb
> sb.magic ok
> sb.first_sector 8 [match]
> sb.csum 54824DA53D2DC101 [match]
> sb.version 1 [backing device]
>
> dev.label (empty)
> dev.uuid c5e43e66-4b28-48d7-bd46-73fb6e1efdbb
> dev.sectors_per_block 1
> dev.sectors_per_bucket 1024
> dev.data.first_sector 16
> dev.data.cache_mode 0 [writethrough]
> dev.data.cache_state 1 [clean]
>
> cset.uuid a79f8422-ed64-4b92-8688-692f48cd6d97
> [root@base ~]# bcache-super-show /dev/vda2
> sb.magic ok
> sb.first_sector 8 [match]
> sb.csum EF7AB1A8928862E6 [match]
> sb.version 3 [cache device]
>
> dev.label (empty)
> dev.uuid 5762b745-77f8-4831-81ce-9ee7866b8d84
> dev.sectors_per_block 1
> dev.sectors_per_bucket 1024
> dev.cache.first_sector 1024
> dev.cache.cache_sectors 2096128
> dev.cache.total_sectors 2097152
> dev.cache.ordered yes
> dev.cache.discard no
> dev.cache.pos 0
> dev.cache.replacement 0 [lru]
>
> cset.uuid a79f8422-ed64-4b92-8688-692f48cd6d97
>
Hi Zhaoxuan,
Cool, this is an already reported issue but I still have no idea how to
fix it yet... Thank you for the fix and the above detailed information!
Coly Li
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcache: fix unmatched generic_end_io_acct() & generic_start_io_acct()
2018-01-03 4:39 [PATCH] bcache: fix unmatched generic_end_io_acct() & generic_start_io_acct() Zhai Zhaoxuan
2018-01-03 5:45 ` Coly Li
@ 2018-01-03 16:57 ` Michael Lyle
1 sibling, 0 replies; 6+ messages in thread
From: Michael Lyle @ 2018-01-03 16:57 UTC (permalink / raw)
To: Zhai Zhaoxuan, linux-bcache, linux-block; +Cc: tang.junhui
On 01/02/2018 08:39 PM, Zhai Zhaoxuan wrote:
> The function cached_dev_make_request() and flash_dev_make_request() call
> generic_start_io_acct() with (struct bcache_device)->disk when they start a
> closure. Then the function bio_complete() calls generic_end_io_acct() with
> (struct search)->orig_bio->bi_disk when the closure has done.
> Since the `bi_disk` is not the bcache device, the generic_end_io_acct() is
> called with a wrong device queue.
>
> It causes the "inflight" (in struct hd_struct) counter keep increasing
> without decreasing.
>
> This patch fix the problem by calling generic_end_io_acct() with
> (struct bcache_device)->disk.
>
> Signed-off-by: Zhai Zhaoxuan <kxuanobj@gmail.com>
Verified, looks good to me, added to my for-next staging branch.
Reviewed-by: Michael Lyle <mlyle@lyle.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcache: fix unmatched generic_end_io_acct() &generic_start_io_acct()
@ 2018-01-02 16:43 tang.junhui
0 siblings, 0 replies; 6+ messages in thread
From: tang.junhui @ 2018-01-02 16:43 UTC (permalink / raw)
To: kxuanobj; +Cc: mlyle, linux-bcache, linux-block, tang.junhui
From: Tang Junhui <tang.junhui@zte.com.cn>
>The function cached_dev_make_request() and flash_dev_make_request() call
>generic_start_io_acct() with (struct bcache_device)->disk when they start a
>closure. Then the function bio_complete() calls generic_end_io_acct() with
>(struct search)->orig_bio->bi_disk when the closure has done.
>Since the `bi_disk` is not the bcache device, the generic_end_io_acct() is
>called with a wrong device queue.
>
>It causes the "inflight" (in struct hd_struct) counter keep increasing
>without decreasing.
>
>This patch fix the problem by calling generic_end_io_acct() with
>(struct bcache_device)->disk.
>
>Signed-off-by: Zhai Zhaoxuan <kxuanobj@gmail.com>
>---
> drivers/md/bcache/request.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
>index 643c3021624f..83de85fe4542 100644
>--- a/drivers/md/bcache/request.c
>+++ b/drivers/md/bcache/request.c
>@@ -611,8 +611,8 @@ static void request_endio(struct bio *bio)
> static void bio_complete(struct search *s)
> {
> if (s->orig_bio) {
>- struct request_queue *q = s->orig_bio->bi_disk->queue;
>- generic_end_io_acct(q, bio_data_dir(s->orig_bio),
>+ generic_end_io_acct(s->d->disk->queue,
>+ bio_data_dir(s->orig_bio),
> &s->d->disk->part0, s->start_time);
>
> trace_bcache_request_end(s->d, s->orig_bio);
>--
>2.15.1
LGTM.
Reviewed-by: Tang Junhui <tang.junhui@zte.com.cn>
Thanks,
Tang Junhui
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-03 16:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-03 4:39 [PATCH] bcache: fix unmatched generic_end_io_acct() & generic_start_io_acct() Zhai Zhaoxuan
2018-01-03 5:45 ` Coly Li
2018-01-03 7:05 ` Zhaoxuan
2018-01-03 7:21 ` Coly Li
2018-01-03 16:57 ` Michael Lyle
-- strict thread matches above, loose matches on Subject: below --
2018-01-02 16:43 [PATCH] bcache: fix unmatched generic_end_io_acct() &generic_start_io_acct() tang.junhui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox