diff mbox series

[03/16] lpfc: Fix reference counting errors in lpfc_cmpl_els_rsp()

Message ID 20210410173034.67618-4-jsmart2021@gmail.com (mailing list archive)
State Superseded
Headers show
Series lpfc: Update lpfc to revision 12.8.0.9 | expand

Commit Message

James Smart April 10, 2021, 5:30 p.m. UTC 
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

Co-developed-by: Justin Tee <justin.tee@broadcom.com>
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc_els.c | 50 +-----------------------------------
 1 file changed, 1 insertion(+), 49 deletions(-)

Comments

kernel test robot April 10, 2021, 8:21 p.m. UTC | #1 
Hi James,

I love your patch! Perhaps something to improve:

[auto build test WARNING on scsi/for-next]
[also build test WARNING on mkp-scsi/for-next linus/master v5.12-rc6 next-20210409]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/James-Smart/lpfc-Update-lpfc-to-revision-12-8-0-9/20210411-013122
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/14cc4aacd0b5890c9db3ce319c67b956520a2aa7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review James-Smart/lpfc-Update-lpfc-to-revision-12-8-0-9/20210411-013122
        git checkout 14cc4aacd0b5890c9db3ce319c67b956520a2aa7
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/scsi/lpfc/lpfc_els.c: In function 'lpfc_cmpl_els_rsp':
>> drivers/scsi/lpfc/lpfc_els.c:4460:11: warning: variable 'ls_rjt' set but not used [-Wunused-but-set-variable]
    4460 |  uint32_t ls_rjt = 0;
         |           ^~~~~~


vim +/ls_rjt +4460 drivers/scsi/lpfc/lpfc_els.c

858c9f6c19c6f9 James Smart     2007-06-17  4435  
e59058c44025d7 James Smart     2008-08-24  4436  /**
3621a710a7dbb2 James Smart     2009-04-06  4437   * lpfc_cmpl_els_rsp - Completion callback function for els response iocb cmd
e59058c44025d7 James Smart     2008-08-24  4438   * @phba: pointer to lpfc hba data structure.
e59058c44025d7 James Smart     2008-08-24  4439   * @cmdiocb: pointer to lpfc command iocb data structure.
e59058c44025d7 James Smart     2008-08-24  4440   * @rspiocb: pointer to lpfc response iocb data structure.
e59058c44025d7 James Smart     2008-08-24  4441   *
e59058c44025d7 James Smart     2008-08-24  4442   * This routine is the completion callback function for ELS Response IOCB
e59058c44025d7 James Smart     2008-08-24  4443   * command. In normal case, this callback function just properly sets the
e59058c44025d7 James Smart     2008-08-24  4444   * nlp_flag bitmap in the ndlp data structure, if the mbox command reference
e59058c44025d7 James Smart     2008-08-24  4445   * field in the command IOCB is not NULL, the referred mailbox command will
e59058c44025d7 James Smart     2008-08-24  4446   * be send out, and then invokes the lpfc_els_free_iocb() routine to release
14cc4aacd0b589 James Smart     2021-04-10  4447   * the IOCB.
e59058c44025d7 James Smart     2008-08-24  4448   **/
dea3101e0a5c89 James Bottomley 2005-04-17  4449  static void
858c9f6c19c6f9 James Smart     2007-06-17  4450  lpfc_cmpl_els_rsp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
dea3101e0a5c89 James Bottomley 2005-04-17  4451  		  struct lpfc_iocbq *rspiocb)
dea3101e0a5c89 James Bottomley 2005-04-17  4452  {
2e0fef85e098f6 James Smart     2007-06-17  4453  	struct lpfc_nodelist *ndlp = (struct lpfc_nodelist *) cmdiocb->context1;
2e0fef85e098f6 James Smart     2007-06-17  4454  	struct lpfc_vport *vport = ndlp ? ndlp->vport : NULL;
2e0fef85e098f6 James Smart     2007-06-17  4455  	struct Scsi_Host  *shost = vport ? lpfc_shost_from_vport(vport) : NULL;
33ccf8d1080bdc James Smart     2006-08-17  4456  	IOCB_t  *irsp;
87af33fe5f78c2 James Smart     2007-10-27  4457  	uint8_t *pcmd;
dea3101e0a5c89 James Bottomley 2005-04-17  4458  	LPFC_MBOXQ_t *mbox = NULL;
2e0fef85e098f6 James Smart     2007-06-17  4459  	struct lpfc_dmabuf *mp = NULL;
87af33fe5f78c2 James Smart     2007-10-27 @4460  	uint32_t ls_rjt = 0;
dea3101e0a5c89 James Bottomley 2005-04-17  4461  
33ccf8d1080bdc James Smart     2006-08-17  4462  	irsp = &rspiocb->iocb;
33ccf8d1080bdc James Smart     2006-08-17  4463  
43bfea1bffb6b0 James Smart     2019-09-21  4464  	if (!vport) {
372c187b8a705c Dick Kennedy    2020-06-30  4465  		lpfc_printf_log(phba, KERN_ERR, LOG_TRACE_EVENT,
43bfea1bffb6b0 James Smart     2019-09-21  4466  				"3177 ELS response failed\n");
43bfea1bffb6b0 James Smart     2019-09-21  4467  		goto out;
43bfea1bffb6b0 James Smart     2019-09-21  4468  	}
dea3101e0a5c89 James Bottomley 2005-04-17  4469  	if (cmdiocb->context_un.mbox)
dea3101e0a5c89 James Bottomley 2005-04-17  4470  		mbox = cmdiocb->context_un.mbox;
dea3101e0a5c89 James Bottomley 2005-04-17  4471  
fa4066b672821d James Smart     2008-01-11  4472  	/* First determine if this is a LS_RJT cmpl. Note, this callback
fa4066b672821d James Smart     2008-01-11  4473  	 * function can have cmdiocb->contest1 (ndlp) field set to NULL.
fa4066b672821d James Smart     2008-01-11  4474  	 */
87af33fe5f78c2 James Smart     2007-10-27  4475  	pcmd = (uint8_t *) (((struct lpfc_dmabuf *) cmdiocb->context2)->virt);
307e338097dc32 James Smart     2020-11-15  4476  	if (ndlp && (*((uint32_t *) (pcmd)) == ELS_CMD_LS_RJT)) {
fa4066b672821d James Smart     2008-01-11  4477  		/* A LS_RJT associated with Default RPI cleanup has its own
3ad2f3fbb96142 Daniel Mack     2010-02-03  4478  		 * separate code path.
87af33fe5f78c2 James Smart     2007-10-27  4479  		 */
87af33fe5f78c2 James Smart     2007-10-27  4480  		if (!(ndlp->nlp_flag & NLP_RM_DFLT_RPI))
87af33fe5f78c2 James Smart     2007-10-27  4481  			ls_rjt = 1;
87af33fe5f78c2 James Smart     2007-10-27  4482  	}
87af33fe5f78c2 James Smart     2007-10-27  4483  
dea3101e0a5c89 James Bottomley 2005-04-17  4484  	/* Check to see if link went down during discovery */
307e338097dc32 James Smart     2020-11-15  4485  	if (!ndlp || lpfc_els_chk_latt(vport)) {
dea3101e0a5c89 James Bottomley 2005-04-17  4486  		if (mbox) {
3e1f0718921cd1 James Smart     2018-11-29  4487  			mp = (struct lpfc_dmabuf *)mbox->ctx_buf;
146911500f2572 James Smart     2006-12-02  4488  			if (mp) {
146911500f2572 James Smart     2006-12-02  4489  				lpfc_mbuf_free(phba, mp->virt, mp->phys);
146911500f2572 James Smart     2006-12-02  4490  				kfree(mp);
146911500f2572 James Smart     2006-12-02  4491  			}
dea3101e0a5c89 James Bottomley 2005-04-17  4492  			mempool_free(mbox, phba->mbox_mem_pool);
dea3101e0a5c89 James Bottomley 2005-04-17  4493  		}
dea3101e0a5c89 James Bottomley 2005-04-17  4494  		goto out;
dea3101e0a5c89 James Bottomley 2005-04-17  4495  	}
dea3101e0a5c89 James Bottomley 2005-04-17  4496  
858c9f6c19c6f9 James Smart     2007-06-17  4497  	lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_ELS_RSP,
51ef4c26891a73 James Smart     2007-08-02  4498  		"ELS rsp cmpl:    status:x%x/x%x did:x%x",
858c9f6c19c6f9 James Smart     2007-06-17  4499  		irsp->ulpStatus, irsp->un.ulpWord[4],
51ef4c26891a73 James Smart     2007-08-02  4500  		cmdiocb->iocb.un.elsreq64.remoteID);
dea3101e0a5c89 James Bottomley 2005-04-17  4501  	/* ELS response tag <ulpIoTag> completes */
e8b62011d88d6f James Smart     2007-08-02  4502  	lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
e8b62011d88d6f James Smart     2007-08-02  4503  			 "0110 ELS response tag x%x completes "
c9f8735beadfba Jamie Wellnitz  2006-02-28  4504  			 "Data: x%x x%x x%x x%x x%x x%x x%x\n",
dea3101e0a5c89 James Bottomley 2005-04-17  4505  			 cmdiocb->iocb.ulpIoTag, rspiocb->iocb.ulpStatus,
c9f8735beadfba Jamie Wellnitz  2006-02-28  4506  			 rspiocb->iocb.un.ulpWord[4], rspiocb->iocb.ulpTimeout,
c9f8735beadfba Jamie Wellnitz  2006-02-28  4507  			 ndlp->nlp_DID, ndlp->nlp_flag, ndlp->nlp_state,
c9f8735beadfba Jamie Wellnitz  2006-02-28  4508  			 ndlp->nlp_rpi);
dea3101e0a5c89 James Bottomley 2005-04-17  4509  	if (mbox) {
4430f7fd09ecb0 James Smart     2020-11-15  4510  		if ((rspiocb->iocb.ulpStatus == 0) &&
4430f7fd09ecb0 James Smart     2020-11-15  4511  		    (ndlp->nlp_flag & NLP_ACC_REGLOGIN)) {
1fe68477d235e4 Dick Kennedy    2017-08-23  4512  			if (!lpfc_unreg_rpi(vport, ndlp) &&
4430f7fd09ecb0 James Smart     2020-11-15  4513  			    (!(vport->fc_flag & FC_PT2PT))) {
4430f7fd09ecb0 James Smart     2020-11-15  4514  				if (ndlp->nlp_state == NLP_STE_REG_LOGIN_ISSUE) {
1fe68477d235e4 Dick Kennedy    2017-08-23  4515  					lpfc_printf_vlog(vport, KERN_INFO,
1fe68477d235e4 Dick Kennedy    2017-08-23  4516  							 LOG_DISCOVERY,
4430f7fd09ecb0 James Smart     2020-11-15  4517  							 "0314 PLOGI recov "
4430f7fd09ecb0 James Smart     2020-11-15  4518  							 "DID x%x "
1fe68477d235e4 Dick Kennedy    2017-08-23  4519  							 "Data: x%x x%x x%x\n",
4430f7fd09ecb0 James Smart     2020-11-15  4520  							 ndlp->nlp_DID,
4430f7fd09ecb0 James Smart     2020-11-15  4521  							 ndlp->nlp_state,
4430f7fd09ecb0 James Smart     2020-11-15  4522  							 ndlp->nlp_rpi,
4430f7fd09ecb0 James Smart     2020-11-15  4523  							 ndlp->nlp_flag);
3e1f0718921cd1 James Smart     2018-11-29  4524  					mp = mbox->ctx_buf;
1fe68477d235e4 Dick Kennedy    2017-08-23  4525  					if (mp) {
1fe68477d235e4 Dick Kennedy    2017-08-23  4526  						lpfc_mbuf_free(phba, mp->virt,
1fe68477d235e4 Dick Kennedy    2017-08-23  4527  							       mp->phys);
1fe68477d235e4 Dick Kennedy    2017-08-23  4528  						kfree(mp);
1fe68477d235e4 Dick Kennedy    2017-08-23  4529  					}
1fe68477d235e4 Dick Kennedy    2017-08-23  4530  					mempool_free(mbox, phba->mbox_mem_pool);
1fe68477d235e4 Dick Kennedy    2017-08-23  4531  					goto out;
1fe68477d235e4 Dick Kennedy    2017-08-23  4532  				}
4430f7fd09ecb0 James Smart     2020-11-15  4533  			}
1fe68477d235e4 Dick Kennedy    2017-08-23  4534  
e47c9093531d34 James Smart     2008-02-08  4535  			/* Increment reference count to ndlp to hold the
e47c9093531d34 James Smart     2008-02-08  4536  			 * reference to ndlp for the callback function.
e47c9093531d34 James Smart     2008-02-08  4537  			 */
3e1f0718921cd1 James Smart     2018-11-29  4538  			mbox->ctx_ndlp = lpfc_nlp_get(ndlp);
4430f7fd09ecb0 James Smart     2020-11-15  4539  			if (!mbox->ctx_ndlp)
4430f7fd09ecb0 James Smart     2020-11-15  4540  				goto out;
4430f7fd09ecb0 James Smart     2020-11-15  4541  
2e0fef85e098f6 James Smart     2007-06-17  4542  			mbox->vport = vport;
858c9f6c19c6f9 James Smart     2007-06-17  4543  			if (ndlp->nlp_flag & NLP_RM_DFLT_RPI) {
858c9f6c19c6f9 James Smart     2007-06-17  4544  				mbox->mbox_flag |= LPFC_MBX_IMED_UNREG;
858c9f6c19c6f9 James Smart     2007-06-17  4545  				mbox->mbox_cmpl = lpfc_mbx_cmpl_dflt_rpi;
858c9f6c19c6f9 James Smart     2007-06-17  4546  			}
858c9f6c19c6f9 James Smart     2007-06-17  4547  			else {
858c9f6c19c6f9 James Smart     2007-06-17  4548  				mbox->mbox_cmpl = lpfc_mbx_cmpl_reg_login;
5024ab179c13d7 Jamie Wellnitz  2006-02-28  4549  				ndlp->nlp_prev_state = ndlp->nlp_state;
2e0fef85e098f6 James Smart     2007-06-17  4550  				lpfc_nlp_set_state(vport, ndlp,
2e0fef85e098f6 James Smart     2007-06-17  4551  					   NLP_STE_REG_LOGIN_ISSUE);
858c9f6c19c6f9 James Smart     2007-06-17  4552  			}
4b7789b71c916f James Smart     2015-12-16  4553  
4b7789b71c916f James Smart     2015-12-16  4554  			ndlp->nlp_flag |= NLP_REG_LOGIN_SEND;
0b727fea7a700e James Smart     2007-10-27  4555  			if (lpfc_sli_issue_mbox(phba, mbox, MBX_NOWAIT)
e47c9093531d34 James Smart     2008-02-08  4556  			    != MBX_NOT_FINISHED)
dea3101e0a5c89 James Bottomley 2005-04-17  4557  				goto out;
4b7789b71c916f James Smart     2015-12-16  4558  
e47c9093531d34 James Smart     2008-02-08  4559  			/* Decrement the ndlp reference count we
e47c9093531d34 James Smart     2008-02-08  4560  			 * set for this failed mailbox command.
e47c9093531d34 James Smart     2008-02-08  4561  			 */
e47c9093531d34 James Smart     2008-02-08  4562  			lpfc_nlp_put(ndlp);
4b7789b71c916f James Smart     2015-12-16  4563  			ndlp->nlp_flag &= ~NLP_REG_LOGIN_SEND;
98c9ea5c026ee4 James Smart     2007-10-27  4564  
98c9ea5c026ee4 James Smart     2007-10-27  4565  			/* ELS rsp: Cannot issue reg_login for <NPortid> */
372c187b8a705c Dick Kennedy    2020-06-30  4566  			lpfc_printf_vlog(vport, KERN_ERR, LOG_TRACE_EVENT,
98c9ea5c026ee4 James Smart     2007-10-27  4567  				"0138 ELS rsp: Cannot issue reg_login for x%x "
98c9ea5c026ee4 James Smart     2007-10-27  4568  				"Data: x%x x%x x%x\n",
98c9ea5c026ee4 James Smart     2007-10-27  4569  				ndlp->nlp_DID, ndlp->nlp_flag, ndlp->nlp_state,
98c9ea5c026ee4 James Smart     2007-10-27  4570  				ndlp->nlp_rpi);
dea3101e0a5c89 James Bottomley 2005-04-17  4571  		}
3e1f0718921cd1 James Smart     2018-11-29  4572  		mp = (struct lpfc_dmabuf *)mbox->ctx_buf;
146911500f2572 James Smart     2006-12-02  4573  		if (mp) {
146911500f2572 James Smart     2006-12-02  4574  			lpfc_mbuf_free(phba, mp->virt, mp->phys);
146911500f2572 James Smart     2006-12-02  4575  			kfree(mp);
146911500f2572 James Smart     2006-12-02  4576  		}
146911500f2572 James Smart     2006-12-02  4577  		mempool_free(mbox, phba->mbox_mem_pool);
33ccf8d1080bdc James Smart     2006-08-17  4578  	}
dea3101e0a5c89 James Bottomley 2005-04-17  4579  out:
307e338097dc32 James Smart     2020-11-15  4580  	if (ndlp && shost) {
c6adba15019176 James Smart     2020-11-15  4581  		spin_lock_irq(&ndlp->lock);
7b08e89f98cee9 James Smart     2020-08-28  4582  		if (mbox)
7b08e89f98cee9 James Smart     2020-08-28  4583  			ndlp->nlp_flag &= ~NLP_ACC_REGLOGIN;
7b08e89f98cee9 James Smart     2020-08-28  4584  		ndlp->nlp_flag &= ~NLP_RM_DFLT_RPI;
c6adba15019176 James Smart     2020-11-15  4585  		spin_unlock_irq(&ndlp->lock);
dea3101e0a5c89 James Bottomley 2005-04-17  4586  	}
87af33fe5f78c2 James Smart     2007-10-27  4587  
4430f7fd09ecb0 James Smart     2020-11-15  4588  	/* Release the originating I/O reference. */
dea3101e0a5c89 James Bottomley 2005-04-17  4589  	lpfc_els_free_iocb(phba, cmdiocb);
4430f7fd09ecb0 James Smart     2020-11-15  4590  	lpfc_nlp_put(ndlp);
dea3101e0a5c89 James Bottomley 2005-04-17  4591  	return;
dea3101e0a5c89 James Bottomley 2005-04-17  4592  }
dea3101e0a5c89 James Bottomley 2005-04-17  4593  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index a04546eca18f..67159b957344 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -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,
@@ -4494,15 +4491,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 +4568,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 +4583,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. */