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

[Hannes Reinecke <hare@suse.de>]

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.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer