* [PATCH -next 0/2] block: fix cached time in plug
@ 2024-04-11 3:23 Yu Kuai
2024-04-11 3:23 ` [PATCH -next 1/2] block: fix that blk_time_get_ns() doesn't update time after schedule Yu Kuai
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Yu Kuai @ 2024-04-11 3:23 UTC (permalink / raw
To: axboe, johannes.thumshirn
Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun
From: Yu Kuai <yukuai3@huawei.com>
Yu Kuai (2):
block: fix that blk_time_get_ns() doesn't update time after schedule
block: add plug while submitting IO
block/blk-core.c | 7 +++++++
1 file changed, 7 insertions(+)
--
2.39.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH -next 1/2] block: fix that blk_time_get_ns() doesn't update time after schedule
2024-04-11 3:23 [PATCH -next 0/2] block: fix cached time in plug Yu Kuai
@ 2024-04-11 3:23 ` Yu Kuai
2024-04-11 16:44 ` Jens Axboe
2024-04-11 3:23 ` [PATCH -next 2/2] block: add plug while submitting IO Yu Kuai
2024-04-12 14:33 ` (subset) [PATCH -next 0/2] block: fix cached time in plug Jens Axboe
2 siblings, 1 reply; 7+ messages in thread
From: Yu Kuai @ 2024-04-11 3:23 UTC (permalink / raw
To: axboe, johannes.thumshirn
Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun
From: Yu Kuai <yukuai3@huawei.com>
While monitoring the throttle time of IO from iocost, it's found that
such time is always zero after the io_schedule() from ioc_rqos_throttle,
for example, with the following debug patch:
+ printk("%s-%d: %s enter %llu\n", current->comm, current->pid, __func__, blk_time_get_ns());
while (true) {
set_current_state(TASK_UNINTERRUPTIBLE);
if (wait.committed)
break;
io_schedule();
}
+ printk("%s-%d: %s exit %llu\n", current->comm, current->pid, __func__, blk_time_get_ns());
It can be observerd that blk_time_get_ns() always return the same time:
[ 1068.096579] fio-1268: ioc_rqos_throttle enter 1067901962288
[ 1068.272587] fio-1268: ioc_rqos_throttle exit 1067901962288
[ 1068.274389] fio-1268: ioc_rqos_throttle enter 1067901962288
[ 1068.472690] fio-1268: ioc_rqos_throttle exit 1067901962288
[ 1068.474485] fio-1268: ioc_rqos_throttle enter 1067901962288
[ 1068.672656] fio-1268: ioc_rqos_throttle exit 1067901962288
[ 1068.674451] fio-1268: ioc_rqos_throttle enter 1067901962288
[ 1068.872655] fio-1268: ioc_rqos_throttle exit 1067901962288
And I think the root cause is that 'PF_BLOCK_TS' is always cleared
by blk_flush_plug() before scheduel(), hence blk_plug_invalidate_ts()
will never be called:
blk_time_get_ns
plug->cur_ktime = ktime_get_ns();
current->flags |= PF_BLOCK_TS;
io_schedule:
io_schedule_prepare
blk_flush_plug
__blk_flush_plug
/* the flag is cleared, while time is not */
current->flags &= ~PF_BLOCK_TS;
schedule
sched_update_worker
/* the flag is not set, hence plug->cur_ktime is not cleared */
if (tsk->flags & PF_BLOCK_TS)
blk_plug_invalidate_ts()
blk_time_get_ns
/* got the time stashed before schedule */
return plug->cur_ktime;
Fix the problem by clearing cached time in __blk_flush_plug().
Fixes: 06b23f92af87 ("block: update cached timestamp post schedule/preemption")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
block/blk-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/blk-core.c b/block/blk-core.c
index a16b5abdbbf5..e317d7bc0696 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1195,6 +1195,7 @@ void __blk_flush_plug(struct blk_plug *plug, bool from_schedule)
if (unlikely(!rq_list_empty(plug->cached_rq)))
blk_mq_free_plug_rqs(plug);
+ plug->cur_ktime = 0;
current->flags &= ~PF_BLOCK_TS;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH -next 2/2] block: add plug while submitting IO
2024-04-11 3:23 [PATCH -next 0/2] block: fix cached time in plug Yu Kuai
2024-04-11 3:23 ` [PATCH -next 1/2] block: fix that blk_time_get_ns() doesn't update time after schedule Yu Kuai
@ 2024-04-11 3:23 ` Yu Kuai
2024-04-12 14:33 ` (subset) [PATCH -next 0/2] block: fix cached time in plug Jens Axboe
2 siblings, 0 replies; 7+ messages in thread
From: Yu Kuai @ 2024-04-11 3:23 UTC (permalink / raw
To: axboe, johannes.thumshirn
Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun
From: Yu Kuai <yukuai3@huawei.com>
So that if caller didn't use plug, for example, __blkdev_direct_IO_simple()
and __blkdev_direct_IO_async(), block layer can still benefit from caching
nsec time in the plug. And if caller already use plug, then there should be
no affect.
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
block/blk-core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/block/blk-core.c b/block/blk-core.c
index e317d7bc0696..c833bc875a61 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -647,11 +647,13 @@ static void __submit_bio(struct bio *bio)
static void __submit_bio_noacct(struct bio *bio)
{
struct bio_list bio_list_on_stack[2];
+ struct blk_plug plug;
BUG_ON(bio->bi_next);
bio_list_init(&bio_list_on_stack[0]);
current->bio_list = bio_list_on_stack;
+ blk_start_plug(&plug);
do {
struct request_queue *q = bdev_get_queue(bio->bi_bdev);
@@ -685,19 +687,23 @@ static void __submit_bio_noacct(struct bio *bio)
bio_list_merge(&bio_list_on_stack[0], &bio_list_on_stack[1]);
} while ((bio = bio_list_pop(&bio_list_on_stack[0])));
+ blk_finish_plug(&plug);
current->bio_list = NULL;
}
static void __submit_bio_noacct_mq(struct bio *bio)
{
struct bio_list bio_list[2] = { };
+ struct blk_plug plug;
current->bio_list = bio_list;
+ blk_start_plug(&plug);
do {
__submit_bio(bio);
} while ((bio = bio_list_pop(&bio_list[0])));
+ blk_finish_plug(&plug);
current->bio_list = NULL;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH -next 1/2] block: fix that blk_time_get_ns() doesn't update time after schedule
2024-04-11 3:23 ` [PATCH -next 1/2] block: fix that blk_time_get_ns() doesn't update time after schedule Yu Kuai
@ 2024-04-11 16:44 ` Jens Axboe
2024-04-12 1:24 ` Yu Kuai
0 siblings, 1 reply; 7+ messages in thread
From: Jens Axboe @ 2024-04-11 16:44 UTC (permalink / raw
To: Yu Kuai, johannes.thumshirn
Cc: linux-block, linux-kernel, yukuai3, yi.zhang, yangerkun
On 4/10/24 9:23 PM, Yu Kuai wrote:
> diff --git a/block/blk-core.c b/block/blk-core.c
> index a16b5abdbbf5..e317d7bc0696 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1195,6 +1195,7 @@ void __blk_flush_plug(struct blk_plug *plug, bool from_schedule)
> if (unlikely(!rq_list_empty(plug->cached_rq)))
> blk_mq_free_plug_rqs(plug);
>
> + plug->cur_ktime = 0;
> current->flags &= ~PF_BLOCK_TS;
> }
We can just use blk_plug_invalidate_ts() here, but not really important.
I think this one should go into 6.9, and patch 2 should go into 6.10,
however.
--
Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next 1/2] block: fix that blk_time_get_ns() doesn't update time after schedule
2024-04-11 16:44 ` Jens Axboe
@ 2024-04-12 1:24 ` Yu Kuai
2024-04-12 14:33 ` Jens Axboe
0 siblings, 1 reply; 7+ messages in thread
From: Yu Kuai @ 2024-04-12 1:24 UTC (permalink / raw
To: Jens Axboe, Yu Kuai, johannes.thumshirn
Cc: linux-block, linux-kernel, yi.zhang, yangerkun
Hi,
在 2024/04/12 0:44, Jens Axboe 写道:
> On 4/10/24 9:23 PM, Yu Kuai wrote:
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index a16b5abdbbf5..e317d7bc0696 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -1195,6 +1195,7 @@ void __blk_flush_plug(struct blk_plug *plug, bool from_schedule)
>> if (unlikely(!rq_list_empty(plug->cached_rq)))
>> blk_mq_free_plug_rqs(plug);
>>
>> + plug->cur_ktime = 0;
>> current->flags &= ~PF_BLOCK_TS;
>> }
>
> We can just use blk_plug_invalidate_ts() here, but not really important.
> I think this one should go into 6.9, and patch 2 should go into 6.10,
> however.
This sounds great! Do you want me to update and send them separately?
Thanks,
Kuai
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: (subset) [PATCH -next 0/2] block: fix cached time in plug
2024-04-11 3:23 [PATCH -next 0/2] block: fix cached time in plug Yu Kuai
2024-04-11 3:23 ` [PATCH -next 1/2] block: fix that blk_time_get_ns() doesn't update time after schedule Yu Kuai
2024-04-11 3:23 ` [PATCH -next 2/2] block: add plug while submitting IO Yu Kuai
@ 2024-04-12 14:33 ` Jens Axboe
2 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2024-04-12 14:33 UTC (permalink / raw
To: johannes.thumshirn, Yu Kuai
Cc: linux-block, linux-kernel, yukuai3, yi.zhang, yangerkun
On Thu, 11 Apr 2024 11:23:47 +0800, Yu Kuai wrote:
> Yu Kuai (2):
> block: fix that blk_time_get_ns() doesn't update time after schedule
> block: add plug while submitting IO
>
> block/blk-core.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> [...]
Applied, thanks!
[1/2] block: fix that blk_time_get_ns() doesn't update time after schedule
commit: 3ec4848913d695245716ea45ca4872d9dff097a5
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next 1/2] block: fix that blk_time_get_ns() doesn't update time after schedule
2024-04-12 1:24 ` Yu Kuai
@ 2024-04-12 14:33 ` Jens Axboe
0 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2024-04-12 14:33 UTC (permalink / raw
To: Yu Kuai, Yu Kuai, johannes.thumshirn
Cc: linux-block, linux-kernel, yi.zhang, yangerkun
On 4/11/24 7:24 PM, Yu Kuai wrote:
> Hi,
>
> ? 2024/04/12 0:44, Jens Axboe ??:
>> On 4/10/24 9:23 PM, Yu Kuai wrote:
>>> diff --git a/block/blk-core.c b/block/blk-core.c
>>> index a16b5abdbbf5..e317d7bc0696 100644
>>> --- a/block/blk-core.c
>>> +++ b/block/blk-core.c
>>> @@ -1195,6 +1195,7 @@ void __blk_flush_plug(struct blk_plug *plug, bool from_schedule)
>>> if (unlikely(!rq_list_empty(plug->cached_rq)))
>>> blk_mq_free_plug_rqs(plug);
>>> + plug->cur_ktime = 0;
>>> current->flags &= ~PF_BLOCK_TS;
>>> }
>>
>> We can just use blk_plug_invalidate_ts() here, but not really important.
>> I think this one should go into 6.9, and patch 2 should go into 6.10,
>> however.
>
> This sounds great! Do you want me to update and send them separately?
I've applied 1/2 separately, so just resend 2/2 when -rc4 has been
tagged and I'll get that one queued for 6.10.
--
Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-04-12 14:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-11 3:23 [PATCH -next 0/2] block: fix cached time in plug Yu Kuai
2024-04-11 3:23 ` [PATCH -next 1/2] block: fix that blk_time_get_ns() doesn't update time after schedule Yu Kuai
2024-04-11 16:44 ` Jens Axboe
2024-04-12 1:24 ` Yu Kuai
2024-04-12 14:33 ` Jens Axboe
2024-04-11 3:23 ` [PATCH -next 2/2] block: add plug while submitting IO Yu Kuai
2024-04-12 14:33 ` (subset) [PATCH -next 0/2] block: fix cached time in plug Jens Axboe
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.