On 2023/6/27 04:46, Tejun Heo wrote: > Hello, > > I only glanced the blk-mq core part but in general this looks a lot better > than the previous one. Thanks for your review! > > On Mon, Jun 26, 2023 at 01:04:05PM +0800, chengming.zhou@xxxxxxxxx wrote: >> Note we don't skip setting alloc_time_ns and start_time_ns for all >> pre-allocated rq, since the first returned rq still need to be set. > > This part is a bit curious for me tho. Why do we need to set it at batch > allocation time and then at actual dispensing from the bat later? Who uses > the alloc time stamp inbetween? > Yes, this part should need more explanation, and my explanation is not clear. Now the batched pre-allocation code looks like this: ``` if (!rq_list_empty(plug->cached_rq)) get pre-allocated rq from plug cache // we set alloc and start time here return rq else rq = __blk_mq_alloc_requests_batch() do batched allocation (1) (2) return rq ``` In (1) we alloc some requests and push them in plug list, and pop one request to return to use. So this popped one request need to be set time at batch allocation time. Yes, we can also set this popped request time in (2), just before return it to use. Since all requests in batch allocation use the same alloc and start time, so this patch just leave it as it is, and reset it at actual used time. I think both way is ok, do you think it's better to just set the popped one request, leave other requests time to 0 ? If so, I can update to do it. Thanks.
