RE: [PATCH v1 05/13] xprtrdma: Don't drain CQs on transport disconnect

Devesh Sharma
2014-07-02T19:48:47Z

> -----Original Message-----
> From: Steve Wise [mailto:swise@opengridcomputing.com]
> Sent: Thursday, July 03, 2014 1:16 AM
> To: 'Chuck Lever'; Devesh Sharma
> Cc: linux-rdma@vger.kernel.org; 'Linux NFS Mailing List'
> Subject: RE: [PATCH v1 05/13] xprtrdma: Don't drain CQs on transport
> disconnect
>
>
>
> > -----Original Message-----
> > From: Chuck Lever [mailto:chuck.lever@oracle.com]
> > Sent: Wednesday, July 02, 2014 2:40 PM
> > To: Steve Wise; Devesh Sharma
> > Cc: linux-rdma@vger.kernel.org; Linux NFS Mailing List
> > Subject: Re: [PATCH v1 05/13] xprtrdma: Don't drain CQs on transport
> > disconnect
> >
> >
> > On Jul 2, 2014, at 3:28 PM, Steve Wise <swise@opengridcomputing.com>
> wrote:
> >
> > > On 7/2/2014 2:06 PM, Devesh Sharma wrote:
> > >> This change is very much prone to generate poll_cq errors because
> > >> of un-cleaned
> > completions which still
> > >> point to the non-existent QPs. On the new connection when these
> > >> completions are polled,
> > the poll_cq will
> > >> fail because old QP pointer is already NULL.
> > >> Did anyone hit this situation during their testing?
> >
> > I tested this aggressively with a fault injector that triggers regular
> > connection disruption.
> >
> > > Hey Devesh,
> > >
> > > iw_cxgb4 will silently toss CQEs if the QP is not active.
> >
> > xprtrdma relies on getting a completion (either successful or in
> > error) for every WR it has posted. The goal of this patch is to avoid
> > throwing away queued completions after a transport disconnect so we
> > don't lose track of FRMR rkey updates (FAST_REG_MR and LOCAL_INV
> > completions) and we can capture all RPC replies posted before the
> connection was lost.
> >
> > Sounds like we also need to keep the QP around, even in error state,
> > until all known WRs on that QP have completed?
> >

Why not poll and process every completion during rpcrdma_cq_cleanup()....

>
> Perhaps.
>
> >
> > >
> > >
> > >>> -----Original Message-----
> > >>> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> > >>> owner@vger.kernel.org] On Behalf Of Chuck Lever
> > >>> Sent: Tuesday, June 24, 2014 4:10 AM
> > >>> To: linux-rdma@vger.kernel.org; linux-nfs@vger.kernel.org
> > >>> Subject: [PATCH v1 05/13] xprtrdma: Don't drain CQs on transport
> > >>> disconnect
> > >>>
> > >>> CQs are not destroyed until unmount. By draining CQs on transport
> > >>> disconnect, successful completions that can change the
> > >>> r.frmr.state field can be missed.
> > >>>
> > >>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > >>> ---
> > >>> net/sunrpc/xprtrdma/verbs.c | 5 -----
> > >>> 1 file changed, 5 deletions(-)
> > >>>
> > >>> diff --git a/net/sunrpc/xprtrdma/verbs.c
> > >>> b/net/sunrpc/xprtrdma/verbs.c index 3c7f904..451e100 100644
> > >>> --- a/net/sunrpc/xprtrdma/verbs.c
> > >>> +++ b/net/sunrpc/xprtrdma/verbs.c
> > >>> @@ -873,9 +873,6 @@ retry:
> > >>> dprintk("RPC: %s: rpcrdma_ep_disconnect"
> > >>> " status %i\n", __func__, rc);
> > >>>
> > >>> - rpcrdma_clean_cq(ep->rep_attr.recv_cq);
> > >>> - rpcrdma_clean_cq(ep->rep_attr.send_cq);
> > >>> -
> > >>> xprt = container_of(ia, struct rpcrdma_xprt, rx_ia);
> > >>> id = rpcrdma_create_id(xprt, ia,
> > >>> (struct sockaddr *)&xprt->rx_data.addr);
> @@ -985,8 +982,6 @@
> > >>> rpcrdma_ep_disconnect(struct rpcrdma_ep *ep, struct rpcrdma_ia
> > >>> *ia) {
> > >>> int rc;
> > >>>
> > >>> - rpcrdma_clean_cq(ep->rep_attr.recv_cq);
> > >>> - rpcrdma_clean_cq(ep->rep_attr.send_cq);
> > >>> rc = rdma_disconnect(ia->ri_id);
> > >>> if (!rc) {
> > >>> /* returns without wait if not connected */
> > >>>
> > >>> --
> > >>> To unsubscribe from this list: send the line "unsubscribe
> > >>> linux-rdma" in the body of a message to majordomo@vger.kernel.org
> > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >> N r y b X ǧv ^ )޺{.n + { " ^n r z \x1a h & \x1e G
> > >> h \x03( 階
> > ݢj" \x1a ^[m z ޖ f h ~ mml==
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> > > in the body of a message to majordomo@vger.kernel.org More
> majordomo
> > > info at http://vger.kernel.org/majordomo-info.html
> >
> > --
> > Chuck Lever
> > chuck[dot]lever[at]oracle[dot]com
> >
> >
>