From: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> To: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: [PATCH v2 02/13] xprtrdma: Pre-mark remotely invalidated MRs Date: Thu, 08 Jun 2017 11:51:56 -0400 [thread overview] Message-ID: <20170608155156.18945.29883.stgit@manet.1015granger.net> (raw) In-Reply-To: <20170608154339.18945.5500.stgit-FYjufvaPoItvLzlybtyyYzGyq/o6K9yX@public.gmane.org> There are rare cases where an rpcrdma_req and its matched rpcrdma_rep can be re-used, via rpcrdma_buffer_put, while the RPC reply handler is still using that req. This is typically due to a signal firing at just the wrong instant. As part of closing this race window, avoid using the wrong rpcrdma_rep to detect remotely invalidated MRs. Mark MRs as invalidated while we are sure the rep is still OK to use. BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=305 Fixes: 68791649a725 ('xprtrdma: Invalidate in the RPC reply ... ') Signed-off-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> --- net/sunrpc/xprtrdma/frwr_ops.c | 4 +--- net/sunrpc/xprtrdma/rpc_rdma.c | 22 ++++++++++++++++++++-- net/sunrpc/xprtrdma/verbs.c | 1 + net/sunrpc/xprtrdma/xprt_rdma.h | 6 ++++++ 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c index f81dd93..31290cb 100644 --- a/net/sunrpc/xprtrdma/frwr_ops.c +++ b/net/sunrpc/xprtrdma/frwr_ops.c @@ -464,7 +464,6 @@ frwr_op_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req) { struct ib_send_wr *first, **prev, *last, *bad_wr; - struct rpcrdma_rep *rep = req->rl_reply; struct rpcrdma_ia *ia = &r_xprt->rx_ia; struct rpcrdma_frmr *f; struct rpcrdma_mw *mw; @@ -483,8 +482,7 @@ list_for_each_entry(mw, &req->rl_registered, mw_list) { mw->frmr.fr_state = FRMR_IS_INVALID; - if ((rep->rr_wc_flags & IB_WC_WITH_INVALIDATE) && - (mw->mw_handle == rep->rr_inv_rkey)) + if (mw->mw_flags & RPCRDMA_MW_F_RI) continue; f = &mw->frmr; diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c index 694e9b1..2356a63 100644 --- a/net/sunrpc/xprtrdma/rpc_rdma.c +++ b/net/sunrpc/xprtrdma/rpc_rdma.c @@ -928,6 +928,24 @@ static bool rpcrdma_results_inline(struct rpcrdma_xprt *r_xprt, return fixup_copy_count; } +/* Caller must guarantee @rep remains stable during this call. + */ +static void +rpcrdma_mark_remote_invalidation(struct list_head *mws, + struct rpcrdma_rep *rep) +{ + struct rpcrdma_mw *mw; + + if (!(rep->rr_wc_flags & IB_WC_WITH_INVALIDATE)) + return; + + list_for_each_entry(mw, mws, mw_list) + if (mw->mw_handle == rep->rr_inv_rkey) { + mw->mw_flags = RPCRDMA_MW_F_RI; + break; /* only one invalidated MR per RPC */ + } +} + #if defined(CONFIG_SUNRPC_BACKCHANNEL) /* By convention, backchannel calls arrive via rdma_msg type * messages, and never populate the chunk lists. This makes @@ -1006,13 +1024,13 @@ static bool rpcrdma_results_inline(struct rpcrdma_xprt *r_xprt, /* Sanity checking has passed. We are now committed * to complete this transaction. */ + rpcrdma_mark_remote_invalidation(&req->rl_registered, rep); list_del_init(&rqst->rq_list); + req->rl_reply = rep; spin_unlock_bh(&xprt->transport_lock); dprintk("RPC: %s: reply %p completes request %p (xid 0x%08x)\n", __func__, rep, req, be32_to_cpu(headerp->rm_xid)); - /* from here on, the reply is no longer an orphan */ - req->rl_reply = rep; xprt->reestablish_timeout = 0; if (headerp->rm_vers != rpcrdma_version) diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c index 3dbce9a..a8be66d 100644 --- a/net/sunrpc/xprtrdma/verbs.c +++ b/net/sunrpc/xprtrdma/verbs.c @@ -1187,6 +1187,7 @@ struct rpcrdma_mw * if (!mw) goto out_nomws; + mw->mw_flags = 0; return mw; out_nomws: diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h index 1d66acf..2e02733 100644 --- a/net/sunrpc/xprtrdma/xprt_rdma.h +++ b/net/sunrpc/xprtrdma/xprt_rdma.h @@ -271,6 +271,7 @@ struct rpcrdma_mw { struct scatterlist *mw_sg; int mw_nents; enum dma_data_direction mw_dir; + unsigned long mw_flags; union { struct rpcrdma_fmr fmr; struct rpcrdma_frmr frmr; @@ -282,6 +283,11 @@ struct rpcrdma_mw { struct list_head mw_all; }; +/* mw_flags */ +enum { + RPCRDMA_MW_F_RI = 1, +}; + /* * struct rpcrdma_req -- structure central to the request/reply sequence. * -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Chuck Lever <chuck.lever@oracle.com> To: linux-rdma@vger.kernel.org, linux-nfs@vger.kernel.org Subject: [PATCH v2 02/13] xprtrdma: Pre-mark remotely invalidated MRs Date: Thu, 08 Jun 2017 11:51:56 -0400 [thread overview] Message-ID: <20170608155156.18945.29883.stgit@manet.1015granger.net> (raw) In-Reply-To: <20170608154339.18945.5500.stgit@manet.1015granger.net> There are rare cases where an rpcrdma_req and its matched rpcrdma_rep can be re-used, via rpcrdma_buffer_put, while the RPC reply handler is still using that req. This is typically due to a signal firing at just the wrong instant. As part of closing this race window, avoid using the wrong rpcrdma_rep to detect remotely invalidated MRs. Mark MRs as invalidated while we are sure the rep is still OK to use. BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=305 Fixes: 68791649a725 ('xprtrdma: Invalidate in the RPC reply ... ') Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- net/sunrpc/xprtrdma/frwr_ops.c | 4 +--- net/sunrpc/xprtrdma/rpc_rdma.c | 22 ++++++++++++++++++++-- net/sunrpc/xprtrdma/verbs.c | 1 + net/sunrpc/xprtrdma/xprt_rdma.h | 6 ++++++ 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c index f81dd93..31290cb 100644 --- a/net/sunrpc/xprtrdma/frwr_ops.c +++ b/net/sunrpc/xprtrdma/frwr_ops.c @@ -464,7 +464,6 @@ frwr_op_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req) { struct ib_send_wr *first, **prev, *last, *bad_wr; - struct rpcrdma_rep *rep = req->rl_reply; struct rpcrdma_ia *ia = &r_xprt->rx_ia; struct rpcrdma_frmr *f; struct rpcrdma_mw *mw; @@ -483,8 +482,7 @@ list_for_each_entry(mw, &req->rl_registered, mw_list) { mw->frmr.fr_state = FRMR_IS_INVALID; - if ((rep->rr_wc_flags & IB_WC_WITH_INVALIDATE) && - (mw->mw_handle == rep->rr_inv_rkey)) + if (mw->mw_flags & RPCRDMA_MW_F_RI) continue; f = &mw->frmr; diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c index 694e9b1..2356a63 100644 --- a/net/sunrpc/xprtrdma/rpc_rdma.c +++ b/net/sunrpc/xprtrdma/rpc_rdma.c @@ -928,6 +928,24 @@ static bool rpcrdma_results_inline(struct rpcrdma_xprt *r_xprt, return fixup_copy_count; } +/* Caller must guarantee @rep remains stable during this call. + */ +static void +rpcrdma_mark_remote_invalidation(struct list_head *mws, + struct rpcrdma_rep *rep) +{ + struct rpcrdma_mw *mw; + + if (!(rep->rr_wc_flags & IB_WC_WITH_INVALIDATE)) + return; + + list_for_each_entry(mw, mws, mw_list) + if (mw->mw_handle == rep->rr_inv_rkey) { + mw->mw_flags = RPCRDMA_MW_F_RI; + break; /* only one invalidated MR per RPC */ + } +} + #if defined(CONFIG_SUNRPC_BACKCHANNEL) /* By convention, backchannel calls arrive via rdma_msg type * messages, and never populate the chunk lists. This makes @@ -1006,13 +1024,13 @@ static bool rpcrdma_results_inline(struct rpcrdma_xprt *r_xprt, /* Sanity checking has passed. We are now committed * to complete this transaction. */ + rpcrdma_mark_remote_invalidation(&req->rl_registered, rep); list_del_init(&rqst->rq_list); + req->rl_reply = rep; spin_unlock_bh(&xprt->transport_lock); dprintk("RPC: %s: reply %p completes request %p (xid 0x%08x)\n", __func__, rep, req, be32_to_cpu(headerp->rm_xid)); - /* from here on, the reply is no longer an orphan */ - req->rl_reply = rep; xprt->reestablish_timeout = 0; if (headerp->rm_vers != rpcrdma_version) diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c index 3dbce9a..a8be66d 100644 --- a/net/sunrpc/xprtrdma/verbs.c +++ b/net/sunrpc/xprtrdma/verbs.c @@ -1187,6 +1187,7 @@ struct rpcrdma_mw * if (!mw) goto out_nomws; + mw->mw_flags = 0; return mw; out_nomws: diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h index 1d66acf..2e02733 100644 --- a/net/sunrpc/xprtrdma/xprt_rdma.h +++ b/net/sunrpc/xprtrdma/xprt_rdma.h @@ -271,6 +271,7 @@ struct rpcrdma_mw { struct scatterlist *mw_sg; int mw_nents; enum dma_data_direction mw_dir; + unsigned long mw_flags; union { struct rpcrdma_fmr fmr; struct rpcrdma_frmr frmr; @@ -282,6 +283,11 @@ struct rpcrdma_mw { struct list_head mw_all; }; +/* mw_flags */ +enum { + RPCRDMA_MW_F_RI = 1, +}; + /* * struct rpcrdma_req -- structure central to the request/reply sequence. *
next prev parent reply other threads:[~2017-06-08 15:51 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-06-08 15:51 [PATCH v2 00/13] NFS/RDMA client-side patches proposed for v4.13 Chuck Lever 2017-06-08 15:51 ` Chuck Lever [not found] ` <20170608154339.18945.5500.stgit-FYjufvaPoItvLzlybtyyYzGyq/o6K9yX@public.gmane.org> 2017-06-08 15:51 ` [PATCH v2 01/13] xprtrdma: On invalidation failure, remove MWs from rl_registered Chuck Lever 2017-06-08 15:51 ` Chuck Lever 2017-06-08 15:51 ` Chuck Lever [this message] 2017-06-08 15:51 ` [PATCH v2 02/13] xprtrdma: Pre-mark remotely invalidated MRs Chuck Lever 2017-06-08 15:52 ` [PATCH v2 03/13] xprtrdma: Pass only the list of registered MRs to ro_unmap_sync Chuck Lever 2017-06-08 15:52 ` Chuck Lever 2017-06-08 15:52 ` [PATCH v2 04/13] xprtrdma: Rename rpcrdma_req::rl_free Chuck Lever 2017-06-08 15:52 ` Chuck Lever [not found] ` <20170608155212.18945.37327.stgit-FYjufvaPoItvLzlybtyyYzGyq/o6K9yX@public.gmane.org> 2017-06-09 18:58 ` Anna Schumaker 2017-06-09 18:58 ` Anna Schumaker [not found] ` <7ee9b1fd-9628-2767-444e-90c6736d63f9-ZwjVKphTwtPQT0dZR+AlfA@public.gmane.org> 2017-06-09 19:03 ` Chuck Lever 2017-06-09 19:03 ` Chuck Lever [not found] ` <5332A4D9-ADC4-4DA4-A20E-ACC6019F83BF-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 2017-06-09 19:12 ` Chuck Lever 2017-06-09 19:12 ` Chuck Lever [not found] ` <C0C5BD8B-9D97-48F4-A0AD-C7FC17DA9C00-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 2017-06-09 19:15 ` Anna Schumaker 2017-06-09 19:15 ` Anna Schumaker 2017-06-08 15:52 ` [PATCH v2 05/13] xprtrdma: Fix client lock-up after application signal fires Chuck Lever 2017-06-08 15:52 ` Chuck Lever 2017-06-08 15:52 ` [PATCH v2 06/13] xprtrdma: Fix FRWR invalidation error recovery Chuck Lever 2017-06-08 15:52 ` Chuck Lever 2017-06-08 15:52 ` [PATCH v2 07/13] xprtrdma: Don't defer MR recovery if ro_map fails Chuck Lever 2017-06-08 15:52 ` Chuck Lever 2017-06-08 15:52 ` [PATCH v2 08/13] NFSv4.1: Handle EXCHGID4_FLAG_CONFIRMED_R during NFSv4.1 migration Chuck Lever 2017-06-08 15:52 ` Chuck Lever 2017-06-08 15:52 ` [PATCH v2 09/13] NFSv4.1: Use seqid returned by EXCHANGE_ID after state migration Chuck Lever 2017-06-08 15:52 ` Chuck Lever 2017-06-08 15:53 ` [PATCH v2 10/13] xprtrdma: Demote "connect" log messages Chuck Lever 2017-06-08 15:53 ` Chuck Lever 2017-06-08 15:53 ` [PATCH v2 11/13] xprtrdma: FMR does not need list_del_init() Chuck Lever 2017-06-08 15:53 ` Chuck Lever 2017-06-08 15:53 ` [PATCH v2 12/13] xprtrdma: Replace PAGE_MASK with offset_in_page() Chuck Lever 2017-06-08 15:53 ` Chuck Lever 2017-06-08 15:53 ` [PATCH v2 13/13] xprtrdma: Fix documenting comments in frwr_ops.c Chuck Lever 2017-06-08 15:53 ` Chuck Lever
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20170608155156.18945.29883.stgit@manet.1015granger.net \ --to=chuck.lever-qhclzuegtsvqt0dzr+alfa@public.gmane.org \ --cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.