Re: [PATCH v3 04/26] scsi: core: Bypass the queue limit checks for reserved commands

[Bart Van Assche <bvanassche@acm.org>]

On 9/4/25 2:49 AM, John Garry wrote:
> On 27/08/2025 01:06, Bart Van Assche wrote:
>> [ bvanassche: modified patch title and patch description. 
> 
> it's an odd name now... I don't know which queue limits we are bypassing

I can change "queue" into "SCSI host" in the patch title.

>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
>> @@ -1539,6 +1539,14 @@ static void scsi_complete(struct request *rq)
>>       struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
>>       enum scsi_disposition disposition;
>> +    if (blk_mq_is_reserved_rq(rq)) {
>> +        /* Only pass-through requests are supported in this code 
>> path. */
>> +        WARN_ON_ONCE(!blk_rq_is_passthrough(scsi_cmd_to_rq(cmd)));
> 
> eh, do we really have passthough reserved command?

All reserved commands that end up in scsi_complete() should be
pass-through commands (REQ_OP_DRV_IN / REQ_OP_DRV_OUT), isn't it?
I don't think that we should allow other request types for reserved
commands.
>> -    if (unlikely(sdev->sdev_state != SDEV_RUNNING)) {
>> -        ret = scsi_device_state_check(sdev, req);
>> -        if (ret != BLK_STS_OK)
>> -            goto out_put_budget;
>> -    }
>> +    if (!blk_mq_is_reserved_rq(req)) {
>> +        /*
>> +         * If the device is not in running state we will reject some or
>> +         * all commands.
>> +         */
>> +        if (unlikely(sdev->sdev_state != SDEV_RUNNING)) {
> 
> I am curious about this. I mentioned previously if we only send reserved 
> commands to the psuedo sdev (in this seris). If so, would the psuedo 
> sdev not always be running state?

Has the above code change perhaps been misread? The above change causes
the sdev->sdev_state check to be skipped for pseudo SCSI devices.

Thanks,

Bart.