Commit 307e3380 authored by James Smart's avatar James Smart Committed by Martin K. Petersen
Browse files

scsi: lpfc: Rework remote port ref counting and node freeing 

When a remote port is disconnected and disappears, its node structure
(ndlp) stays allocated and on a vport node list. While on the list it can
be matched, thus requires validation checks on state to be added in
numerous code paths. If the node comes back, its possible for there to be
multiple node structures for the same device on the vport node list. There
is no reason to keep the node structure around after it is no longer in
existence, and the current implementation creates problems for itself
(multiple nodes) and lots of unnecessary code for state validation.

Additionally, the reference taking on the node structure didn't follow the
normal model used by the kernel kref api. It included lots of odd logic to
match state with reference count.  The combination of this odd logic plus
the way it was implicitly used in the discovery engine made its reference
taking implementation suspect and extremely hard to follow.

Change the driver such that the reference taking routines are now normal
ref increments/decrements and callout on refcount=0.

With this in place, the rework can be done such that the node structure is
fully removed and deallocated when the remote port no longer exists and all
references are removed.  This removal logic, and the basic ref counting are
intrically tied, thus in a single patch.

Link: https://lore.kernel.org/r/20201115192646.12977-2-james.smart@broadcom.com


Co-developed-by: default avatarDick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: default avatarDick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: default avatarJames Smart <james.smart@broadcom.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 4ab2990a

drivers/scsi/lpfc/lpfc_attr.c

+3 −8
Original line number Diff line number Diff line
@@ -3634,8 +3634,6 @@ lpfc_update_rport_devloss_tmo(struct lpfc_vport *vport) 
	shost = lpfc_shost_from_vport(vport);

	spin_lock_irq(shost->host_lock);

	list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {

		if (!NLP_CHK_NODE_ACT(ndlp))

			continue;

		if (ndlp->rport)

			ndlp->rport->dev_loss_tmo = vport->cfg_devloss_tmo;

#if (IS_ENABLED(CONFIG_NVME_FC))
@@ -4404,7 +4402,7 @@ sysfs_drvr_stat_data_read(struct file *filp, struct kobject *kobj, 


	spin_lock_irq(shost->host_lock);

	list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {

		if (!NLP_CHK_NODE_ACT(ndlp) || !ndlp->lat_data)

		if (!ndlp->lat_data)

			continue;



		if (nport_index > 0) {
@@ -5472,8 +5470,6 @@ lpfc_max_scsicmpl_time_set(struct lpfc_vport *vport, int val) 


	spin_lock_irq(shost->host_lock);

	list_for_each_entry_safe(ndlp, next_ndlp, &vport->fc_nodes, nlp_listp) {

		if (!NLP_CHK_NODE_ACT(ndlp))

			continue;

		if (ndlp->nlp_state == NLP_STE_UNUSED_NODE)

			continue;

		ndlp->cmd_qdepth = vport->cfg_tgt_queue_depth;
@@ -6982,8 +6978,7 @@ lpfc_get_node_by_target(struct scsi_target *starget) 
	spin_lock_irq(shost->host_lock);

	/* Search for this, mapped, target ID */

	list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {

		if (NLP_CHK_NODE_ACT(ndlp) &&

		    ndlp->nlp_state == NLP_STE_MAPPED_NODE &&

		if (ndlp->nlp_state == NLP_STE_MAPPED_NODE &&

		    starget->id == ndlp->nlp_sid) {

			spin_unlock_irq(shost->host_lock);

			return ndlp;
@@ -7058,7 +7053,7 @@ lpfc_set_rport_loss_tmo(struct fc_rport *rport, uint32_t timeout) 
	else

		rport->dev_loss_tmo = 1;



	if (!ndlp || !NLP_CHK_NODE_ACT(ndlp)) {

	if (!ndlp) {

		dev_info(&rport->dev, "Cannot find remote node to "

				      "set rport dev loss tmo, port_id x%x\n",

				      rport->port_id);

drivers/scsi/lpfc/lpfc_bsg.c

+1 −1
Original line number Diff line number Diff line
@@ -1520,7 +1520,7 @@ lpfc_issue_ct_rsp(struct lpfc_hba *phba, struct bsg_job *job, uint32_t tag, 
		}



		/* Check if the ndlp is active */

		if (!ndlp || !NLP_CHK_NODE_ACT(ndlp)) {

		if (!ndlp) {

			rc = IOCB_ERROR;

			goto issue_ct_rsp_exit;

		}

drivers/scsi/lpfc/lpfc_crtn.h

+0 −2
Original line number Diff line number Diff line
@@ -88,8 +88,6 @@ void lpfc_mbx_cmpl_reg_vfi(struct lpfc_hba *, LPFC_MBOXQ_t *); 
void lpfc_unregister_vfi_cmpl(struct lpfc_hba *, LPFC_MBOXQ_t *);

void lpfc_enqueue_node(struct lpfc_vport *, struct lpfc_nodelist *);

void lpfc_dequeue_node(struct lpfc_vport *, struct lpfc_nodelist *);

struct lpfc_nodelist *lpfc_enable_node(struct lpfc_vport *,

					struct lpfc_nodelist *, int);

void lpfc_nlp_set_state(struct lpfc_vport *, struct lpfc_nodelist *, int);

void lpfc_drop_node(struct lpfc_vport *, struct lpfc_nodelist *);

void lpfc_set_disctmo(struct lpfc_vport *);

drivers/scsi/lpfc/lpfc_ct.c

+7 −8
Original line number Diff line number Diff line
@@ -740,7 +740,7 @@ lpfc_prep_node_fc4type(struct lpfc_vport *vport, uint32_t Did, uint8_t fc4_type) 


		ndlp = lpfc_setup_disc_node(vport, Did);



		if (ndlp && NLP_CHK_NODE_ACT(ndlp)) {

		if (ndlp) {

			lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_CT,

				"Parse GID_FTrsp: did:x%x flg:x%x x%x",

				Did, ndlp->nlp_flag, vport->fc_flag);
@@ -791,7 +791,7 @@ lpfc_prep_node_fc4type(struct lpfc_vport *vport, uint32_t Did, uint8_t fc4_type) 
			 * Don't even bother to send GFF_ID.

			 */

			ndlp = lpfc_findnode_did(vport, Did);

			if (ndlp && NLP_CHK_NODE_ACT(ndlp) &&

			if (ndlp &&

			    (ndlp->nlp_type &

			    (NLP_FCP_TARGET | NLP_NVME_TARGET))) {

				if (fc4_type == FC_TYPE_FCP)
@@ -1437,7 +1437,7 @@ lpfc_cmpl_ct_cmd_gff_id(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb, 


	/* This is a target port, unregistered port, or the GFF_ID failed */

	ndlp = lpfc_setup_disc_node(vport, did);

	if (ndlp && NLP_CHK_NODE_ACT(ndlp)) {

	if (ndlp) {

		lpfc_printf_vlog(vport, KERN_INFO, LOG_DISCOVERY,

				 "0242 Process x%x GFF "

				 "NameServer Rsp Data: x%x x%x x%x\n",
@@ -1872,8 +1872,7 @@ lpfc_ns_cmd(struct lpfc_vport *vport, int cmdcode, 
	int rc = 0;



	ndlp = lpfc_findnode_did(vport, NameServer_DID);

	if (!ndlp || !NLP_CHK_NODE_ACT(ndlp)

	    || ndlp->nlp_state != NLP_STE_UNMAPPED_NODE) {

	if (!ndlp || ndlp->nlp_state != NLP_STE_UNMAPPED_NODE) {

		rc=1;

		goto ns_cmd_exit;

	}
@@ -2204,7 +2203,7 @@ lpfc_cmpl_ct_disc_fdmi(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb, 
	lpfc_ct_free_iocb(phba, cmdiocb);



	ndlp = lpfc_findnode_did(vport, FDMI_DID);

	if (!ndlp || !NLP_CHK_NODE_ACT(ndlp))

	if (!ndlp)

		return;



	/* Check for a CT LS_RJT response */
@@ -2343,7 +2342,7 @@ lpfc_fdmi_change_check(struct lpfc_vport *vport) 
		return;



	ndlp = lpfc_findnode_did(vport, FDMI_DID);

	if (!ndlp || !NLP_CHK_NODE_ACT(ndlp))

	if (!ndlp)

		return;



	/* Check if system hostname changed */
@@ -3389,7 +3388,7 @@ lpfc_fdmi_cmd(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp, 
	void (*cmpl)(struct lpfc_hba *, struct lpfc_iocbq *,

		     struct lpfc_iocbq *);



	if (!ndlp || !NLP_CHK_NODE_ACT(ndlp))

	if (!ndlp)

		return 0;



	cmpl = lpfc_cmpl_ct_disc_fdmi; /* called from discovery */

drivers/scsi/lpfc/lpfc_debugfs.c

+0 −2
Original line number Diff line number Diff line
@@ -895,8 +895,6 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size) 
		if (ndlp->nlp_type & NLP_NVME_INITIATOR)

			len += scnprintf(buf + len,

					size - len, "NVME_INITIATOR ");

		len += scnprintf(buf+len, size-len, "usgmap:%x ",

			ndlp->nlp_usg_map);

		len += scnprintf(buf+len, size-len, "refcnt:%x",

			kref_read(&ndlp->kref));

		if (iocnt) {