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: 6987 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 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: x86_64-randconfig-a013-20201215 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project a29ecca7819a6ed4250d3689b12b1f664bb790d7)
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
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # 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=clang make.cross 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/libfc/fc_exch.c:1629:20: warning: missing terminating '"' character [-Winvalid-pp-token]
                           FC_EXCH_DBG(ep, " ep is completed already,
                                           ^
   drivers/scsi/libfc/fc_exch.c:1630:35: warning: missing terminating '"' character [-Winvalid-pp-token]
                                           hence skip calling the resp\n");
                                                                        ^
   drivers/scsi/libfc/fc_exch.c:1911:19: warning: missing terminating '"' character [-Winvalid-pp-token]
                   FC_EXCH_DBG(ep, " ep is completed already,
                                   ^
   drivers/scsi/libfc/fc_exch.c:1912:34: warning: missing terminating '"' character [-Winvalid-pp-token]
                                   hence skip calling the resp\n");
                                                                ^
   drivers/scsi/libfc/fc_exch.c:1629:4: error: unterminated function-like macro invocation
                           FC_EXCH_DBG(ep, " ep is completed already,
                           ^
   drivers/scsi/libfc/fc_libfc.h:73:9: note: macro 'FC_EXCH_DBG' defined here
   #define FC_EXCH_DBG(exch, fmt, args...)                                 \
           ^
   drivers/scsi/libfc/fc_exch.c:2712:2: error: expected '}'
   }
    ^
   drivers/scsi/libfc/fc_exch.c:1628:10: note: to match this '{'
                   } else {
                          ^
   drivers/scsi/libfc/fc_exch.c:2712:2: error: expected '}'
   }
    ^
   drivers/scsi/libfc/fc_exch.c:1621:40: note: to match this '{'
               (FC_FC_LAST_SEQ | FC_FC_END_SEQ)) {
                                                 ^
   drivers/scsi/libfc/fc_exch.c:2712:2: error: expected '}'
   }
    ^
   drivers/scsi/libfc/fc_exch.c:1573:1: note: to match this '{'
   {
   ^
   drivers/scsi/libfc/fc_exch.c:1588:8: error: use of undeclared label 'rel'
                   goto rel;
                        ^
   drivers/scsi/libfc/fc_exch.c:1584:8: error: use of undeclared label 'out'
                   goto out;
                        ^
   4 warnings and 6 errors generated.

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: 40121 bytes --]