On 5/31/19 11:42 AM, John Garry wrote:
>>>>>
>>>>> -static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba,
>>>>> - struct scsi_cmnd *scsi_cmnd)
>>>>> +static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba)
>>>>> {
>>>>> int index;
>>>>> void *bitmap = hisi_hba->slot_index_tags;
>>>>> unsigned long flags;
>>>>>
>>>>> - if (scsi_cmnd)
>>>>> - return scsi_cmnd->request->tag;
>>>>> -
>>>>> spin_lock_irqsave(&hisi_hba->lock, flags);
>>>>> index = find_next_zero_bit(bitmap, hisi_hba->slot_index_count,
>>>>> hisi_hba->last_slot_index + 1);
>>>>
>>>> Then you switch to hisi_sas_slot_index_alloc() for allocating the
>>>> unique
>>>> tag via spin_lock & find_next_zero_bit(). Do you think this way is more
>>>> efficient than blk-mq's sbitmap?
>
> These are not fast path, as used only for TMF, internal IO, etc.
>
> Having said that, hopefully we can move to scsi_{get,put}_reserved_cmd()
> when available, so that the LLDD has to stop managing them.
>
>>>>
>>> slot_index_alloc() is only used for commands which do _not_ have a tag
>>> (eg internal commands), or for v2 hardware which has weird allocation
>>> rules.
>>
>> But this patch has switched to this allocation unconditionally for all
>> commands:
>>
>
> As Hannes said, v2 had a few bugs which meant that we had to make a
> specific version of this function for that hw revision, cf.
> slot_index_alloc_quirk_v2_hw(), and it cannot use request queue tag.
>
> But, indeed, we could consider sbitmap for v2 hw. I'm not sure if it
> would help, considering the weird rules.
>
We should be able to get away by shifting all tags by 1 to the left,
and adding '1' to SMP/SAS commands, and '2' to STP commands, no?
Then index '0' would be free, and the allocation rules are satisfied.
I'll see to whip up a patch.
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)
