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

scsi: lpfc: Fix reference counting errors in lpfc_cmpl_els_rsp() 

Call traces are being seen that result from a nodelist structure ref
counting error. They are typically seen after transmission of an LS_RJT ELS
response.

Aged code in lpfc_cmpl_els_rsp() calls lpfc_nlp_not_used() which, if the
ndlp reference count is exactly 1, will decrement the reference count.
Previously lpfc_nlp_put() was within lpfc_els_free_iocb(), and the 'put'
within the free would only be invoked if cmdiocb->context1 was not NULL.
Since the nodelist structure reference count is decremented when exiting
lpfc_cmpl_els_rsp() the lpfc_nlp_not_used() calls are no longer required.
Calling them is causing the reference count issue.

Fix by removing the lpfc_nlp_not_used() calls.

Link: https://lore.kernel.org/r/20210412013127.2387-4-jsmart2021@gmail.com


Co-developed-by: default avatarJustin Tee <justin.tee@broadcom.com>
Signed-off-by: default avatarJustin Tee <justin.tee@broadcom.com>
Signed-off-by: default avatarJames Smart <jsmart2021@gmail.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent fffd18ec

drivers/scsi/lpfc/lpfc_els.c

+1 −63
Original line number Diff line number Diff line
@@ -4444,10 +4444,7 @@ lpfc_mbx_cmpl_dflt_rpi(struct lpfc_hba *phba, LPFC_MBOXQ_t *pmb) 
 * nlp_flag bitmap in the ndlp data structure, if the mbox command reference

 * field in the command IOCB is not NULL, the referred mailbox command will

 * be send out, and then invokes the lpfc_els_free_iocb() routine to release

 * the IOCB. Under error conditions, such as when a LS_RJT is returned or a

 * link down event occurred during the discovery, the lpfc_nlp_not_used()

 * routine shall be invoked trying to release the ndlp if no other threads

 * are currently referring it.

 * the IOCB.

 **/

static void

lpfc_cmpl_els_rsp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
@@ -4457,10 +4454,8 @@ lpfc_cmpl_els_rsp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb, 
	struct lpfc_vport *vport = ndlp ? ndlp->vport : NULL;

	struct Scsi_Host  *shost = vport ? lpfc_shost_from_vport(vport) : NULL;

	IOCB_t  *irsp;

	uint8_t *pcmd;

	LPFC_MBOXQ_t *mbox = NULL;

	struct lpfc_dmabuf *mp = NULL;

	uint32_t ls_rjt = 0;



	irsp = &rspiocb->iocb;
@@ -4472,18 +4467,6 @@ lpfc_cmpl_els_rsp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb, 
	if (cmdiocb->context_un.mbox)

		mbox = cmdiocb->context_un.mbox;



	/* First determine if this is a LS_RJT cmpl. Note, this callback

	 * function can have cmdiocb->contest1 (ndlp) field set to NULL.

	 */

	pcmd = (uint8_t *) (((struct lpfc_dmabuf *) cmdiocb->context2)->virt);

	if (ndlp && (*((uint32_t *) (pcmd)) == ELS_CMD_LS_RJT)) {

		/* A LS_RJT associated with Default RPI cleanup has its own

		 * separate code path.

		 */

		if (!(ndlp->nlp_flag & NLP_RM_DFLT_RPI))

			ls_rjt = 1;

	}



	/* Check to see if link went down during discovery */

	if (!ndlp || lpfc_els_chk_latt(vport)) {

		if (mbox) {
@@ -4494,15 +4477,6 @@ lpfc_cmpl_els_rsp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb, 
			}

			mempool_free(mbox, phba->mbox_mem_pool);

		}

		if (ndlp && (ndlp->nlp_flag & NLP_RM_DFLT_RPI))

			if (lpfc_nlp_not_used(ndlp)) {

				ndlp = NULL;

				/* Indicate the node has already released,

				 * should not reference to it from within

				 * the routine lpfc_els_free_iocb.

				 */

				cmdiocb->context1 = NULL;

			}

		goto out;

	}
@@ -4580,29 +4554,6 @@ lpfc_cmpl_els_rsp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb, 
				"Data: x%x x%x x%x\n",

				ndlp->nlp_DID, ndlp->nlp_flag, ndlp->nlp_state,

				ndlp->nlp_rpi);



			if (lpfc_nlp_not_used(ndlp)) {

				ndlp = NULL;

				/* Indicate node has already been released,

				 * should not reference to it from within

				 * the routine lpfc_els_free_iocb.

				 */

				cmdiocb->context1 = NULL;

			}

		} else {

			/* Do not drop node for lpfc_els_abort'ed ELS cmds */

			if (!lpfc_error_lost_link(irsp) &&

			    ndlp->nlp_flag & NLP_ACC_REGLOGIN) {

				if (lpfc_nlp_not_used(ndlp)) {

					ndlp = NULL;

					/* Indicate node has already been

					 * released, should not reference

					 * to it from within the routine

					 * lpfc_els_free_iocb.

					 */

					cmdiocb->context1 = NULL;

				}

			}

		}

		mp = (struct lpfc_dmabuf *)mbox->ctx_buf;

		if (mp) {
@@ -4618,19 +4569,6 @@ lpfc_cmpl_els_rsp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb, 
			ndlp->nlp_flag &= ~NLP_ACC_REGLOGIN;

		ndlp->nlp_flag &= ~NLP_RM_DFLT_RPI;

		spin_unlock_irq(&ndlp->lock);



		/* If the node is not being used by another discovery thread,

		 * and we are sending a reject, we are done with it.

		 * Release driver reference count here and free associated

		 * resources.

		 */

		if (ls_rjt)

			if (lpfc_nlp_not_used(ndlp))

				/* Indicate node has already been released,

				 * should not reference to it from within

				 * the routine lpfc_els_free_iocb.

				 */

				cmdiocb->context1 = NULL;

	}



	/* Release the originating I/O reference. */