On Mar 16, 2015, at 3:46 AM, Sagi Grimberg <[email protected]> wrote:
> On 3/13/2015 11:27 PM, Chuck Lever wrote: >> Memory Region objects associated with a transport instance are >> destroyed before the instance is shutdown and destroyed. >> >> Signed-off-by: Chuck Lever <[email protected]> >> --- >> net/sunrpc/xprtrdma/fmr_ops.c | 21 +++++++++++++++ >> net/sunrpc/xprtrdma/frwr_ops.c | 17 ++++++++++++ >> net/sunrpc/xprtrdma/physical_ops.c | 6 ++++ >> net/sunrpc/xprtrdma/verbs.c | 52 >> +----------------------------------- >> net/sunrpc/xprtrdma/xprt_rdma.h | 1 + >> 5 files changed, 46 insertions(+), 51 deletions(-) >> >> diff --git a/net/sunrpc/xprtrdma/fmr_ops.c b/net/sunrpc/xprtrdma/fmr_ops.c >> index 1ccb3de..3115e4b 100644 >> --- a/net/sunrpc/xprtrdma/fmr_ops.c >> +++ b/net/sunrpc/xprtrdma/fmr_ops.c >> @@ -178,11 +178,32 @@ fmr_op_reset(struct rpcrdma_xprt *r_xprt) >> __func__, rc); >> } >> >> +static void >> +fmr_op_destroy(struct rpcrdma_buffer *buf) >> +{ >> + struct rpcrdma_mw *r; >> + int rc; >> + >> + while (!list_empty(&buf->rb_all)) { >> + r = list_entry(buf->rb_all.next, struct rpcrdma_mw, mw_all); >> + list_del(&r->mw_all); >> + list_del(&r->mw_list); > > Again, I understand this patch is just moving code, but is it > guaranteed that mr_list is in rb_mws at this point? Good call, there probably is no such guarantee in the current code base. Since the transport is being destroyed anyway, I think I can just remove that list_del(&r->mw_list). > >> + >> + rc = ib_dealloc_fmr(r->r.fmr); >> + if (rc) >> + dprintk("RPC: %s: ib_dealloc_fmr failed %i\n", >> + __func__, rc); >> + >> + kfree(r); >> + } >> +} >> + >> 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_init = fmr_op_init, >> .ro_reset = fmr_op_reset, >> + .ro_destroy = fmr_op_destroy, >> .ro_displayname = "fmr", >> }; >> diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c >> index b4ce0e5..fc3a228 100644 >> --- a/net/sunrpc/xprtrdma/frwr_ops.c >> +++ b/net/sunrpc/xprtrdma/frwr_ops.c >> @@ -271,11 +271,28 @@ frwr_op_reset(struct rpcrdma_xprt *r_xprt) >> } >> } >> >> +static void >> +frwr_op_destroy(struct rpcrdma_buffer *buf) >> +{ >> + struct rpcrdma_mw *r; >> + >> + while (!list_empty(&buf->rb_all)) { >> + r = list_entry(buf->rb_all.next, struct rpcrdma_mw, mw_all); >> + list_del(&r->mw_all); >> + list_del(&r->mw_list); >> + >> + __frwr_release(r); >> + >> + kfree(r); >> + } >> +} >> + >> 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_init = frwr_op_init, >> .ro_reset = frwr_op_reset, >> + .ro_destroy = frwr_op_destroy, >> .ro_displayname = "frwr", >> }; >> diff --git a/net/sunrpc/xprtrdma/physical_ops.c >> b/net/sunrpc/xprtrdma/physical_ops.c >> index 0afc691..f8da8c4 100644 >> --- a/net/sunrpc/xprtrdma/physical_ops.c >> +++ b/net/sunrpc/xprtrdma/physical_ops.c >> @@ -67,11 +67,17 @@ physical_op_reset(struct rpcrdma_xprt *r_xprt) >> { >> } >> >> +static void >> +physical_op_destroy(struct rpcrdma_buffer *buf) >> +{ >> +} >> + >> 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_init = physical_op_init, >> .ro_reset = physical_op_reset, >> + .ro_destroy = physical_op_destroy, >> .ro_displayname = "physical", >> }; >> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c >> index e17d91a..dcbc736 100644 >> --- a/net/sunrpc/xprtrdma/verbs.c >> +++ b/net/sunrpc/xprtrdma/verbs.c >> @@ -1198,47 +1198,6 @@ rpcrdma_destroy_req(struct rpcrdma_ia *ia, struct >> rpcrdma_req *req) >> kfree(req); >> } >> >> -static void >> -rpcrdma_destroy_fmrs(struct rpcrdma_buffer *buf) >> -{ >> - struct rpcrdma_mw *r; >> - int rc; >> - >> - while (!list_empty(&buf->rb_all)) { >> - r = list_entry(buf->rb_all.next, struct rpcrdma_mw, mw_all); >> - list_del(&r->mw_all); >> - list_del(&r->mw_list); >> - >> - rc = ib_dealloc_fmr(r->r.fmr); >> - if (rc) >> - dprintk("RPC: %s: ib_dealloc_fmr failed %i\n", >> - __func__, rc); >> - >> - kfree(r); >> - } >> -} >> - >> -static void >> -rpcrdma_destroy_frmrs(struct rpcrdma_buffer *buf) >> -{ >> - struct rpcrdma_mw *r; >> - int rc; >> - >> - while (!list_empty(&buf->rb_all)) { >> - r = list_entry(buf->rb_all.next, struct rpcrdma_mw, mw_all); >> - list_del(&r->mw_all); >> - list_del(&r->mw_list); >> - >> - rc = ib_dereg_mr(r->r.frmr.fr_mr); >> - if (rc) >> - dprintk("RPC: %s: ib_dereg_mr failed %i\n", >> - __func__, rc); >> - ib_free_fast_reg_page_list(r->r.frmr.fr_pgl); >> - >> - kfree(r); >> - } >> -} >> - >> void >> rpcrdma_buffer_destroy(struct rpcrdma_buffer *buf) >> { >> @@ -1259,16 +1218,7 @@ rpcrdma_buffer_destroy(struct rpcrdma_buffer *buf) >> rpcrdma_destroy_req(ia, buf->rb_send_bufs[i]); >> } >> >> - switch (ia->ri_memreg_strategy) { >> - case RPCRDMA_FRMR: >> - rpcrdma_destroy_frmrs(buf); >> - break; >> - case RPCRDMA_MTHCAFMR: >> - rpcrdma_destroy_fmrs(buf); >> - break; >> - default: >> - break; >> - } >> + ia->ri_ops->ro_destroy(buf); >> >> kfree(buf->rb_pool); >> } >> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h >> b/net/sunrpc/xprtrdma/xprt_rdma.h >> index cdf6763..a0e3c3e 100644 >> --- a/net/sunrpc/xprtrdma/xprt_rdma.h >> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h >> @@ -343,6 +343,7 @@ struct rpcrdma_memreg_ops { >> size_t (*ro_maxpages)(struct rpcrdma_xprt *); >> int (*ro_init)(struct rpcrdma_xprt *); >> void (*ro_reset)(struct rpcrdma_xprt *); >> + void (*ro_destroy)(struct rpcrdma_buffer *); >> const char *ro_displayname; >> }; >> >> > > Other than that, > > Reviewed-by: Sagi Grimberg <[email protected]> -- Chuck Lever chuck[dot]lever[at]oracle[dot]com -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
