James Smart wrote:
>
> Fix rport leak.
>
> Correct locking and refcounting in tracking our rports
>
> Signed-off-by: Dick Kennedy <dick.kennedy@xxxxxxxxxxxxx>
> Signed-off-by: James Smart <james.smart@xxxxxxxxxxxxx>
> ---
> drivers/scsi/lpfc/lpfc_disc.h | 4 +-
> drivers/scsi/lpfc/lpfc_els.c | 12 +++-
> drivers/scsi/lpfc/lpfc_hbadisc.c | 145 +++++++++++++++++++--------------------
> 3 files changed, 79 insertions(+), 82 deletions(-)
snip
> diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
> index 0dfa566..88af258 100644
> --- 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);
Additional space here
> +
> + spin_lock_irq(&phba->hbalock);
> if (evtp->evt_arg1) {
> evtp->evt = LPFC_EVT_DEV_LOSS;
> list_add_tail(&evtp->evt_listp, &phba->work_list);
snip
> @@ -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);
checkpatch suggests to not split this user-visible string.
> + rport = ndlp->rport;
> + rdata = rport->dd_data;
> rdata->pnode = NULL;
> ndlp->rport = NULL;
> + put_device(&rport->dev);
> }
> }
>
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
