Message ID | 20220901214749.18875-1-brian@purestorage.com |
---|---|
State | New |
Headers | show |
Series | scsi: core: Remove ACTION_RETRY since it is redundant | expand |
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@purestorage.com> > Acked-by: Seamus Connor <sconnor@purestorage.com> > Signed-off-by: Brian Bunker <brian@purestorage.com> 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);
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);