Re: [PATCH v2 7/7] nvme: add reserved ioq tags for cancel

[John Meneghini <jmeneghi@redhat.com>]

On 6/26/24 15:03, Keith Busch wrote:
> On Wed, Jun 26, 2024 at 02:38:19PM -0400, John Meneghini wrote:
>> If the nvme Cancel command is supported, we need to reserve 2 tags for
>> each IO queue. Note that one addition tag is reserved to account for
>> the case where this is a fabrics controller.
>   
>> @@ -4580,9 +4581,13 @@ int nvme_alloc_io_tag_set(struct nvme_ctrl *ctrl, struct blk_mq_tag_set *set,
>>   	 */
>>   	if (ctrl->quirks & NVME_QUIRK_SHARED_TAGS)
>>   		set->reserved_tags = NVME_AQ_DEPTH;
>> +	else if  (effects & NVME_CMD_EFFECTS_CSUPP)
>> +		/* Reserve 2 X io_queue count for NVMe Cancel */
>> +		set->reserved_tags = (2 * ctrl->queue_count);
> 
> The reserved_tags are already per queue, no need to multiply.

Ahh, ok. I wasn't sure about this. So the tag set belongs to the controller, and is shared by all IO queues, but reserved tags 
are reserved for each IO queue.  Let me fix that before I begin testing this.

>>   	else if (ctrl->ops->flags & NVME_F_FABRICS)
>>   		/* Reserved for fabric connect */
>>   		set->reserved_tags = 1;
> 
> You mentioned an addtional tags is supposed to be reserved for fabrics.
> Shouldn't this just be an "if", not an "else if", and then increment
> reserved_tags?

Yes, good point. I was thinking we don't need more than 2 tags if it's a fabric.

So you want to reserve 3 tags when both cancel and fabrics are true?

/John