Return-Path: Received: from aserp1040.oracle.com ([141.146.126.69]:20903 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753291AbbCQPEh convert rfc822-to-8bit (ORCPT ); Tue, 17 Mar 2015 11:04:37 -0400 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) Subject: Re: [PATCH v1 06/16] xprtrdma: Add a "deregister_external" op for each memreg mode From: Chuck Lever In-Reply-To: <55083C31.9020704@Netapp.com> Date: Tue, 17 Mar 2015 08:04:33 -0700 Cc: Linux NFS Mailing List Message-Id: References: <20150313211124.22471.14517.stgit@manet.1015granger.net> <20150313212214.22471.33013.stgit@manet.1015granger.net> <55083C31.9020704@Netapp.com> To: Anna Schumaker Sender: linux-nfs-owner@vger.kernel.org List-ID: On Mar 17, 2015, at 7:37 AM, Anna Schumaker wrote: > On 03/13/2015 05:22 PM, Chuck Lever wrote: >> There is very little common processing among the different external >> memory deregistration functions. >> >> In addition, instead of calling the deregistration function for each >> segment, have one call release all segments for a request. This makes >> the API a little asymmetrical, but a hair faster. > > The common processing would be the for-each loop that you moved into the ro_unmap functions. I'm not completely sold on this... how often do unmaps happen? Once for every RPC. > Anna > >> >> Signed-off-by: Chuck Lever >> --- >> net/sunrpc/xprtrdma/fmr_ops.c | 37 ++++++++++++++++ >> net/sunrpc/xprtrdma/frwr_ops.c | 46 ++++++++++++++++++++ >> net/sunrpc/xprtrdma/physical_ops.c | 13 ++++++ >> net/sunrpc/xprtrdma/rpc_rdma.c | 7 +-- >> net/sunrpc/xprtrdma/transport.c | 8 +--- >> net/sunrpc/xprtrdma/verbs.c | 81 ------------------------------------ >> net/sunrpc/xprtrdma/xprt_rdma.h | 5 +- >> 7 files changed, 103 insertions(+), 94 deletions(-) >> >> diff --git a/net/sunrpc/xprtrdma/fmr_ops.c b/net/sunrpc/xprtrdma/fmr_ops.c >> index 45fb646..9b983b4 100644 >> --- a/net/sunrpc/xprtrdma/fmr_ops.c >> +++ b/net/sunrpc/xprtrdma/fmr_ops.c >> @@ -20,6 +20,32 @@ >> /* Maximum scatter/gather per FMR */ >> #define RPCRDMA_MAX_FMR_SGES (64) >> >> +/* Use the ib_unmap_fmr() verb to prevent further remote >> + * access via RDMA READ or RDMA WRITE. >> + */ >> +static int >> +__fmr_unmap(struct rpcrdma_xprt *r_xprt, struct rpcrdma_mr_seg *seg) >> +{ >> + struct rpcrdma_ia *ia = &r_xprt->rx_ia; >> + struct rpcrdma_mr_seg *seg1 = seg; >> + int rc, nsegs = seg->mr_nsegs; >> + LIST_HEAD(l); >> + >> + list_add(&seg1->rl_mw->r.fmr->list, &l); >> + rc = ib_unmap_fmr(&l); >> + read_lock(&ia->ri_qplock); >> + while (seg1->mr_nsegs--) >> + rpcrdma_unmap_one(ia, seg++); >> + read_unlock(&ia->ri_qplock); >> + if (rc) >> + goto out_err; >> + return nsegs; >> + >> +out_err: >> + dprintk("RPC: %s: ib_unmap_fmr status %i\n", __func__, rc); >> + return nsegs; >> +} >> + >> /* FMR mode conveys up to 64 pages of payload per chunk segment. >> */ >> static size_t >> @@ -79,8 +105,19 @@ out_maperr: >> return rc; >> } >> >> +static void >> +fmr_op_unmap(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req, >> + unsigned int count) >> +{ >> + unsigned int i; >> + >> + for (i = 0; count--;) >> + i += __fmr_unmap(r_xprt, &req->rl_segments[i]); >> +} >> + >> const struct rpcrdma_memreg_ops rpcrdma_fmr_memreg_ops = { >> .ro_map = fmr_op_map, >> + .ro_unmap = fmr_op_unmap, >> .ro_maxpages = fmr_op_maxpages, >> .ro_displayname = "fmr", >> }; >> diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c >> index 2b5ccb0..05b5761 100644 >> --- a/net/sunrpc/xprtrdma/frwr_ops.c >> +++ b/net/sunrpc/xprtrdma/frwr_ops.c >> @@ -17,6 +17,41 @@ >> # define RPCDBG_FACILITY RPCDBG_TRANS >> #endif >> >> +/* Post a LOCAL_INV Work Request to prevent further remote access >> + * via RDMA READ or RDMA WRITE. >> + */ >> +static int >> +__frwr_unmap(struct rpcrdma_xprt *r_xprt, struct rpcrdma_mr_seg *seg) >> +{ >> + struct rpcrdma_mr_seg *seg1 = seg; >> + struct rpcrdma_ia *ia = &r_xprt->rx_ia; >> + struct ib_send_wr invalidate_wr, *bad_wr; >> + int rc, nsegs = seg->mr_nsegs; >> + >> + seg1->rl_mw->r.frmr.fr_state = FRMR_IS_INVALID; >> + >> + memset(&invalidate_wr, 0, sizeof(invalidate_wr)); >> + invalidate_wr.wr_id = (unsigned long)(void *)seg1->rl_mw; >> + invalidate_wr.opcode = IB_WR_LOCAL_INV; >> + invalidate_wr.ex.invalidate_rkey = seg1->rl_mw->r.frmr.fr_mr->rkey; >> + DECR_CQCOUNT(&r_xprt->rx_ep); >> + >> + read_lock(&ia->ri_qplock); >> + while (seg1->mr_nsegs--) >> + rpcrdma_unmap_one(ia, seg++); >> + rc = ib_post_send(ia->ri_id->qp, &invalidate_wr, &bad_wr); >> + read_unlock(&ia->ri_qplock); >> + if (rc) >> + goto out_err; >> + return nsegs; >> + >> +out_err: >> + /* Force rpcrdma_buffer_get() to retry */ >> + seg1->rl_mw->r.frmr.fr_state = FRMR_IS_STALE; >> + dprintk("RPC: %s: ib_post_send status %i\n", __func__, rc); >> + return nsegs; >> +} >> + >> /* FRWR mode conveys a list of pages per chunk segment. The >> * maximum length of that list is the FRWR page list depth. >> */ >> @@ -116,8 +151,19 @@ out_err: >> return rc; >> } >> >> +static void >> +frwr_op_unmap(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req, >> + unsigned int count) >> +{ >> + unsigned int i; >> + >> + for (i = 0; count--;) >> + i += __frwr_unmap(r_xprt, &req->rl_segments[i]); >> +} >> + >> const struct rpcrdma_memreg_ops rpcrdma_frwr_memreg_ops = { >> .ro_map = frwr_op_map, >> + .ro_unmap = frwr_op_unmap, >> .ro_maxpages = frwr_op_maxpages, >> .ro_displayname = "frwr", >> }; >> diff --git a/net/sunrpc/xprtrdma/physical_ops.c b/net/sunrpc/xprtrdma/physical_ops.c >> index 5a284ee..f2c15be 100644 >> --- a/net/sunrpc/xprtrdma/physical_ops.c >> +++ b/net/sunrpc/xprtrdma/physical_ops.c >> @@ -44,8 +44,21 @@ physical_op_map(struct rpcrdma_xprt *r_xprt, struct rpcrdma_mr_seg *seg, >> return 1; >> } >> >> +/* Unmap a memory region, but leave it registered. >> + */ >> +static void >> +physical_op_unmap(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req, >> + unsigned int count) >> +{ >> + unsigned int i; >> + >> + for (i = 0; i < count; i++) >> + rpcrdma_unmap_one(&r_xprt->rx_ia, &req->rl_segments[i]); >> +} >> + >> const struct rpcrdma_memreg_ops rpcrdma_physical_memreg_ops = { >> .ro_map = physical_op_map, >> + .ro_unmap = physical_op_unmap, >> .ro_maxpages = physical_op_maxpages, >> .ro_displayname = "physical", >> }; >> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c >> index 6ab1d03..7b51d9d 100644 >> --- a/net/sunrpc/xprtrdma/rpc_rdma.c >> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c >> @@ -284,11 +284,8 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target, >> return (unsigned char *)iptr - (unsigned char *)headerp; >> >> out: >> - if (r_xprt->rx_ia.ri_memreg_strategy != RPCRDMA_FRMR) { >> - for (pos = 0; nchunks--;) >> - pos += rpcrdma_deregister_external( >> - &req->rl_segments[pos], r_xprt); >> - } >> + if (r_xprt->rx_ia.ri_memreg_strategy != RPCRDMA_FRMR) >> + r_xprt->rx_ia.ri_ops->ro_unmap(r_xprt, req, nchunks); >> return n; >> } >> >> diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c >> index 9a9da40..c484671 100644 >> --- a/net/sunrpc/xprtrdma/transport.c >> +++ b/net/sunrpc/xprtrdma/transport.c >> @@ -571,7 +571,6 @@ xprt_rdma_free(void *buffer) >> struct rpcrdma_req *req; >> struct rpcrdma_xprt *r_xprt; >> struct rpcrdma_regbuf *rb; >> - int i; >> >> if (buffer == NULL) >> return; >> @@ -582,11 +581,8 @@ xprt_rdma_free(void *buffer) >> >> dprintk("RPC: %s: called on 0x%p\n", __func__, req->rl_reply); >> >> - for (i = 0; req->rl_nchunks;) { >> - --req->rl_nchunks; >> - i += rpcrdma_deregister_external( >> - &req->rl_segments[i], r_xprt); >> - } >> + r_xprt->rx_ia.ri_ops->ro_unmap(r_xprt, req, req->rl_nchunks); >> + req->rl_nchunks = 0; >> >> rpcrdma_buffer_put(req); >> } >> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c >> index 851ed97..a1621fd 100644 >> --- a/net/sunrpc/xprtrdma/verbs.c >> +++ b/net/sunrpc/xprtrdma/verbs.c >> @@ -1509,7 +1509,7 @@ rpcrdma_buffer_put_sendbuf(struct rpcrdma_req *req, struct rpcrdma_buffer *buf) >> } >> } >> >> -/* rpcrdma_unmap_one() was already done by rpcrdma_deregister_frmr_external(). >> +/* rpcrdma_unmap_one() was already done during deregistration. >> * Redo only the ib_post_send(). >> */ >> static void >> @@ -1889,85 +1889,6 @@ rpcrdma_unmap_one(struct rpcrdma_ia *ia, struct rpcrdma_mr_seg *seg) >> seg->mr_dma, seg->mr_dmalen, seg->mr_dir); >> } >> >> -static int >> -rpcrdma_deregister_frmr_external(struct rpcrdma_mr_seg *seg, >> - struct rpcrdma_ia *ia, struct rpcrdma_xprt *r_xprt) >> -{ >> - struct rpcrdma_mr_seg *seg1 = seg; >> - struct ib_send_wr invalidate_wr, *bad_wr; >> - int rc; >> - >> - seg1->rl_mw->r.frmr.fr_state = FRMR_IS_INVALID; >> - >> - memset(&invalidate_wr, 0, sizeof invalidate_wr); >> - invalidate_wr.wr_id = (unsigned long)(void *)seg1->rl_mw; >> - invalidate_wr.opcode = IB_WR_LOCAL_INV; >> - invalidate_wr.ex.invalidate_rkey = seg1->rl_mw->r.frmr.fr_mr->rkey; >> - DECR_CQCOUNT(&r_xprt->rx_ep); >> - >> - read_lock(&ia->ri_qplock); >> - while (seg1->mr_nsegs--) >> - rpcrdma_unmap_one(ia, seg++); >> - rc = ib_post_send(ia->ri_id->qp, &invalidate_wr, &bad_wr); >> - read_unlock(&ia->ri_qplock); >> - if (rc) { >> - /* Force rpcrdma_buffer_get() to retry */ >> - seg1->rl_mw->r.frmr.fr_state = FRMR_IS_STALE; >> - dprintk("RPC: %s: failed ib_post_send for invalidate," >> - " status %i\n", __func__, rc); >> - } >> - return rc; >> -} >> - >> -static int >> -rpcrdma_deregister_fmr_external(struct rpcrdma_mr_seg *seg, >> - struct rpcrdma_ia *ia) >> -{ >> - struct rpcrdma_mr_seg *seg1 = seg; >> - LIST_HEAD(l); >> - int rc; >> - >> - list_add(&seg1->rl_mw->r.fmr->list, &l); >> - rc = ib_unmap_fmr(&l); >> - read_lock(&ia->ri_qplock); >> - while (seg1->mr_nsegs--) >> - rpcrdma_unmap_one(ia, seg++); >> - read_unlock(&ia->ri_qplock); >> - if (rc) >> - dprintk("RPC: %s: failed ib_unmap_fmr," >> - " status %i\n", __func__, rc); >> - return rc; >> -} >> - >> -int >> -rpcrdma_deregister_external(struct rpcrdma_mr_seg *seg, >> - struct rpcrdma_xprt *r_xprt) >> -{ >> - struct rpcrdma_ia *ia = &r_xprt->rx_ia; >> - int nsegs = seg->mr_nsegs, rc; >> - >> - switch (ia->ri_memreg_strategy) { >> - >> - case RPCRDMA_ALLPHYSICAL: >> - read_lock(&ia->ri_qplock); >> - rpcrdma_unmap_one(ia, seg); >> - read_unlock(&ia->ri_qplock); >> - break; >> - >> - case RPCRDMA_FRMR: >> - rc = rpcrdma_deregister_frmr_external(seg, ia, r_xprt); >> - break; >> - >> - case RPCRDMA_MTHCAFMR: >> - rc = rpcrdma_deregister_fmr_external(seg, ia); >> - break; >> - >> - default: >> - break; >> - } >> - return nsegs; >> -} >> - >> /* >> * Prepost any receive buffer, then post send. >> * >> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h >> index 7bf077b..3aabbb2 100644 >> --- a/net/sunrpc/xprtrdma/xprt_rdma.h >> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h >> @@ -338,6 +338,8 @@ struct rpcrdma_xprt; >> struct rpcrdma_memreg_ops { >> int (*ro_map)(struct rpcrdma_xprt *, >> struct rpcrdma_mr_seg *, int, bool); >> + void (*ro_unmap)(struct rpcrdma_xprt *, >> + struct rpcrdma_req *, unsigned int); >> size_t (*ro_maxpages)(struct rpcrdma_xprt *); >> const char *ro_displayname; >> }; >> @@ -405,9 +407,6 @@ void rpcrdma_buffer_put(struct rpcrdma_req *); >> void rpcrdma_recv_buffer_get(struct rpcrdma_req *); >> void rpcrdma_recv_buffer_put(struct rpcrdma_rep *); >> >> -int rpcrdma_deregister_external(struct rpcrdma_mr_seg *, >> - struct rpcrdma_xprt *); >> - >> struct rpcrdma_regbuf *rpcrdma_alloc_regbuf(struct rpcrdma_ia *, >> size_t, gfp_t); >> void rpcrdma_free_regbuf(struct rpcrdma_ia *, >> >> -- >> 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 >> > > -- > 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