Re: [PATCH] scsi: qedf: Avoid invoking response handler twice if ep is already completed.

[kernel test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 9563 bytes --]

Hi Javed,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next target/for-next v5.10 next-20201215]
[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/Javed-Hasan/scsi-qedf-Avoid-invoking-response-handler-twice-if-ep-is-already-completed/20201216-001607
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/eafc014c649de737d637ee480fc1f5868dc5165a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Javed-Hasan/scsi-qedf-Avoid-invoking-response-handler-twice-if-ep-is-already-completed/20201216-001607
        git checkout eafc014c649de737d637ee480fc1f5868dc5165a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips 

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/libfc/fc_exch.c: In function 'fc_exch_recv_seq_resp':
>> drivers/scsi/libfc/fc_exch.c:1629:20: warning: missing terminating " character
    1629 |    FC_EXCH_DBG(ep, " ep is completed already,
         |                    ^
   drivers/scsi/libfc/fc_exch.c:1630:35: warning: missing terminating " character
    1630 |      hence skip calling the resp\n");
         |                                   ^
   drivers/scsi/libfc/fc_exch.c:1911:19: warning: missing terminating " character
    1911 |   FC_EXCH_DBG(ep, " ep is completed already,
         |                   ^
   drivers/scsi/libfc/fc_exch.c:1912:34: warning: missing terminating " character
    1912 |     hence skip calling the resp\n");
         |                                  ^
   drivers/scsi/libfc/fc_exch.c:2712: error: unterminated argument list invoking macro "FC_EXCH_DBG"
    2712 | }
         | 
   drivers/scsi/libfc/fc_exch.c:1629:4: error: 'FC_EXCH_DBG' undeclared (first use in this function)
    1629 |    FC_EXCH_DBG(ep, " ep is completed already,
         |    ^~~~~~~~~~~
   drivers/scsi/libfc/fc_exch.c:1629:4: note: each undeclared identifier is reported only once for each function it appears in
   drivers/scsi/libfc/fc_exch.c:1629:15: error: expected ';' at end of input
    1629 |    FC_EXCH_DBG(ep, " ep is completed already,
         |               ^
         |               ;
   ......
    2712 | }
         |                
   drivers/scsi/libfc/fc_exch.c:1629:4: error: expected declaration or statement at end of input
    1629 |    FC_EXCH_DBG(ep, " ep is completed already,
         |    ^~~~~~~~~~~
   drivers/scsi/libfc/fc_exch.c:1629:4: error: expected declaration or statement at end of input
   drivers/scsi/libfc/fc_exch.c:1629:4: error: expected declaration or statement at end of input
   drivers/scsi/libfc/fc_exch.c:1599:3: error: label 'rel' used but not defined
    1599 |   goto rel;
         |   ^~~~
   drivers/scsi/libfc/fc_exch.c:1584:3: error: label 'out' used but not defined
    1584 |   goto out;
         |   ^~~~
   drivers/scsi/libfc/fc_exch.c: At top level:
>> drivers/scsi/libfc/fc_exch.c:121:13: warning: 'fc_exch_rrq' used but never defined
     121 | static void fc_exch_rrq(struct fc_exch *);
         |             ^~~~~~~~~~~
>> drivers/scsi/libfc/fc_exch.c:122:13: warning: 'fc_seq_ls_acc' used but never defined
     122 | static void fc_seq_ls_acc(struct fc_frame *);
         |             ^~~~~~~~~~~~~
>> drivers/scsi/libfc/fc_exch.c:123:13: warning: 'fc_seq_ls_rjt' used but never defined
     123 | static void fc_seq_ls_rjt(struct fc_frame *, enum fc_els_rjt_reason,
         |             ^~~~~~~~~~~~~
>> drivers/scsi/libfc/fc_exch.c:125:13: warning: 'fc_exch_els_rec' used but never defined
     125 | static void fc_exch_els_rec(struct fc_frame *);
         |             ^~~~~~~~~~~~~~~
>> drivers/scsi/libfc/fc_exch.c:126:13: warning: 'fc_exch_els_rrq' used but never defined
     126 | static void fc_exch_els_rrq(struct fc_frame *);
         |             ^~~~~~~~~~~~~~~
   drivers/scsi/libfc/fc_exch.c:1572:13: warning: 'fc_exch_recv_seq_resp' defined but not used [-Wunused-function]
    1572 | static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
         |             ^~~~~~~~~~~~~~~~~~~~~
   drivers/scsi/libfc/fc_exch.c:1510:13: warning: 'fc_exch_recv_req' defined but not used [-Wunused-function]
    1510 | static void fc_exch_recv_req(struct fc_lport *lport, struct fc_exch_mgr *mp,
         |             ^~~~~~~~~~~~~~~~
   drivers/scsi/libfc/fc_exch.c:1412:13: warning: 'fc_exch_recv_abts' defined but not used [-Wunused-function]
    1412 | static void fc_exch_recv_abts(struct fc_exch *ep, struct fc_frame *rx_fp)
         |             ^~~~~~~~~~~~~~~~~
   drivers/scsi/libfc/fc_exch.c:1206:13: warning: 'fc_exch_set_addr' defined but not used [-Wunused-function]
    1206 | static void fc_exch_set_addr(struct fc_exch *ep,
         |             ^~~~~~~~~~~~~~~~
   drivers/scsi/libfc/fc_exch.c:1169:23: warning: 'fc_seq_lookup_orig' defined but not used [-Wunused-function]
    1169 | static struct fc_seq *fc_seq_lookup_orig(struct fc_exch_mgr *mp,
         |                       ^~~~~~~~~~~~~~~~~~
   drivers/scsi/libfc/fc_exch.c:366:13: warning: 'fc_exch_timer_set' defined but not used [-Wunused-function]
     366 | static void fc_exch_timer_set(struct fc_exch *ep, unsigned int timer_msec)
         |             ^~~~~~~~~~~~~~~~~
   drivers/scsi/libfc/fc_exch.c:237:20: warning: 'fc_exch_rctl_name' defined but not used [-Wunused-function]
     237 | static const char *fc_exch_rctl_name(unsigned int op)
         |                    ^~~~~~~~~~~~~~~~~
   drivers/scsi/libfc/fc_exch.c:29:27: warning: 'fc_em_cachep' defined but not used [-Wunused-variable]
      29 | static struct kmem_cache *fc_em_cachep;        /* cache for exchanges */
         |                           ^~~~~~~~~~~~

vim +1629 drivers/scsi/libfc/fc_exch.c

  1564	
  1565	/**
  1566	 * fc_exch_recv_seq_resp() - Handler for an incoming response where the other
  1567	 *			     end is the originator of the sequence that is a
  1568	 *			     response to our initial exchange
  1569	 * @mp: The EM that the exchange is on
  1570	 * @fp: The response frame
  1571	 */
  1572	static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
  1573	{
  1574		struct fc_frame_header *fh = fc_frame_header_get(fp);
  1575		struct fc_seq *sp;
  1576		struct fc_exch *ep;
  1577		enum fc_sof sof;
  1578		u32 f_ctl;
  1579		int rc;
  1580	
  1581		ep = fc_exch_find(mp, ntohs(fh->fh_ox_id));
  1582		if (!ep) {
  1583			atomic_inc(&mp->stats.xid_not_found);
  1584			goto out;
  1585		}
  1586		if (ep->esb_stat & ESB_ST_COMPLETE) {
  1587			atomic_inc(&mp->stats.xid_not_found);
  1588			goto rel;
  1589		}
  1590		if (ep->rxid == FC_XID_UNKNOWN)
  1591			ep->rxid = ntohs(fh->fh_rx_id);
  1592		if (ep->sid != 0 && ep->sid != ntoh24(fh->fh_d_id)) {
  1593			atomic_inc(&mp->stats.xid_not_found);
  1594			goto rel;
  1595		}
  1596		if (ep->did != ntoh24(fh->fh_s_id) &&
  1597		    ep->did != FC_FID_FLOGI) {
  1598			atomic_inc(&mp->stats.xid_not_found);
  1599			goto rel;
  1600		}
  1601		sof = fr_sof(fp);
  1602		sp = &ep->seq;
  1603		if (fc_sof_is_init(sof)) {
  1604			sp->ssb_stat |= SSB_ST_RESP;
  1605			sp->id = fh->fh_seq_id;
  1606		}
  1607	
  1608		f_ctl = ntoh24(fh->fh_f_ctl);
  1609		fr_seq(fp) = sp;
  1610	
  1611		spin_lock_bh(&ep->ex_lock);
  1612		if (f_ctl & FC_FC_SEQ_INIT)
  1613			ep->esb_stat |= ESB_ST_SEQ_INIT;
  1614		spin_unlock_bh(&ep->ex_lock);
  1615	
  1616		if (fc_sof_needs_ack(sof))
  1617			fc_seq_send_ack(sp, fp);
  1618	
  1619		if (fh->fh_type != FC_TYPE_FCP && fr_eof(fp) == FC_EOF_T &&
  1620		    (f_ctl & (FC_FC_LAST_SEQ | FC_FC_END_SEQ)) ==
  1621		    (FC_FC_LAST_SEQ | FC_FC_END_SEQ)) {
  1622			spin_lock_bh(&ep->ex_lock);
  1623			rc = fc_exch_done_locked(ep);
  1624			WARN_ON(fc_seq_exch(sp) != ep);
  1625			spin_unlock_bh(&ep->ex_lock);
  1626			if (!rc) {
  1627				fc_exch_delete(ep);
  1628			} else {
> 1629				FC_EXCH_DBG(ep, " ep is completed already,
  1630						hence skip calling the resp\n");
  1631				goto skip_resp;
  1632			}
  1633		}
  1634	
  1635		/*
  1636		 * Call the receive function.
  1637		 * The sequence is held (has a refcnt) for us,
  1638		 * but not for the receive function.
  1639		 *
  1640		 * The receive function may allocate a new sequence
  1641		 * over the old one, so we shouldn't change the
  1642		 * sequence after this.
  1643		 *
  1644		 * The frame will be freed by the receive function.
  1645		 * If new exch resp handler is valid then call that
  1646		 * first.
  1647		 */
  1648		if (!fc_invoke_resp(ep, sp, fp))
  1649			fc_frame_free(fp);
  1650	
  1651	skip_resp:
  1652		fc_exch_release(ep);
  1653		return;
  1654	rel:
  1655		fc_exch_release(ep);
  1656	out:
  1657		fc_frame_free(fp);
  1658	}
  1659	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69143 bytes --]