Re: [PATCH] scsi_lib: allow the ALUA transitioning state time to complete

[Brian Bunker <brian@xxxxxxxxxxxxxxx>]

On Thu, Jun 2, 2022 at 8:12 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> On Thu, Jun 02, 2022 at 08:10:28AM -0700, Brian Bunker wrote:
> >  static void scsi_io_completion_reprep(struct scsi_cmnd *cmd,
> > -                                     struct request_queue *q)
> > +                                     struct request_queue *q,
> > unsigned long msecs)
> >  {
> >         /* A new command will be prepared and issued. */
> > -       scsi_mq_requeue_cmd(cmd);
> > +       scsi_mq_requeue_cmd(cmd, msecs);
>
> q is unused.  But I think it is better if we just kill this pointless
> wrapper anyway.
>
> > +       case ACTION_DELAYED_REPREP:
> > +               scsi_io_completion_reprep(cmd, q, ALUA_TRANSITION_REPREP_DELAY);
>
> This is using spaces where it should use tabs.
>

Like this then:

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index e9db7da0c79c..e16a129fb064 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -118,7 +118,7 @@ scsi_set_blocked(struct scsi_cmnd *cmd, int
reason)
        }
 }

-static void scsi_mq_requeue_cmd(struct scsi_cmnd *cmd)
+static void scsi_mq_requeue_cmd(struct scsi_cmnd *cmd, unsigned long
msecs)
 {
        struct request *rq = scsi_cmd_to_rq(cmd);

@@ -128,7 +128,12 @@ static void scsi_mq_requeue_cmd(struct scsi_cmnd
*cmd)
        } else {
                WARN_ON_ONCE(true);
        }
-       blk_mq_requeue_request(rq, true);
+
+       if (msecs) {
+               blk_mq_requeue_request(rq, false);
+               blk_mq_delay_kick_requeue_list(rq->q, msecs);
+       } else
+               blk_mq_requeue_request(rq, true);
 }

 /**
@@ -658,14 +663,6 @@ static unsigned int scsi_rq_err_bytes(const
struct request                            *rq)
        return bytes;
 }

-/* Helper for scsi_io_completion() when "reprep" action required. */
-static void scsi_io_completion_reprep(struct scsi_cmnd *cmd,
-                                     struct request_queue *q)
-{
-       /* A new command will be prepared and issued. */
-       scsi_mq_requeue_cmd(cmd);
-}
-
 static bool scsi_cmd_runtime_exceeced(struct scsi_cmnd *cmd)
 {
        struct request *req = scsi_cmd_to_rq(cmd);
@@ -683,14 +680,22 @@ static bool scsi_cmd_runtime_exceeced(struct
scsi_cmnd *cm                           d)
        return false;
 }

+/*
+ * When ALUA transition state is returned, reprep the cmd to
+ * use the ALUA handlers transition timeout. Delay the reprep
+ * 1 sec to avoid aggressive retries of the target in that
+ * state.
+ */
+#define ALUA_TRANSITION_REPREP_DELAY   1000
+
 /* Helper for scsi_io_completion() when special action required. */
 static void scsi_io_completion_action(struct scsi_cmnd *cmd, int result)
 {
        struct request_queue *q = cmd->device->request_queue;
        struct request *req = scsi_cmd_to_rq(cmd);
        int level = 0;
-       enum {ACTION_FAIL, ACTION_REPREP, ACTION_RETRY,
-             ACTION_DELAYED_RETRY} action;
+       enum {ACTION_FAIL, ACTION_REPREP, ACTION_DELAYED_REPREP,
+             ACTION_RETRY, ACTION_DELAYED_RETRY} action;
        struct scsi_sense_hdr sshdr;
        bool sense_valid;
        bool sense_current = true;      /* false implies "deferred sense" */
@@ -779,8 +784,8 @@ static void scsi_io_completion_action(struct
scsi_cmnd *cmd, int result)
                                        action = ACTION_DELAYED_RETRY;
                                        break;
                                case 0x0a: /* ALUA state transition */
-                                       blk_stat = BLK_STS_AGAIN;
-                                       fallthrough;
+                                       action = ACTION_DELAYED_REPREP;
+                                       break;
                                default:
                                        action = ACTION_FAIL;
                                        break;
@@ -839,7 +844,10 @@ static void scsi_io_completion_action(struct
scsi_cmnd *cmd, int result)
                        return;
                fallthrough;
        case ACTION_REPREP:
-               scsi_io_completion_reprep(cmd, q);
+               scsi_mq_requeue_cmd(cmd, 0);
+               break;
+       case ACTION_DELAYED_REPREP:
+               scsi_mq_requeue_cmd(cmd, msecs);
                break;
        case ACTION_RETRY:
                /* Retry the same command immediately */
@@ -933,7 +941,7 @@ static int scsi_io_completion_nz_result(struct
scsi_cmnd *cmd, int result,
  * command block will be released and the queue function will be goosed. If we
  * are not done then we have to figure out what to do next:
  *
- *   a) We can call scsi_io_completion_reprep().  The request will be
+ *   a) We can call scsi_mq_requeue_cmd().  The request will be
  *     unprepared and put back on the queue.  Then a new command will
  *     be created for it.  This should be used if we made forward
  *     progress, or if we want to switch from READ(10) to READ(6) for
@@ -986,7 +994,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd,
unsigned int good_bytes)
         * request just queue the command up again.
         */
        if (likely(result == 0))
-               scsi_io_completion_reprep(cmd, q);
+               scsi_mq_requeue_cmd(cmd, 0);
        else
                scsi_io_completion_action(cmd, result);
 }


-- 

Brian Bunker
PURE Storage, Inc.
brian@xxxxxxxxxxxxxxx