On Mar 16, 2015, at 3:46 AM, Sagi Grimberg <sagig@xxxxxxxxxxxxxxxxxx> 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 <chuck.lever@xxxxxxxxxx>
>> ---
>> 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 <sagig@xxxxxxxxxxxx>
--
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
