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. > 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. >> >>>> - if (scsi_cmnd) >>>> - return scsi_cmnd->request->tag; >>>> - >>> >>> Otherwise duplicated slot can be used from different blk-mq hw queue. >>> >>>> >>>>> The worsen thing is that V3's actual max queue depth is (4096 - 96), but >>>>> this patch claims that the device can support (4096 - 96) * 32 command >>>>> slots, finally hisi_sas_slot_index_alloc() is used to respect the actual >>>>> max queue depth(4000). >>>>> >>>> Well, this patch is an RFC to demonstrate my idea. Of course the queue >>>> depth should be identical before and after the conversion. >>> >>> That is why I call it is hard to partition the hostwide tags to MQ. >>> >> It's not. The driver already sets aside a portion of tags for internal >> commands (check HISI_SAS_RESERVED_IPTT_CNT), so it is already >> effectively partitioned. > > I meant partitioning for each hw queue because you said it is host-wide > tags. > > I still don't understand why you can it is host-wide tags, and now each > queue has same tag space with the host-wide tags. > See above. For host-wide tags the tagset is shared between all devices (cf scsi_mq_alloc_queue() and scsi_mq_setup_tags()), so we do not need to partition the tagspace between devices. And as there is only one tagset the sbitmap allocator ensures that each tag is in fact unique across _all_ devices. Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@xxxxxxx +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)
