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.
2) some drivers(hisi_v3 hw, hpsa, megraid_sas, mpt3sas) only support
single hw queue, but has multiple reply queue which can avoid CPU
lockup, so we are working towards converting the private reply queue
into blk-mq hw queue. That is what BLK_MQ_F_HOST_TAGS covers.
Now you think 1) is enough for converting the private reply queue into
blk-mq hw queue, that is definitely not correct since blk-mq MQ doesn't
support shared tags among hw queues.
Please take a look at the patch you posted before, and you will see
the point.
https://marc.info/?l=linux-block&m=149132580511346&w=2
Thanks,
Ming
