Add a method to queue reserved commands.
Signed-off-by: John Garry <[email protected]>
---
drivers/scsi/hosts.c | 6 ++++++
drivers/scsi/scsi_lib.c | 25 +++++++++++++++++++++++++
include/scsi/scsi_host.h | 1 +
3 files changed, 32 insertions(+)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index db89afc37bc9..78968553089f 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -230,6 +230,12 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
goto fail;
}
+ if (shost->nr_reserved_cmds && !sht->reserved_queuecommand) {
+ shost_printk(KERN_ERR, shost,
+ "nr_reserved_cmds set but no method to queue\n");
+ goto fail;
+ }
+
/* Use min_t(int, ...) in case shost->can_queue exceeds SHRT_MAX */
shost->cmd_per_lun = min_t(int, shost->cmd_per_lun,
shost->can_queue);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a8c4e7c037ae..08015c42c326 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1428,6 +1428,16 @@ 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)) {
+ struct scsi_device *sdev = cmd->device;
+
+ scsi_mq_uninit_cmd(cmd);
+ scsi_device_unbusy(sdev, cmd);
+ __blk_mq_end_request(rq, 0);
+
+ return;
+ }
+
INIT_LIST_HEAD(&cmd->eh_entry);
atomic_inc(&cmd->device->iodone_cnt);
@@ -1718,6 +1728,21 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
blk_status_t ret;
int reason;
+ if (blk_mq_is_reserved_rq(req)) {
+ if (!(req->rq_flags & RQF_DONTPREP)) {
+ ret = scsi_prepare_cmd(req);
+ if (ret != BLK_STS_OK)
+ goto out_dec_host_busy;
+
+ req->rq_flags |= RQF_DONTPREP;
+ } else {
+ clear_bit(SCMD_STATE_COMPLETE, &cmd->state);
+ }
+ blk_mq_start_request(req);
+
+ return shost->hostt->reserved_queuecommand(shost, cmd);
+ }
+
WARN_ON_ONCE(cmd->budget_token < 0);
/*
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 91678c77398e..a39f36aa0b0d 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -73,6 +73,7 @@ struct scsi_host_template {
* STATUS: REQUIRED
*/
int (* queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
+ int (*reserved_queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
/*
* The commit_rqs function is used to trigger a hardware
--
2.35.3
On 10/25/22 19:17, John Garry wrote:
> Add a method to queue reserved commands.
>
> Signed-off-by: John Garry <[email protected]>
> ---
> drivers/scsi/hosts.c | 6 ++++++
> drivers/scsi/scsi_lib.c | 25 +++++++++++++++++++++++++
> include/scsi/scsi_host.h | 1 +
> 3 files changed, 32 insertions(+)
>
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index db89afc37bc9..78968553089f 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -230,6 +230,12 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
> goto fail;
> }
>
> + if (shost->nr_reserved_cmds && !sht->reserved_queuecommand) {
> + shost_printk(KERN_ERR, shost,
> + "nr_reserved_cmds set but no method to queue\n");
> + goto fail;
> + }
> +
> /* Use min_t(int, ...) in case shost->can_queue exceeds SHRT_MAX */
> shost->cmd_per_lun = min_t(int, shost->cmd_per_lun,
> shost->can_queue);
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index a8c4e7c037ae..08015c42c326 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1428,6 +1428,16 @@ 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)) {
> + struct scsi_device *sdev = cmd->device;
This variable is not really needed. You can call:
scsi_device_unbusy(cmd->device, cmd);
No ?
> +
> + scsi_mq_uninit_cmd(cmd);
> + scsi_device_unbusy(sdev, cmd);
> + __blk_mq_end_request(rq, 0);
> +
> + return;
> + }
> +
> INIT_LIST_HEAD(&cmd->eh_entry);
>
> atomic_inc(&cmd->device->iodone_cnt);
> @@ -1718,6 +1728,21 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
> blk_status_t ret;
> int reason;
>
> + if (blk_mq_is_reserved_rq(req)) {
> + if (!(req->rq_flags & RQF_DONTPREP)) {
> + ret = scsi_prepare_cmd(req);
> + if (ret != BLK_STS_OK)
> + goto out_dec_host_busy;
> +
> + req->rq_flags |= RQF_DONTPREP;
> + } else {
> + clear_bit(SCMD_STATE_COMPLETE, &cmd->state);
> + }
> + blk_mq_start_request(req);
> +
> + return shost->hostt->reserved_queuecommand(shost, cmd);
> + }
> +
> WARN_ON_ONCE(cmd->budget_token < 0);
>
> /*
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 91678c77398e..a39f36aa0b0d 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -73,6 +73,7 @@ struct scsi_host_template {
> * STATUS: REQUIRED
> */
> int (* queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
> + int (*reserved_queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
Nit: This op name sound like something returning a bool... May be a
straight "queue_reserved_command" name would be clearer ?
>
> /*
> * The commit_rqs function is used to trigger a hardware
--
Damien Le Moal
Western Digital Research
On 27/10/2022 02:21, Damien Le Moal wrote:
>>
>> + if (blk_mq_is_reserved_rq(rq)) {
>> + struct scsi_device *sdev = cmd->device;
> This variable is not really needed. You can call:
>
> scsi_device_unbusy(cmd->device, cmd);
>
> No ?
ok, your suggestion is good
>
>> +
>> + scsi_mq_uninit_cmd(cmd);
>> + scsi_device_unbusy(sdev, cmd);
>> + __blk_mq_end_request(rq, 0);
>> +
>> + return;
>> + }
>> +
>> INIT_LIST_HEAD(&cmd->eh_entry);
>>
>> atomic_inc(&cmd->device->iodone_cnt);
>> @@ -1718,6 +1728,21 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
>> blk_status_t ret;
>> int reason;
>>
>> + if (blk_mq_is_reserved_rq(req)) {
>> + if (!(req->rq_flags & RQF_DONTPREP)) {
>> + ret = scsi_prepare_cmd(req);
>> + if (ret != BLK_STS_OK)
>> + goto out_dec_host_busy;
>> +
>> + req->rq_flags |= RQF_DONTPREP;
>> + } else {
>> + clear_bit(SCMD_STATE_COMPLETE, &cmd->state);
>> + }
>> + blk_mq_start_request(req);
>> +
>> + return shost->hostt->reserved_queuecommand(shost, cmd);
>> + }
>> +
>> WARN_ON_ONCE(cmd->budget_token < 0);
>>
>> /*
>> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
>> index 91678c77398e..a39f36aa0b0d 100644
>> --- a/include/scsi/scsi_host.h
>> +++ b/include/scsi/scsi_host.h
>> @@ -73,6 +73,7 @@ struct scsi_host_template {
>> * STATUS: REQUIRED
>> */
>> int (* queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
>> + int (*reserved_queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
> Nit: This op name sound like something returning a bool... May be a
> straight "queue_reserved_command" name would be clearer ?
or queuecommand_reserved ? I'm just trying to have the name a variant of
"queuecommand".
>
thanks,
John
On 10/27/22 18:13, John Garry wrote:
> On 27/10/2022 02:21, Damien Le Moal wrote:
>>> + if (blk_mq_is_reserved_rq(rq)) {
>>> + struct scsi_device *sdev = cmd->device;
>> This variable is not really needed. You can call:
>>
>> scsi_device_unbusy(cmd->device, cmd);
>>
>> No ?
>
> ok, your suggestion is good
>
>>
>>> +
>>> + scsi_mq_uninit_cmd(cmd);
>>> + scsi_device_unbusy(sdev, cmd);
>>> + __blk_mq_end_request(rq, 0);
>>> +
>>> + return;
>>> + }
>>> +
>>> INIT_LIST_HEAD(&cmd->eh_entry);
>>> atomic_inc(&cmd->device->iodone_cnt);
>>> @@ -1718,6 +1728,21 @@ static blk_status_t scsi_queue_rq(struct
>>> blk_mq_hw_ctx *hctx,
>>> blk_status_t ret;
>>> int reason;
>>> + if (blk_mq_is_reserved_rq(req)) {
>>> + if (!(req->rq_flags & RQF_DONTPREP)) {
>>> + ret = scsi_prepare_cmd(req);
>>> + if (ret != BLK_STS_OK)
>>> + goto out_dec_host_busy;
>>> +
>>> + req->rq_flags |= RQF_DONTPREP;
>>> + } else {
>>> + clear_bit(SCMD_STATE_COMPLETE, &cmd->state);
>>> + }
>>> + blk_mq_start_request(req);
>>> +
>>> + return shost->hostt->reserved_queuecommand(shost, cmd);
>>> + }
>>> +
>>> WARN_ON_ONCE(cmd->budget_token < 0);
>>> /*
>>> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
>>> index 91678c77398e..a39f36aa0b0d 100644
>>> --- a/include/scsi/scsi_host.h
>>> +++ b/include/scsi/scsi_host.h
>>> @@ -73,6 +73,7 @@ struct scsi_host_template {
>>> * STATUS: REQUIRED
>>> */
>>> int (* queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
>>> + int (*reserved_queuecommand)(struct Scsi_Host *, struct
>>> scsi_cmnd *);
>> Nit: This op name sound like something returning a bool... May be a
>> straight "queue_reserved_command" name would be clearer ?
>
> or queuecommand_reserved ? I'm just trying to have the name a variant of
> "queuecommand".
I figured that :)
queuereservedcommand ? (hard to read...)
queuecommand_reserved is OK I guess.
>
>>
>
> thanks,
> John
--
Damien Le Moal
Western Digital Research