lpfc: Fix rport leak.

Correct locking and refcounting in tracking our rports

Signed-off-by: Dick Kennedy <dick.kennedy@avagotech.com>
Signed-off-by: James Smart <james.smart@avagotech.com>
Signed-off-by: James Bottomley <JBottomley@Odin.com>

diff --git a/drivers/scsi/lpfc/lpfc_disc.h b/drivers/scsi/lpfc/lpfc_disc.h
index 6977027979beb6be433adad4cf9211c7787ba214..361f5b3d9d936bcdb075caf9405231e70c97016b 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_disc.h
+++ b/drivers/scsi/lpfc/lpfc_disc.h
@@ -79,7 +79,6 @@ struct lpfc_nodelist {
        struct lpfc_name nlp_portname;
        struct lpfc_name nlp_nodename;
        uint32_t         nlp_flag;              /* entry flags */
-       uint32_t         nlp_add_flag;          /* additional flags */
        uint32_t         nlp_DID;               /* FC D_ID of entry */
        uint32_t         nlp_last_elscmd;       /* Last ELS cmd sent */
        uint16_t         nlp_type;
@@ -147,6 +146,7 @@ struct lpfc_node_rrq {
 #define NLP_LOGO_ACC       0x00100000  /* Process LOGO after ACC completes */
 #define NLP_TGT_NO_SCSIID  0x00200000  /* good PRLI but no binding for scsid */
 #define NLP_ISSUE_LOGO     0x00400000  /* waiting to issue a LOGO */
+#define NLP_IN_DEV_LOSS    0x00800000  /* devloss in progress */
 #define NLP_ACC_REGLOGIN   0x01000000  /* Issue Reg Login after successful
                                           ACC */
 #define NLP_NPR_ADISC      0x02000000  /* Issue ADISC when dq'ed from
@@ -158,8 +158,6 @@ struct lpfc_node_rrq {
 #define NLP_FIRSTBURST     0x40000000  /* Target supports FirstBurst */
 #define NLP_RPI_REGISTERED 0x80000000  /* nlp_rpi is valid */
 
-/* Defines for nlp_add_flag (uint32) */
-#define NLP_IN_DEV_LOSS  0x00000001    /* Dev Loss processing in progress */
 
 /* ndlp usage management macros */
 #define NLP_CHK_NODE_ACT(ndlp)         (((ndlp)->nlp_usg_map \

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 4d3d931b177a73d60f4d3129744e7fbf94c8a066..011c8d8dba0b4eeabd43c643e7fc58ac4e0f2f04 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -1624,8 +1624,9 @@ lpfc_plogi_confirm_nport(struct lpfc_hba *phba, uint32_t *prsp,
                if (rport) {
                        rdata = rport->dd_data;
                        if (rdata->pnode == ndlp) {
-                               lpfc_nlp_put(ndlp);
+                               /* break the link before dropping the ref */
                                ndlp->rport = NULL;
+                               lpfc_nlp_put(ndlp);
                                rdata->pnode = lpfc_nlp_get(new_ndlp);
                                new_ndlp->rport = rport;
                        }
@@ -3674,6 +3675,7 @@ lpfc_cmpl_els_logo_acc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
         * Remove the ndlp reference if it's a fabric node that has
         * sent us an unsolicted LOGO.
         */
+       /* FIXME: this one frees ndlp before breaking rport link */
        if (ndlp->nlp_type & NLP_FABRIC)
                lpfc_nlp_put(ndlp);
 
@@ -7351,8 +7353,13 @@ lpfc_els_unsol_buffer(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
         * Do not process any unsolicited ELS commands
         * if the ndlp is in DEV_LOSS
         */
-       if (ndlp->nlp_add_flag & NLP_IN_DEV_LOSS)
+       shost = lpfc_shost_from_vport(vport);
+       spin_lock_irq(shost->host_lock);
+       if (ndlp->nlp_flag & NLP_IN_DEV_LOSS) {
+               spin_unlock_irq(shost->host_lock);
                goto dropit;
+       }
+       spin_unlock_irq(shost->host_lock);
 
        elsiocb->context1 = lpfc_nlp_get(ndlp);
        elsiocb->vport = vport;
@@ -7396,7 +7403,6 @@ lpfc_els_unsol_buffer(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
                        rjt_exp = LSEXP_NOTHING_MORE;
                        break;
                }
-               shost = lpfc_shost_from_vport(vport);
                if (vport->port_state < LPFC_DISC_AUTH) {
                        if (!(phba->pport->fc_flag & FC_PT2PT) ||
                                (phba->pport->fc_flag & FC_PT2PT_PLOGI)) {

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 0dfa56604c9123dad6531d0bbb9b91e839ff1a90..88af258147d08c576c83c335c15918f1eacf3487 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -106,6 +106,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
        struct lpfc_rport_data *rdata;
        struct lpfc_nodelist * ndlp;
        struct lpfc_vport *vport;
+       struct Scsi_Host *shost;
        struct lpfc_hba   *phba;
        struct lpfc_work_evt *evtp;
        int  put_node;
@@ -146,49 +147,32 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
        if (ndlp->nlp_state == NLP_STE_MAPPED_NODE)
                return;
 
-       if (ndlp->nlp_type & NLP_FABRIC) {
-
-               /* If the WWPN of the rport and ndlp don't match, ignore it */
-               if (rport->port_name != wwn_to_u64(ndlp->nlp_portname.u.wwn)) {
-                       lpfc_printf_vlog(vport, KERN_ERR, LOG_NODE,
-                               "6789 rport name %lx != node port name %lx",
-                               (unsigned long)rport->port_name,
-                               (unsigned long)wwn_to_u64(
-                                               ndlp->nlp_portname.u.wwn));
-                       put_node = rdata->pnode != NULL;
-                       put_rport = ndlp->rport != NULL;
-                       rdata->pnode = NULL;
-                       ndlp->rport = NULL;
-                       if (put_node)
-                               lpfc_nlp_put(ndlp);
-                       if (put_rport)
-                               put_device(&rport->dev);
-                       return;
-               }
-
-               put_node = rdata->pnode != NULL;
-               put_rport = ndlp->rport != NULL;
-               rdata->pnode = NULL;
-               ndlp->rport = NULL;
-               if (put_node)
-                       lpfc_nlp_put(ndlp);
-               if (put_rport)
-                       put_device(&rport->dev);
-               return;
-       }
+       if (rport->port_name != wwn_to_u64(ndlp->nlp_portname.u.wwn))
+               lpfc_printf_vlog(vport, KERN_ERR, LOG_NODE,
+                               "6789 rport name %llx != node port name %llx",
+                               rport->port_name,
+                               wwn_to_u64(ndlp->nlp_portname.u.wwn));
 
        evtp = &ndlp->dev_loss_evt;
 
-       if (!list_empty(&evtp->evt_listp))
+       if (!list_empty(&evtp->evt_listp)) {
+               lpfc_printf_vlog(vport, KERN_ERR, LOG_NODE,
+                               "6790 rport name %llx dev_loss_evt pending",
+                               rport->port_name);
                return;
+       }
 
-       evtp->evt_arg1  = lpfc_nlp_get(ndlp);
-       ndlp->nlp_add_flag |= NLP_IN_DEV_LOSS;
+       shost = lpfc_shost_from_vport(vport);
+       spin_lock_irq(shost->host_lock);
+       ndlp->nlp_flag |= NLP_IN_DEV_LOSS;
+       spin_unlock_irq(shost->host_lock);
 
-       spin_lock_irq(&phba->hbalock);
        /* We need to hold the node by incrementing the reference
         * count until this queued work is done
         */
+       evtp->evt_arg1  = lpfc_nlp_get(ndlp);
+
+       spin_lock_irq(&phba->hbalock);
        if (evtp->evt_arg1) {
                evtp->evt = LPFC_EVT_DEV_LOSS;
                list_add_tail(&evtp->evt_listp, &phba->work_list);
@@ -216,22 +200,24 @@ lpfc_dev_loss_tmo_handler(struct lpfc_nodelist *ndlp)
        struct fc_rport   *rport;
        struct lpfc_vport *vport;
        struct lpfc_hba   *phba;
+       struct Scsi_Host  *shost;
        uint8_t *name;
        int  put_node;
-       int  put_rport;
        int warn_on = 0;
        int fcf_inuse = 0;
 
        rport = ndlp->rport;
+       vport = ndlp->vport;
+       shost = lpfc_shost_from_vport(vport);
 
-       if (!rport) {
-               ndlp->nlp_add_flag &= ~NLP_IN_DEV_LOSS;
+       spin_lock_irq(shost->host_lock);
+       ndlp->nlp_flag &= ~NLP_IN_DEV_LOSS;
+       spin_unlock_irq(shost->host_lock);
+
+       if (!rport)
                return fcf_inuse;
-       }
 
-       rdata = rport->dd_data;
        name = (uint8_t *) &ndlp->nlp_portname;
-       vport = ndlp->vport;
        phba  = vport->phba;
 
        if (phba->sli_rev == LPFC_SLI_REV4)
@@ -245,6 +231,13 @@ lpfc_dev_loss_tmo_handler(struct lpfc_nodelist *ndlp)
                         "3182 dev_loss_tmo_handler x%06x, rport %p flg x%x\n",
                         ndlp->nlp_DID, ndlp->rport, ndlp->nlp_flag);
 
+       /*
+        * lpfc_nlp_remove if reached with dangling rport drops the
+        * reference. To make sure that does not happen clear rport
+        * pointer in ndlp before lpfc_nlp_put.
+        */
+       rdata = rport->dd_data;
+
        /* Don't defer this if we are in the process of deleting the vport
         * or unloading the driver. The unload will cleanup the node
         * appropriately we just need to cleanup the ndlp rport info here.
@@ -257,14 +250,12 @@ lpfc_dev_loss_tmo_handler(struct lpfc_nodelist *ndlp)
                                        ndlp->nlp_sid, 0, LPFC_CTX_TGT);
                }
                put_node = rdata->pnode != NULL;
-               put_rport = ndlp->rport != NULL;
                rdata->pnode = NULL;
                ndlp->rport = NULL;
-               ndlp->nlp_add_flag &= ~NLP_IN_DEV_LOSS;
                if (put_node)
                        lpfc_nlp_put(ndlp);
-               if (put_rport)
-                       put_device(&rport->dev);
+               put_device(&rport->dev);
+
                return fcf_inuse;
        }
 
@@ -276,28 +267,21 @@ lpfc_dev_loss_tmo_handler(struct lpfc_nodelist *ndlp)
                                 *name, *(name+1), *(name+2), *(name+3),
                                 *(name+4), *(name+5), *(name+6), *(name+7),
                                 ndlp->nlp_DID);
-               ndlp->nlp_add_flag &= ~NLP_IN_DEV_LOSS;
                return fcf_inuse;
        }
 
-       if (ndlp->nlp_type & NLP_FABRIC) {
-               /* We will clean up these Nodes in linkup */
-               put_node = rdata->pnode != NULL;
-               put_rport = ndlp->rport != NULL;
-               rdata->pnode = NULL;
-               ndlp->rport = NULL;
-               ndlp->nlp_add_flag &= ~NLP_IN_DEV_LOSS;
-               if (put_node)
-                       lpfc_nlp_put(ndlp);
-               if (put_rport)
-                       put_device(&rport->dev);
+       put_node = rdata->pnode != NULL;
+       rdata->pnode = NULL;
+       ndlp->rport = NULL;
+       if (put_node)
+               lpfc_nlp_put(ndlp);
+       put_device(&rport->dev);
+
+       if (ndlp->nlp_type & NLP_FABRIC)
                return fcf_inuse;
-       }
 
        if (ndlp->nlp_sid != NLP_NO_SID) {
                warn_on = 1;
-               /* flush the target */
-               ndlp->nlp_add_flag &= ~NLP_IN_DEV_LOSS;
                lpfc_sli_abort_iocb(vport, &phba->sli.ring[phba->sli.fcp_ring],
                                    ndlp->nlp_sid, 0, LPFC_CTX_TGT);
        }
@@ -322,16 +306,6 @@ lpfc_dev_loss_tmo_handler(struct lpfc_nodelist *ndlp)
                                 ndlp->nlp_state, ndlp->nlp_rpi);
        }
 
-       put_node = rdata->pnode != NULL;
-       put_rport = ndlp->rport != NULL;
-       rdata->pnode = NULL;
-       ndlp->rport = NULL;
-       ndlp->nlp_add_flag &= ~NLP_IN_DEV_LOSS;
-       if (put_node)
-               lpfc_nlp_put(ndlp);
-       if (put_rport)
-               put_device(&rport->dev);
-
        if (!(vport->load_flag & FC_UNLOADING) &&
            !(ndlp->nlp_flag & NLP_DELAY_TMO) &&
            !(ndlp->nlp_flag & NLP_NPR_2B_DISC) &&
@@ -3919,9 +3893,17 @@ lpfc_register_remote_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
         * registered port, drop the reference that we took the last time we
         * registered the port.
         */
-       if (ndlp->rport && ndlp->rport->dd_data &&
-           ((struct lpfc_rport_data *) ndlp->rport->dd_data)->pnode == ndlp)
-               lpfc_nlp_put(ndlp);
+       rport = ndlp->rport;
+       if (rport) {
+               rdata = rport->dd_data;
+               /* break the link before dropping the ref */
+               ndlp->rport = NULL;
+               if (rdata && rdata->pnode == ndlp)
+                       lpfc_nlp_put(ndlp);
+               rdata->pnode = NULL;
+               /* drop reference for earlier registeration */
+               put_device(&rport->dev);
+       }
 
        lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_RPORT,
                "rport add:       did:x%x flg:x%x type x%x",
@@ -4762,6 +4744,7 @@ lpfc_nlp_remove(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
 {
        struct lpfc_hba  *phba = vport->phba;
        struct lpfc_rport_data *rdata;
+       struct fc_rport *rport;
        LPFC_MBOXQ_t *mbox;
        int rc;
 
@@ -4799,14 +4782,24 @@ lpfc_nlp_remove(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
        lpfc_cleanup_node(vport, ndlp);
 
        /*
-        * We can get here with a non-NULL ndlp->rport because when we
-        * unregister a rport we don't break the rport/node linkage.  So if we
-        * do, make sure we don't leaving any dangling pointers behind.
+        * ndlp->rport must be set to NULL before it reaches here
+        * i.e. break rport/node link before doing lpfc_nlp_put for
+        * registered rport and then drop the reference of rport.
         */
        if (ndlp->rport) {
-               rdata = ndlp->rport->dd_data;
+               /*
+                * extra lpfc_nlp_put dropped the reference of ndlp
+                * for registered rport so need to cleanup rport
+                */
+               lpfc_printf_vlog(vport, KERN_WARNING, LOG_NODE,
+                               "0940 removed node x%p DID x%x "
+                               " rport not null %p\n",
+                               ndlp, ndlp->nlp_DID, ndlp->rport);
+               rport = ndlp->rport;
+               rdata = rport->dd_data;
                rdata->pnode = NULL;
                ndlp->rport = NULL;
+               put_device(&rport->dev);
        }
 }