On 6/3/19 11:31 AM, Ming Lei wrote:
> On Mon, Jun 03, 2019 at 10:47:24AM +0200, Hannes Reinecke wrote:
>> On 6/3/19 10:16 AM, Ming Lei wrote:
>>> On Mon, Jun 03, 2019 at 09:46:39AM +0200, Hannes Reinecke wrote:
>>>> On 6/3/19 9:37 AM, Ming Lei wrote:
>>>>> On Mon, Jun 03, 2019 at 08:08:18AM +0200, Hannes Reinecke wrote:
>>>>>> On 6/1/19 1:06 AM, Ming Lei wrote:
>>>>>>> On Fri, May 31, 2019 at 12:26:56PM +0200, Hannes Reinecke wrote:
>>>>>>>> On 5/31/19 10:46 AM, Ming Lei wrote:
>>>>>> [ .. ]
>>>>>>>> First we check for the 'slot_index_alloc()' callback to handle weird v2
>>>>>>>> allocation rules, _then_ we look for a tag, and only if we do _not_
>>>>>>>> have
>>>>>>>> a tag we're using the bitmap.
>>>>>>>
>>>>>>> OK, looks I miss the above change.
>>>>>>>
>>>>>>>> And the bitmap is already correctly sized, as otherwise we'd have a
>>>>>>>> clash between internal and tagged I/O commands even now.
>>>>>>>
>>>>>>> But now the big problem is in the following two line code:
>>>>>>>
>>>>>>> + else if (blk_tag != (u32)-1)
>>>>>>> + rc = blk_mq_unique_tag_to_tag(blk_tag);
>>>>>>>
>>>>>>> Request from different blk-mq hw queue has same tag returned from
>>>>>>> blk_mq_unique_tag_to_tag().
>>>>>>>
>>>>>> Yes, but the sbitmap allocator will ensure that each command will get a
>>>>>> unique tag.
>>>>>
>>>>> Each hw queue has independent sbitmap allocator, so commands with same
>>>>> tag can come from different hw queue.
>>>>>
>>>> It does not for SCSI.
>>>> See below.
>>>>
>>>>> So you meant this RFC patch depends on the host-wide tags patchset I
>>>>> posted?
>>>>>
>>>>>>
>>>>>>> Now the biggest question is that if V3 hw supports per-queue tags,
>>>>>>> If yes, it should be real MQ hardware, otherwise I guess commands with
>>>>>>> same tag at the same time may not work for host-wide tags.
>>>>>>>
>>>>>>
>>>>>> Of course you can't have different commands with the same tag. But the
>>>>>> sbitmap allocator prevents this from happening, as for host-wide tags
>>>>>> the tagset is _shared_ between all devices, so the sbitmap allocator
>>>>>> will only ever run on _one_ tagset for all commands.
>>>>>
>>>>> But blk-mq doesn't support host-wide tags yet, so how can this single
>>>>> patch work?
>>>>>
>>>> Wrong. It does:
>>>>
>>>> struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev)
>>>> {
>>>> sdev->request_queue = blk_mq_init_queue(&sdev->host->tag_set);
>>>> if (IS_ERR(sdev->request_queue))
>>>> return NULL;
>>>>
>>>> sdev->request_queue->queuedata = sdev;
>>>> __scsi_init_queue(sdev->host, sdev->request_queue);
>>>> blk_queue_flag_set(QUEUE_FLAG_SCSI_PASSTHROUGH, sdev->request_queue);
>>>> return sdev->request_queue;
>>>> }
>>>>
>>>>
>>>> IE every scsi device is using the tagset from the host.
>>>
>>> Looks we are not in the same page, and you misunderstood two concepts:
>>> scsi's host-wide tagset, and the new host-tags of BLK_MQ_F_HOST_TAGS.
>>>
>>> I admit that the new flag of BLK_MQ_F_HOST_TAGS is misleading.
>>>
>>> Now let me clarify it a bit:
>>>
>>> 1) the current SCSI hostwide tags means all LUNs share the host tagset,
>>> but the tagset may include multiple hw queues, and each hw queue still
>>> has independent tags, that is why blk-mq provides blk_mq_unique_tag().
>>> In short, each LUN's hw queue has independent tags.
>>>
>> Which is where I fundamentally disagree.
>> Each hw queue does _not_ have independent tags.
>> Each hw queue will use tags from the same (host-wide) tagset; the tags
>> themselves will be allocated for each queue on an ad-hoc base, ie there
>> is no fixed mapping between tag values and hardware queues.
>
> Tagset is set of tags, and one tags is for serving one hw queue.
>
> Each hw queue has its own tags, please see __blk_mq_alloc_rq_map()
> in which standalone sbitmap allocator and rq pool is allocated to
> each hw queue represented by 'hctx_idx'.
>
Yes, but ...
> And for each hw queue, the allocated tag value for request is in
> the range of 0 ~ queue_depth - 1, that is why I say requests from
> different hw queue may have same tag.
>
But this is not what I have been observing working with lpfc and qla2xxx.
Both drivers have been converted to using scsi-mq with nr_hw_queues > 1
some years ago, and do work just fine.
And none of those drivers allow for re-using an in-flight tag on
different hardware queues.
If your reasoning is correct none of these drivers would work.
> Your RFC patch changes to allow requests with same tag submitted to driver
> & hardware at the same time, so we should double-check if hisi_v3 hardware
> is happy with this change.
>
> John, is hisi_sas v3 fine with this way?
>
As mentioned above, I don't think this can happen.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
