Re: [PATCH 1/4] block: add mq_ops->queue_rqs hook

[Jens Axboe <axboe@kernel.dk>]

On 12/5/21 2:07 AM, Hannes Reinecke wrote:
> On 12/4/21 9:13 PM, Jens Axboe wrote:
>> On 12/4/21 3:43 AM, Hannes Reinecke wrote:
>>> On 12/3/21 10:45 PM, Jens Axboe wrote:
>>>> If we have a list of requests in our plug list, send it to the driver in
>>>> one go, if possible. The driver must set mq_ops->queue_rqs() to support
>>>> this, if not the usual one-by-one path is used.
>>>>
>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>> ---
>>>>    block/blk-mq.c         | 24 +++++++++++++++++++++---
>>>>    include/linux/blk-mq.h |  8 ++++++++
>>>>    2 files changed, 29 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>>>> index 22ec21aa0c22..9ac9174a2ba4 100644
>>>> --- a/block/blk-mq.c
>>>> +++ b/block/blk-mq.c
>>>> @@ -2513,6 +2513,7 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule)
>>>>    {
>>>>    	struct blk_mq_hw_ctx *this_hctx;
>>>>    	struct blk_mq_ctx *this_ctx;
>>>> +	struct request *rq;
>>>>    	unsigned int depth;
>>>>    	LIST_HEAD(list);
>>>>    
>>>> @@ -2521,7 +2522,26 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule)
>>>>    	plug->rq_count = 0;
>>>>    
>>>>    	if (!plug->multiple_queues && !plug->has_elevator && !from_schedule) {
>>>> -		blk_mq_run_dispatch_ops(plug->mq_list->q,
>>>> +		struct request_queue *q;
>>>> +
>>>> +		rq = plug->mq_list;
>>>> +		q = rq->q;
>>>> +
>>>> +		/*
>>>> +		 * Peek first request and see if we have a ->queue_rqs() hook.
>>>> +		 * If we do, we can dispatch the whole plug list in one go. We
>>>> +		 * already know at this point that all requests belong to the
>>>> +		 * same queue, caller must ensure that's the case.
>>>> +		 */
>>>> +		if (q->mq_ops->queue_rqs &&
>>>> +		    !(rq->mq_hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)) {
>>>
>>> What is the dependency on shared tags here?
>>>   From what I've seen it's just about submitting requests; the only
>>> difference to shared tags is the way the tags are allocated.
>>> Care to explain?
>>
>> For shared tags, we need to actively increment the use count per
>> request. This path doesn't do that, so it's disabled for now. It could
>> be done, but then it'd have to be in the caller, so I'd rather leave it
>> for a future optimization if anyone cares enough about this for shared
>> tags.
>>
>> I can add a comment about it if that helps.
>>
> Please do.
> It'll act as a reminder what needs to be done if and when one of the 
> drivers requiring shared tags is looking at implementing queue_rqs.

I added to the comment yesterday:

https://git.kernel.dk/cgit/linux-block/commit/?h=perf-wip&id=f9f526700607bf804fa8541c824ea54253f4241a

-- 
Jens Axboe