Skip to content

Commit

Permalink
scsi: lpfc: Fix reference counting errors in lpfc_cmpl_els_rsp()
Browse files Browse the repository at this point in the history
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/[email protected]
Co-developed-by: Justin Tee <[email protected]>
Signed-off-by: Justin Tee <[email protected]>
Signed-off-by: James Smart <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
  • Loading branch information
jsmart-gh authored and martinkpetersen committed Apr 13, 2021
1 parent fffd18e commit f866eb0
Showing 1 changed file with 1 addition and 63 deletions.
64 changes: 1 addition & 63 deletions drivers/scsi/lpfc/lpfc_els.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;

Expand All @@ -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) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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. */
Expand Down

0 comments on commit f866eb0

Please sign in to comment.