lkml.org 
[lkml]   [2023]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] blk-mq: fix incorrect rq start_time_ns and alloc_time_ns after throttled
Hello,

On Tue, Jun 06, 2023 at 06:22:28PM +0800, Chengming Zhou wrote:
...
> But for plug batched allocation introduced by the commit 47c122e35d7e
> ("block: pre-allocate requests if plug is started and is a batch"), we can
> rq_qos_throttle() after the allocation of the request. This is what the
> blk_mq_get_cached_request() does.
>
> In this case, the cached request alloc_time_ns or start_time_ns is much ahead
> if block in any qos ->throttle().

Ah, okay, that's problematic.

> >> This patch add nr_flush counter in blk_plug, so we can tell if the task
> >> has throttled in any qos ->throttle(), in which case we need to correct
> >> the rq start_time_ns and alloc_time_ns.
> >>
> >> Another solution may be make rq_qos_throttle() return bool to indicate
> >> if it has throttled in any qos ->throttle(). But this need more changes.
> >>
> >> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
> >
> > Depending on the flush behavior and adjusting alloc_time_ns seems fragile to
> > me and will likely confuse other users of alloc_time_ns too.
>
> I agree with you, this code is not good. My basic idea is to adjust the cached
> request alloc_time_ns and start_time_ns when throttled.

Would it make sense to skip setting the alloc_time_ns during pre-allocation
and set it later when the pre-allocated rq is actually used? That should
jive better.

Thanks.

--
tejun

\
 
 \ /
  Last update: 2023-06-09 00:57    [W:0.065 / U:1.280 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site