Re: [PATCH 04/24] scsi: core: Implement reserved command handling

[Hannes Reinecke <hare@suse.de>]

On 4/16/25 09:16, John Garry wrote:
> On 15/04/2025 18:21, Bart Van Assche wrote:
>> Using blk_mq_tag_to_rq() to translate a tag of a reserved
>> request into a request pointer only works after the block layer has
>> set tags->rqs[tag_of_reserved_request] first. That pointer is set by
>> blk_mq_get_driver_tag(). blk_mq_get_driver_tag() is called by the
>> block layer code that submits a request to a block driver. Hence, to
>> ensure that blk_mq_get_driver_tag() is called for reserved requests
>> the reserved requests would have to pass through scsi_queue_rq().
> 
> Ah, yes, we had the same discussion when trying to implement SCSI 
> reserved commands previously.
> 
>> For reserved requests sdev == NULL as explained in a previous email.
> 
> which mail exactly? I could not see any specific mention in this thread.
> 
>> There are many statements in the SCSI command submission and completion
>> path in which it is assumed that sdev != NULL. I don't think that the
>> SCSI maintainer (Martin) would agree with adding "if (sdev)" statements
>> in many places in the SCSI core.
> 
> Sure, so, IIRC, Hannes solved this by using a shost sdev in https:// 
> lore.kernel.org/linux-scsi/20211125151048.103910-2-hare@suse.de/
> 
>>
>> Letting UFS reserved requests being processed by another function than
>> scsi_queue_rq() doesn't seem feasible to me either.
> 
> JFYI, I was working on another solution (different to Hannes') which 
> allows reserved requests pass through scsi_queue_rq(), but I stopped 
> that work when I changed employer.
> 
Oh, really?
Do you have the patchset still around?
I'd be very much interested in that...

>> Although it is easy
>> to create an additional request queue for reserved requests, that
>> request queue shares its tag set with the SCSI host and hence also the
>> request submission function. From scsi_host.h:
>>
>> struct Scsi_Host {
>>      struct blk_mq_tag_set    tag_set;
>>      [ ... ]
>> };
>>
>>  From blk-mq.h:
>>
>> struct blk_mq_tag_set {
>>      const struct blk_mq_ops    *ops;
>>      [ ... ]
>> };
>>
>> Unless someone comes up with an elegant proposal, I will keep the
>> approach where ufshcd_tag_to_cmd() handles reserved tags and regular
>> tags differently.
>>
>> It should be possible to do this without referencing tags->static_rqs[]
>> directly from the UFS driver.
> 
> I'm not sure how that will look, but my preference is to fully implement 
> reserved tags support which can be used by all SCSI LLDs.
> 
Sure, that was my goal, too.
But then I got stuck as some drivers (most notably aacraid) use internal
commands to detect the hardware and allocate SCSI devices, so with my
approach we need a 'fake' SCSI device for the host to handle that.
If you have other ideas I'd be very interested in thems.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich