scsi: lpfc: Fill in missing ndlp kref puts in error paths
authorJames Smart <jsmart2021@gmail.com>
Fri, 6 May 2022 03:55:09 +0000 (20:55 -0700)
committerMartin K. Petersen <martin.petersen@oracle.com>
Wed, 11 May 2022 02:12:02 +0000 (22:12 -0400)
Code review, following every lpfc_nlp_get() call vs calls during error
handling, discovered cases of missing put calls.

Correct by adding ndlp kref puts in the respective error paths.

Also added comments to several of the error paths to record relationships
to reference counts.

Link: https://lore.kernel.org/r/20220506035519.50908-3-jsmart2021@gmail.com
Co-developed-by: Justin Tee <justin.tee@broadcom.com>
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
drivers/scsi/lpfc/lpfc_els.c
drivers/scsi/lpfc/lpfc_nportdisc.c
drivers/scsi/lpfc/lpfc_nvme.c
drivers/scsi/lpfc/lpfc_sli.c

index 527f2c148e0412417c695a59545c1beb905c870f..ace812ce857d839d4afd86d04bcc0f3f4c7ce0ba 100644 (file)
@@ -8725,19 +8725,18 @@ lpfc_issue_els_rrq(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
        elsiocb->cmd_cmpl = lpfc_cmpl_els_rrq;
 
        elsiocb->ndlp = lpfc_nlp_get(ndlp);
-       if (!elsiocb->ndlp) {
-               lpfc_els_free_iocb(phba, elsiocb);
-               return 1;
-       }
+       if (!elsiocb->ndlp)
+               goto io_err;
 
        ret = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
-       if (ret == IOCB_ERROR)
+       if (ret == IOCB_ERROR) {
+               lpfc_nlp_put(ndlp);
                goto io_err;
+       }
        return 0;
 
  io_err:
        lpfc_els_free_iocb(phba, elsiocb);
-       lpfc_nlp_put(ndlp);
        return 1;
 }
 
index 5e4822bf54f474a81febdfec459a7c9bd22ef26d..639f866351271e690735c715497ea706a26687c8 100644 (file)
@@ -513,6 +513,10 @@ lpfc_rcv_plogi(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
                        lpfc_config_link(phba, link_mbox);
                        link_mbox->mbox_cmpl = lpfc_sli_def_mbox_cmpl;
                        link_mbox->vport = vport;
+
+                       /* The default completion handling for CONFIG_LINK
+                        * does not require the ndlp so no reference is needed.
+                        */
                        link_mbox->ctx_ndlp = ndlp;
 
                        rc = lpfc_sli_issue_mbox(phba, link_mbox, MBX_NOWAIT);
@@ -633,6 +637,9 @@ lpfc_rcv_plogi(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
         */
        login_mbox->mbox_cmpl = lpfc_defer_plogi_acc;
        login_mbox->ctx_ndlp = lpfc_nlp_get(ndlp);
+       if (!login_mbox->ctx_ndlp)
+               goto out;
+
        login_mbox->context3 = save_iocb; /* For PLOGI ACC */
 
        spin_lock_irq(&ndlp->lock);
@@ -641,8 +648,10 @@ lpfc_rcv_plogi(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
 
        /* Start the ball rolling by issuing REG_LOGIN here */
        rc = lpfc_sli_issue_mbox(phba, login_mbox, MBX_NOWAIT);
-       if (rc == MBX_NOT_FINISHED)
+       if (rc == MBX_NOT_FINISHED) {
+               lpfc_nlp_put(ndlp);
                goto out;
+       }
        lpfc_nlp_set_state(vport, ndlp, NLP_STE_REG_LOGIN_ISSUE);
 
        return 1;
@@ -1099,8 +1108,10 @@ lpfc_release_rpi(struct lpfc_hba *phba, struct lpfc_vport *vport,
                                 ndlp->nlp_rpi, ndlp->nlp_DID, ndlp->nlp_flag);
 
                rc = lpfc_sli_issue_mbox(phba, pmb, MBX_NOWAIT);
-               if (rc == MBX_NOT_FINISHED)
+               if (rc == MBX_NOT_FINISHED) {
+                       lpfc_nlp_put(ndlp);
                        mempool_free(pmb, phba->mbox_mem_pool);
+               }
        }
 }
 
index 376f6c0265c0c14378c756be5fe485265e80cd5e..3aebd01e07fdbe76acc963bade0e3028009f6e64 100644 (file)
@@ -2357,6 +2357,11 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
                rpinfo.dev_loss_tmo = vport->cfg_devloss_tmo;
 
        spin_lock_irq(&ndlp->lock);
+
+       /* If an oldrport exists, so does the ndlp reference.  If not
+        * a new reference is needed because either the node has never
+        * been registered or it's been unregistered and getting deleted.
+        */
        oldrport = lpfc_ndlp_get_nrport(ndlp);
        if (oldrport) {
                prev_ndlp = oldrport->ndlp;
index d2900ac8de9d58e82e5fe25c514f6469cda1d032..fe4eb36426df8cafce8af000304343af5d6cb1ac 100644 (file)
@@ -20684,8 +20684,12 @@ lpfc_cleanup_pending_mbox(struct lpfc_vport *vport)
                        mb->mbox_cmpl = lpfc_sli_def_mbox_cmpl;
                if (mb->u.mb.mbxCommand == MBX_REG_LOGIN64) {
                        act_mbx_ndlp = (struct lpfc_nodelist *)mb->ctx_ndlp;
-                       /* Put reference count for delayed processing */
+
+                       /* This reference is local to this routine.  The
+                        * reference is removed at routine exit.
+                        */
                        act_mbx_ndlp = lpfc_nlp_get(act_mbx_ndlp);
+
                        /* Unregister the RPI when mailbox complete */
                        mb->mbox_flag |= LPFC_MBX_IMED_UNREG;
                }