On Thu, 2022-09-01 at 14:47 -0700, Brian Bunker wrote:
> The case of ACTION_RETRY in scsi_io_completion_action does nothing
> different than ACTION_DELAYED_RETRY, and by name gives the idea
> that it does.
>
> Following ACTION_RETRY:
> __scsi_queue_insert(cmd, SCSI_MLQUEUE_EH_RETRY, false);
>
> Following ACTION_DELAYED_RETRY:
> __scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY, false);
>
> Then __scsi_queue_insert calls scsi_set_blocked(cmd, reason) where
> both of the reasons set by ACTION_RETRY and ACTION_DELAYED_RETRY
> fall into the same case.
>
> case SCSI_MLQUEUE_DEVICE_BUSY:
> case SCSI_MLQUEUE_EH_RETRY:
> atomic_set(&device->device_blocked,
> device->max_device_blocked);
> break;
>
> Acked-by: Krishna Kant <krishna.kant@xxxxxxxxxxxxxxx>
> Acked-by: Seamus Connor <sconnor@xxxxxxxxxxxxxxx>
> Signed-off-by: Brian Bunker <brian@xxxxxxxxxxxxxxx>
I like this, but I would suggest to keep ACTION_RETRY and ditch
ACTION_DELAYED_RETRY instead. The retry isn't delayed in the usual
sense of the word (which would imply some sort of time interval). We
just wait for the device_blocked counter to go to zero.
Thanks,
Martin
> ---
> drivers/scsi/scsi_lib.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index ef08029a0079..d85d72bc01f2 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -687,7 +687,7 @@ static void scsi_io_completion_action(struct
> scsi_cmnd *cmd, int result)
> struct request *req = scsi_cmd_to_rq(cmd);
> int level = 0;
> enum {ACTION_FAIL, ACTION_REPREP, ACTION_DELAYED_REPREP,
> - ACTION_RETRY, ACTION_DELAYED_RETRY} action;
> + ACTION_DELAYED_RETRY} action;
> struct scsi_sense_hdr sshdr;
> bool sense_valid;
> bool sense_current = true; /* false implies "deferred
> sense" */
> @@ -704,7 +704,7 @@ static void scsi_io_completion_action(struct
> scsi_cmnd *cmd, int result)
> * reasons. Just retry the command and see what
> * happens.
> */
> - action = ACTION_RETRY;
> + action = ACTION_DELAYED_RETRY;
> } else if (sense_valid && sense_current) {
> switch (sshdr.sense_key) {
> case UNIT_ATTENTION:
> @@ -720,7 +720,7 @@ static void scsi_io_completion_action(struct
> scsi_cmnd *cmd, int result)
> * media change, so we just retry the
> * command and see what happens.
> */
> - action = ACTION_RETRY;
> + action = ACTION_DELAYED_RETRY;
> }
> break;
> case ILLEGAL_REQUEST:
> @@ -841,10 +841,6 @@ static void scsi_io_completion_action(struct
> scsi_cmnd *cmd, int result)
> case ACTION_DELAYED_REPREP:
> scsi_mq_requeue_cmd(cmd,
> ALUA_TRANSITION_REPREP_DELAY);
> break;
> - case ACTION_RETRY:
> - /* Retry the same command immediately */
> - __scsi_queue_insert(cmd, SCSI_MLQUEUE_EH_RETRY,
> false);
> - break;
> case ACTION_DELAYED_RETRY:
> /* Retry the same command after a delay */
> __scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY,
> false);
