| Message ID | 20200207115209.25933-1-krishna2@chelsio.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-review/for-rc/for-rc] RDMA/siw: Remove unwanted WARN_ON in siw_cm_llp_data_ready() | expand |
On Friday, February 02/07/20, 2020 at 17:22:09 +0530, Krishnamraju Eraparaju wrote: > Warnings like below can fill up the dmesg while disconnecting RDMA > connections. > Hence, removing the unwanted WARN_ON. > > [72103.557612] WARNING: CPU: 6 PID: 0 at > drivers/infiniband/sw/siw/siw_cm.c:1229 siw_cm_llp_data_ready+0xc1/0xd0 > [siw] > [72103.557677] RIP: 0010:siw_cm_llp_data_ready+0xc1/0xd0 [siw] > [72103.557693] Call Trace: > [72103.557711] tcp_data_queue+0x226/0xb40 > [72103.557714] tcp_rcv_established+0x220/0x620 > [72103.557720] tcp_v4_do_rcv+0x12a/0x1e0 > [72103.557722] tcp_v4_rcv+0xb05/0xc00 > [72103.557728] ip_local_deliver_finish+0x69/0x210 > [72103.557730] ip_local_deliver+0x6b/0xe0 > [72103.557735] ip_rcv+0x273/0x362 > [72103.557740] __netif_receive_skb_core+0xb35/0xc30 > [72103.557752] netif_receive_skb_internal+0x3d/0xb0 > [72103.557754] napi_gro_frags+0x13b/0x200 > [72103.557788] t4_ethrx_handler+0x433/0x7d0 [cxgb4] > [72103.557800] process_responses+0x318/0x580 [cxgb4] > [72103.557820] napi_rx_handler+0x14/0x100 [cxgb4] > [72103.557822] net_rx_action+0x149/0x3b0 > [72103.557826] __do_softirq+0xe3/0x30a > [72103.557831] irq_exit+0x100/0x110 > [72103.557834] do_IRQ+0x7f/0xe0 > [72103.557837] common_interrupt+0xf/0xf > > Signed-off-by: Krishnamraju Eraparaju <krishna2@chelsio.com> > --- > drivers/infiniband/sw/siw/siw_cm.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c > index 3bccfef40e7e..bcbc54998ace 100644 > --- a/drivers/infiniband/sw/siw/siw_cm.c > +++ b/drivers/infiniband/sw/siw/siw_cm.c > @@ -1226,7 +1226,6 @@ static void siw_cm_llp_data_ready(struct sock *sk) > > cep = sk_to_cep(sk); > if (!cep) { > - WARN_ON(1); > goto out; > } > siw_dbg_cep(cep, "state: %d\n", cep->state); > -- > 2.23.0.rc0 > Please ignore this patch as I mistakenly put "for-review/for-rc/for-rc" in the subject line instead of "for-rc".
On Fri, Feb 07, 2020 at 05:22:09PM +0530, Krishnamraju Eraparaju wrote: > Warnings like below can fill up the dmesg while disconnecting RDMA > connections. > Hence, removing the unwanted WARN_ON. Please explain why it the code is correct to take this error path. Bernard clearly thought this shouldn't be happening Jason
-----"Jason Gunthorpe" <jgg@mellanox.com> wrote: ----- >To: "Krishnamraju Eraparaju" <krishna2@chelsio.com> >From: "Jason Gunthorpe" <jgg@mellanox.com> >Date: 02/07/2020 03:18PM >Cc: dledford@redhat.com, bmt@zurich.ibm.com, >linux-rdma@vger.kernel.org, bharat@chelsio.com, nirranjan@chelsio.com >Subject: [EXTERNAL] Re: [PATCH for-review/for-rc/for-rc] RDMA/siw: >Remove unwanted WARN_ON in siw_cm_llp_data_ready() > >On Fri, Feb 07, 2020 at 05:22:09PM +0530, Krishnamraju Eraparaju >wrote: >> Warnings like below can fill up the dmesg while disconnecting RDMA >> connections. >> Hence, removing the unwanted WARN_ON. > >Please explain why it the code is correct to take this error >path. Bernard clearly thought this shouldn't be happening > >Jason > > I have to look into it as well. It's maybe on me revising my thoughts. Thanks, Bernard.
On Friday, February 02/07/20, 2020 at 10:18:20 -0400, Jason Gunthorpe wrote: > On Fri, Feb 07, 2020 at 05:22:09PM +0530, Krishnamraju Eraparaju wrote: > > Warnings like below can fill up the dmesg while disconnecting RDMA > > connections. > > Hence, removing the unwanted WARN_ON. > > Please explain why it the code is correct to take this error > path. Bernard clearly thought this shouldn't be happening > > Jason As part of iSER multipath testcase, target(iw_cxgb4) responds with MPA reject to initiator(SIW) when iw_cxgb4 resources gets exhaused(expected as per testcase), then SIW performs the connection teardown and dissociates 'cep' from tcp socket 'sk'. And if any "data_ready" notifications from TCP stack after this connection teardown will hit WARN_ON() in siw_cm_llp_data_ready(). Bernard, is this WARN_ON() useful to identify any error conditions? Thanks, Krishna.
-----"Krishnamraju Eraparaju" <krishna2@chelsio.com> wrote: ----- >To: "Jason Gunthorpe" <jgg@mellanox.com> >From: "Krishnamraju Eraparaju" <krishna2@chelsio.com> >Date: 02/10/2020 07:00PM >Cc: dledford@redhat.com, bmt@zurich.ibm.com, >linux-rdma@vger.kernel.org, bharat@chelsio.com, nirranjan@chelsio.com >Subject: [EXTERNAL] Re: [PATCH for-review/for-rc/for-rc] RDMA/siw: >Remove unwanted WARN_ON in siw_cm_llp_data_ready() > >On Friday, February 02/07/20, 2020 at 10:18:20 -0400, Jason Gunthorpe >wrote: >> On Fri, Feb 07, 2020 at 05:22:09PM +0530, Krishnamraju Eraparaju >wrote: >> > Warnings like below can fill up the dmesg while disconnecting >RDMA >> > connections. >> > Hence, removing the unwanted WARN_ON. >> >> Please explain why it the code is correct to take this error >> path. Bernard clearly thought this shouldn't be happening >> >> Jason >As part of iSER multipath testcase, target(iw_cxgb4) responds with >MPA reject >to initiator(SIW) when iw_cxgb4 resources gets exhaused(expected as >per >testcase), then SIW performs the connection teardown and dissociates >'cep' from tcp socket 'sk'. And if any "data_ready" notifications >from >TCP stack after this connection teardown will hit WARN_ON() in >siw_cm_llp_data_ready(). > >Bernard, is this WARN_ON() useful to identify any error conditions? > Let me try recreating the issue. I'll come back asap. Thanks Bernard.
-----"Krishnamraju Eraparaju" <krishna2@chelsio.com> wrote: ----- >To: "Jason Gunthorpe" <jgg@mellanox.com> >From: "Krishnamraju Eraparaju" <krishna2@chelsio.com> >Date: 02/10/2020 07:00PM >Cc: dledford@redhat.com, bmt@zurich.ibm.com, >linux-rdma@vger.kernel.org, bharat@chelsio.com, nirranjan@chelsio.com >Subject: [EXTERNAL] Re: [PATCH for-review/for-rc/for-rc] RDMA/siw: >Remove unwanted WARN_ON in siw_cm_llp_data_ready() > >On Friday, February 02/07/20, 2020 at 10:18:20 -0400, Jason Gunthorpe >wrote: >> On Fri, Feb 07, 2020 at 05:22:09PM +0530, Krishnamraju Eraparaju >wrote: >> > Warnings like below can fill up the dmesg while disconnecting >RDMA >> > connections. >> > Hence, removing the unwanted WARN_ON. >> >> Please explain why it the code is correct to take this error >> path. Bernard clearly thought this shouldn't be happening >> >> Jason >As part of iSER multipath testcase, target(iw_cxgb4) responds with >MPA reject >to initiator(SIW) when iw_cxgb4 resources gets exhaused(expected as >per >testcase), then SIW performs the connection teardown and dissociates >'cep' from tcp socket 'sk'. And if any "data_ready" notifications >from >TCP stack after this connection teardown will hit WARN_ON() in >siw_cm_llp_data_ready(). > >Bernard, is this WARN_ON() useful to identify any error conditions? > So, this WARN_ON() is wrong. It can very well happen that the socket already got disassociated from siw, but a sockets data_ready() upcall races with the disassociation procedure. Since we hold the sk_callback_lock while setting or testing sk->sk_user_data, a NULL pointer tells us this socket just got disassociated from siw and we can bail out. Thanks Krishna for finding that out. Your patch is correct. Bernard.
diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c index 3bccfef40e7e..bcbc54998ace 100644 --- a/drivers/infiniband/sw/siw/siw_cm.c +++ b/drivers/infiniband/sw/siw/siw_cm.c @@ -1226,7 +1226,6 @@ static void siw_cm_llp_data_ready(struct sock *sk) cep = sk_to_cep(sk); if (!cep) { - WARN_ON(1); goto out; } siw_dbg_cep(cep, "state: %d\n", cep->state);
Warnings like below can fill up the dmesg while disconnecting RDMA connections. Hence, removing the unwanted WARN_ON. [72103.557612] WARNING: CPU: 6 PID: 0 at drivers/infiniband/sw/siw/siw_cm.c:1229 siw_cm_llp_data_ready+0xc1/0xd0 [siw] [72103.557677] RIP: 0010:siw_cm_llp_data_ready+0xc1/0xd0 [siw] [72103.557693] Call Trace: [72103.557711] tcp_data_queue+0x226/0xb40 [72103.557714] tcp_rcv_established+0x220/0x620 [72103.557720] tcp_v4_do_rcv+0x12a/0x1e0 [72103.557722] tcp_v4_rcv+0xb05/0xc00 [72103.557728] ip_local_deliver_finish+0x69/0x210 [72103.557730] ip_local_deliver+0x6b/0xe0 [72103.557735] ip_rcv+0x273/0x362 [72103.557740] __netif_receive_skb_core+0xb35/0xc30 [72103.557752] netif_receive_skb_internal+0x3d/0xb0 [72103.557754] napi_gro_frags+0x13b/0x200 [72103.557788] t4_ethrx_handler+0x433/0x7d0 [cxgb4] [72103.557800] process_responses+0x318/0x580 [cxgb4] [72103.557820] napi_rx_handler+0x14/0x100 [cxgb4] [72103.557822] net_rx_action+0x149/0x3b0 [72103.557826] __do_softirq+0xe3/0x30a [72103.557831] irq_exit+0x100/0x110 [72103.557834] do_IRQ+0x7f/0xe0 [72103.557837] common_interrupt+0xf/0xf Signed-off-by: Krishnamraju Eraparaju <krishna2@chelsio.com> --- drivers/infiniband/sw/siw/siw_cm.c | 1 - 1 file changed, 1 deletion(-)