* [PATCH] scsi: qedf: Avoid invoking response handler twice if ep is already completed.
@ 2020-12-15 15:44 Javed Hasan
2020-12-15 19:15 ` kernel test robot
2020-12-15 21:15 ` kernel test robot
0 siblings, 2 replies; 5+ messages in thread
From: Javed Hasan @ 2020-12-15 15:44 UTC (permalink / raw)
To: martin.petersen; +Cc: linux-scsi, GR-QLogic-Storage-Upstream, jhasan
Issue :- race condition getting hit between the response handler
get called because of the exchange_mgr_reset() which clear out all
the active XID and the response we get via interrupt as the same time
Below are the sequence of events occurring in case of
"issue/race condition" :-
rport ba0200: Port timeout, state PLOGI
rport ba0200: Port entered PLOGI state from PLOGI state
xid 1052: Exchange timer armed : 20000 msecs xid timer armed here
rport ba0200: Received LOGO request while in state PLOGI
rport ba0200: Delete port
rport ba0200: work event 3
rport ba0200: lld callback ev 3
bnx2fc: rport_event_hdlr: event = 3, port_id = 0xba0200
bnx2fc: ba0200 - rport not created Yet!!
/* Here we reset any outstanding exchanges before
freeing rport using the exch_mgr_reset() */
xid 1052: Exchange timer canceled
/*Here we got two response for one xid*/
xid 1052: invoking resp(), esb 20000000 state 3
xid 1052: invoking resp(), esb 20000000 state 3
xid 1052: fc_rport_plogi_resp() : ep->resp_active 2
xid 1052: fc_rport_plogi_resp() : ep->resp_active 2
Signed-off-by: Javed Hasan <jhasan@marvell.com>
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 16eb3b60ed58..5a1491afcac6 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -1624,8 +1624,13 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
rc = fc_exch_done_locked(ep);
WARN_ON(fc_seq_exch(sp) != ep);
spin_unlock_bh(&ep->ex_lock);
- if (!rc)
+ if (!rc) {
fc_exch_delete(ep);
+ } else {
+ FC_EXCH_DBG(ep, " ep is completed already,
+ hence skip calling the resp\n");
+ goto skip_resp;
+ }
}
/*
@@ -1644,6 +1649,7 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
if (!fc_invoke_resp(ep, sp, fp))
fc_frame_free(fp);
+skip_resp:
fc_exch_release(ep);
return;
rel:
@@ -1900,10 +1906,16 @@ static void fc_exch_reset(struct fc_exch *ep)
fc_exch_hold(ep);
- if (!rc)
+ if (!rc) {
fc_exch_delete(ep);
+ } else {
+ FC_EXCH_DBG(ep, " ep is completed already,
+ hence skip calling the resp\n");
+ goto skip_resp;
+ }
fc_invoke_resp(ep, sp, ERR_PTR(-FC_EX_CLOSED));
+skip_resp:
fc_seq_set_resp(sp, NULL, ep->arg);
fc_exch_release(ep);
}
--
2.18.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: qedf: Avoid invoking response handler twice if ep is already completed.
2020-12-15 15:44 [PATCH] scsi: qedf: Avoid invoking response handler twice if ep is already completed Javed Hasan
@ 2020-12-15 19:15 ` kernel test robot
2020-12-15 21:15 ` kernel test robot
1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-12-15 19:15 UTC (permalink / raw)
To: Javed Hasan, martin.petersen
Cc: kbuild-all, linux-scsi, GR-QLogic-Storage-Upstream, jhasan
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: qedf: Avoid invoking response handler twice if ep is already completed.
@ 2020-12-15 19:15 ` kernel test robot
0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-12-15 19:15 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 9775 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(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 69143 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: qedf: Avoid invoking response handler twice if ep is already completed.
2020-12-15 15:44 [PATCH] scsi: qedf: Avoid invoking response handler twice if ep is already completed Javed Hasan
@ 2020-12-15 21:15 ` kernel test robot
2020-12-15 21:15 ` kernel test robot
1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-12-15 21:15 UTC (permalink / raw)
To: Javed Hasan, martin.petersen
Cc: kbuild-all, clang-built-linux, linux-scsi,
GR-QLogic-Storage-Upstream, jhasan
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: qedf: Avoid invoking response handler twice if ep is already completed.
@ 2020-12-15 21:15 ` kernel test robot
0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-12-15 21:15 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 7166 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(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 40121 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-15 21:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 15:44 [PATCH] scsi: qedf: Avoid invoking response handler twice if ep is already completed Javed Hasan
2020-12-15 19:15 ` kernel test robot
2020-12-15 19:15 ` kernel test robot
2020-12-15 21:15 ` kernel test robot
2020-12-15 21:15 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.