On 10/18/22 3:29 PM, Bart Van Assche wrote:
> The current behavior for SCSI commands submitted while error recovery
> is ongoing is to retry command submission after error recovery has
> finished. See also the scsi_host_in_recovery() check in
> scsi_host_queue_ready(). Add support for failing SCSI commands while
> host recovery is in progress. This functionality will be used to fix a
> deadlock in the UFS driver.
>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Ming Lei <ming.lei@xxxxxxxxxx>
> Cc: John Garry <john.garry@xxxxxxxxxx>
> Cc: Mike Christie <michael.christie@xxxxxxxxxx>
> Cc: Hannes Reinecke <hare@xxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> drivers/scsi/scsi_lib.c | 8 +++++---
> include/scsi/scsi_cmnd.h | 3 ++-
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index fa96d3cfdfa3..ec890865abae 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1341,9 +1341,6 @@ static inline int scsi_host_queue_ready(struct request_queue *q,
> struct scsi_device *sdev,
> struct scsi_cmnd *cmd)
> {
> - if (scsi_host_in_recovery(shost))
> - return 0;
> -
> if (atomic_read(&shost->host_blocked) > 0) {
> if (scsi_host_busy(shost) > 0)
> goto starved;
> @@ -1732,6 +1729,11 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
> ret = BLK_STS_RESOURCE;
> if (!scsi_target_queue_ready(shost, sdev))
> goto out_put_budget;
> + if (unlikely(scsi_host_in_recovery(shost))) {
> + if (cmd->flags & SCMD_FAIL_IF_RECOVERING)
> + ret = BLK_STS_OFFLINE;
> + goto out_dec_target_busy;
> + }
Hey,
Will we always hit this check? For example, if we have hit the
device's queue depth limit so
scsi_mq_get_budget -> scsi_dev_queue_ready
is returning -1, will we not even call scsi_queue_rq? So because
we are in recovery and no commands are completing, we will be
stuck waiting for a token to be put back on the sdev->budget_map.
Do you need a similar check in scsi_dev_queue_ready or should
the check go in there only or can we hit a race for the latter?
