Messages in this thread | | | Date | Thu, 8 Jun 2023 12:56:35 -1000 | From | Tejun Heo <> | Subject | Re: [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
| |