Re: [PATCH 03/21] scsi: add scsi_{get,put}_internal_cmd() helper

From: Hannes Reinecke <hare@suse.de>
To: John Garry <john.garry@huawei.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>,
	James Bottomley <james.bottomley@hansenpartnership.com>,
	Bart van Assche <bvanassche@acm.org>,
	Don Brace <don.brace@microchip.com>,
	linux-scsi@vger.kernel.org, chenxiang <chenxiang66@hisilicon.com>
Subject: Re: [PATCH 03/21] scsi: add scsi_{get,put}_internal_cmd() helper
Date: Mon, 7 Dec 2020 16:52:07 +0100	[thread overview]
Message-ID: <0633db5c-0616-e42d-2eda-db43d739294a@suse.de> (raw)
In-Reply-To: <2debadf2-a120-e73f-7fd6-14a3eb89e320@huawei.com>

On 12/7/20 11:15 AM, John Garry wrote:
> On 16/11/2020 09:03, Hannes Reinecke wrote:
>> On 11/16/20 9:56 AM, John Garry wrote:
>>> On 03/07/2020 14:01, Hannes Reinecke wrote:
>>>> Add helper functions to allow LLDDs to allocate and free
>>>> internal commands.
>>>
>>> Hi Hannes,
>>>
>>> Is there any way to ensure that the request allocated is associated 
>>> with some determined HW queue here?
>>>
>>> The reason for this requirement is that sometimes the LLDD must 
>>> submit some internal IO (for which we allocate an "internal command") 
>>> on a specific HW queue. An example of this is internal abort IO 
>>> commands, which should be submitted on the same queue as the IO which 
>>> we are attempting to abort was submitted.
>>>
>>> So, for sure, the LLDD does not have to honor the hwq associated with 
>>> the request and submit on the desired queue, but then we lose the 
>>> blk-mq CPU hotplug protection. And maybe other problems.
>>>
>>> One way to achieve this is to run scsi_get_internal_cmd() on a CPU 
>>> associated with the desired HW queue, but that's a bit hacky. Not 
>>> sure of another way.
>>>
>> Hmm. You are correct for the 'abort' command; that typically needs to 
>> be submitted to a specific hwq.
>>
>> Let me think about it...
>>
> 
> 
> Hannes,
> 
> Earlier you mentioned " some drivers not only the command needs a tag, 
> but the sgls also, thereby completely messing up our mq tags logic.
> So to map those we'd need to allocate _several_ tags for one command ..."
> 
> Which drivers are these? Can you provide a pointer? Is this the blocker?
> 
Not actually a blocker, more something which I had been thinking about.
Point is, that the current blk-mq design assumes that each command 
requires a tag, and each tag maps to a single entity placed on the 
hardware submission queue.
While this is okay for some drivers, it's not a straightforward match 
for other drivers:
- FC uses 'Exchange IDs' to map commands; however, the lifetime for 
these XIDs is slightly different from the command lifetime.
In case of an error the XID must not be re-used (on the same queue) 
until error recovery has taken place. Which means we cannot use the 
block-layer tags for XIDs but have to implement an internal facility for 
that (cf drivers/scsi/libfc/fc_exch.c to get some idea about that).
- Some drivers like smartpqi or mpt3sas require the command _and the 
sgls_ to be placed on the submission queue, and every entity requires an 
unique tag. So again, we cannot leverage the block-layer tags to control 
the submission queue but have to implement a driver-specific thing.

But I'll be rebasing my patches.

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