scsi: lpfc: Correct driver deregistrations with host nvme transport

The driver's interaction with the host nvme transport has been incorrect
for a while. The driver did not wait for the unregister callbacks
(waited only 5 jiffies). Thus the driver may remove objects that may be
referenced by subsequent abort commands from the transport, and the
actual unregister callback was effectively a noop. This was especially
problematic if the driver was unloaded.

The driver now waits for the unregister callbacks, as it should, before
continuing with teardown.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <james.smart@broadcom.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/lpfc/lpfc_disc.h b/drivers/scsi/lpfc/lpfc_disc.h
index f9a566eaef0442258a2bd27a6093da76909ecfa4..5a7547f9d8d8fd63d2bb037228ee062f21756be6 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_disc.h
+++ b/drivers/scsi/lpfc/lpfc_disc.h
@@ -134,6 +134,8 @@ struct lpfc_nodelist {
        struct lpfc_scsicmd_bkt *lat_data;      /* Latency data */
        uint32_t fc4_prli_sent;
        uint32_t upcall_flags;
+#define NLP_WAIT_FOR_UNREG    0x1
+
        uint32_t nvme_fb_size; /* NVME target's supported byte cnt */
 #define NVME_FB_BIT_SHIFT 9    /* PRLI Rsp first burst in 512B units. */
 };

diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 60592af30014cbd95a3b455a56e786ec4fdcd4fd..874612b108292866b085ec8712d521f419f61a6f 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -154,6 +154,10 @@ lpfc_nvme_localport_delete(struct nvme_fc_local_port *localport)
 {
        struct lpfc_nvme_lport *lport = localport->private;
 
+       lpfc_printf_vlog(lport->vport, KERN_INFO, LOG_NVME,
+                        "6173 localport %p delete complete\n",
+                        lport);
+
        /* release any threads waiting for the unreg to complete */
        complete(&lport->lport_unreg_done);
 }
@@ -946,10 +950,19 @@ out_err:
        freqpriv->nvme_buf = NULL;
 
        /* NVME targets need completion held off until the abort exchange
-        * completes.
+        * completes unless the NVME Rport is getting unregistered.
         */
-       if (!(lpfc_ncmd->flags & LPFC_SBUF_XBUSY))
+       if (!(lpfc_ncmd->flags & LPFC_SBUF_XBUSY) ||
+           ndlp->upcall_flags & NLP_WAIT_FOR_UNREG) {
+               /* Clear the XBUSY flag to prevent double completions.
+                * The nvme rport is getting unregistered and there is
+                * no need to defer the IO.
+                */
+               if (lpfc_ncmd->flags & LPFC_SBUF_XBUSY)
+                       lpfc_ncmd->flags &= ~LPFC_SBUF_XBUSY;
+
                nCmd->done(nCmd);
+       }
 
        spin_lock_irqsave(&phba->hbalock, flags);
        lpfc_ncmd->nrport = NULL;
@@ -2234,6 +2247,47 @@ lpfc_nvme_create_localport(struct lpfc_vport *vport)
        return ret;
 }
 
+/* lpfc_nvme_lport_unreg_wait - Wait for the host to complete an lport unreg.
+ *
+ * The driver has to wait for the host nvme transport to callback
+ * indicating the localport has successfully unregistered all
+ * resources.  Since this is an uninterruptible wait, loop every ten
+ * seconds and print a message indicating no progress.
+ *
+ * An uninterruptible wait is used because of the risk of transport-to-
+ * driver state mismatch.
+ */
+void
+lpfc_nvme_lport_unreg_wait(struct lpfc_vport *vport,
+                          struct lpfc_nvme_lport *lport)
+{
+#if (IS_ENABLED(CONFIG_NVME_FC))
+       u32 wait_tmo;
+       int ret;
+
+       /* Host transport has to clean up and confirm requiring an indefinite
+        * wait. Print a message if a 10 second wait expires and renew the
+        * wait. This is unexpected.
+        */
+       wait_tmo = msecs_to_jiffies(LPFC_NVME_WAIT_TMO * 1000);
+       while (true) {
+               ret = wait_for_completion_timeout(&lport->lport_unreg_done,
+                                                 wait_tmo);
+               if (unlikely(!ret)) {
+                       lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_IOERR,
+                                        "6176 Lport %p Localport %p wait "
+                                        "timed out. Renewing.\n",
+                                        lport, vport->localport);
+                       continue;
+               }
+               break;
+       }
+       lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_IOERR,
+                        "6177 Lport %p Localport %p Complete Success\n",
+                        lport, vport->localport);
+#endif
+}
+
 /**
  * lpfc_nvme_destroy_localport - Destroy lpfc_nvme bound to nvme transport.
  * @pnvme: pointer to lpfc nvme data structure.
@@ -2268,7 +2322,11 @@ lpfc_nvme_destroy_localport(struct lpfc_vport *vport)
         */
        init_completion(&lport->lport_unreg_done);
        ret = nvme_fc_unregister_localport(localport);
-       wait_for_completion_timeout(&lport->lport_unreg_done, 5);
+
+       /* Wait for completion.  This either blocks
+        * indefinitely or succeeds
+        */
+       lpfc_nvme_lport_unreg_wait(vport, lport);
 
        /* Regardless of the unregister upcall response, clear
         * nvmei_support.  All rports are unregistered and the
@@ -2424,6 +2482,47 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 #endif
 }
 
+/* lpfc_nvme_rport_unreg_wait - Wait for the host to complete an rport unreg.
+ *
+ * The driver has to wait for the host nvme transport to callback
+ * indicating the remoteport has successfully unregistered all
+ * resources.  Since this is an uninterruptible wait, loop every ten
+ * seconds and print a message indicating no progress.
+ *
+ * An uninterruptible wait is used because of the risk of transport-to-
+ * driver state mismatch.
+ */
+void
+lpfc_nvme_rport_unreg_wait(struct lpfc_vport *vport,
+                          struct lpfc_nvme_rport *rport)
+{
+#if (IS_ENABLED(CONFIG_NVME_FC))
+       u32 wait_tmo;
+       int ret;
+
+       /* Host transport has to clean up and confirm requiring an indefinite
+        * wait. Print a message if a 10 second wait expires and renew the
+        * wait. This is unexpected.
+        */
+       wait_tmo = msecs_to_jiffies(LPFC_NVME_WAIT_TMO * 1000);
+       while (true) {
+               ret = wait_for_completion_timeout(&rport->rport_unreg_done,
+                                                 wait_tmo);
+               if (unlikely(!ret)) {
+                       lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_IOERR,
+                                        "6174 Rport %p Remoteport %p wait "
+                                        "timed out. Renewing.\n",
+                                        rport, rport->remoteport);
+                       continue;
+               }
+               break;
+       }
+       lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_IOERR,
+                        "6175 Rport %p Remoteport %p Complete Success\n",
+                        rport, rport->remoteport);
+#endif
+}
+
 /* lpfc_nvme_unregister_port - unbind the DID and port_role from this rport.
  *
  * There is no notion of Devloss or rport recovery from the current
@@ -2480,14 +2579,19 @@ lpfc_nvme_unregister_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
                /* No concern about the role change on the nvme remoteport.
                 * The transport will update it.
                 */
+               ndlp->upcall_flags |= NLP_WAIT_FOR_UNREG;
                ret = nvme_fc_unregister_remoteport(remoteport);
-               if (ret != 0) {
+               if (ret != 0)
                        lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_DISC,
                                         "6167 NVME unregister failed %d "
                                         "port_state x%x\n",
                                         ret, remoteport->port_state);
-               }
-
+               else
+                       /* Wait for completion.  This either blocks
+                        * indefinitely or succeeds
+                        */
+                       lpfc_nvme_rport_unreg_wait(vport, rport);
+               ndlp->upcall_flags &= ~NLP_WAIT_FOR_UNREG;
        }
        return;
 

diff --git a/drivers/scsi/lpfc/lpfc_nvme.h b/drivers/scsi/lpfc/lpfc_nvme.h
index fbfc1786cd04893f61987de55400b31c6402b331..903ec37f465fa0c9f00e41f6bec9eb2a8751ba3e 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_nvme.h
+++ b/drivers/scsi/lpfc/lpfc_nvme.h
@@ -27,6 +27,8 @@
 
 #define LPFC_NVME_ERSP_LEN             0x20
 
+#define LPFC_NVME_WAIT_TMO              10
+
 struct lpfc_nvme_qhandle {
        uint32_t index;         /* WQ index to use */
        uint32_t qidx;          /* queue index passed to create */