Reviewed-by: Devesh Sharma <devesh.sharma@xxxxxxxxxxxxx>
On Thu, May 7, 2015 at 4:06 PM, Sagi Grimberg <sagig@xxxxxxxxxxxxxxxxxx> wrote:
> On 5/4/2015 8:58 PM, Chuck Lever wrote:
>>
>> An RPC can exit at any time. When it does so, xprt_rdma_free() is
>> called, and it calls ->op_unmap().
>>
>> If ->ro_reset() is running due to a transport disconnect, the two
>> methods can race while processing the same rpcrdma_mw. The results
>> are unpredictable.
>>
>> Because of this, in previous patches I've replaced the ->ro_reset()
>> methods with a recovery workqueue. ->ro_reset() is no longer used
>> and can be removed.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> net/sunrpc/xprtrdma/fmr_ops.c | 11 -----------
>> net/sunrpc/xprtrdma/frwr_ops.c | 16 ----------------
>> net/sunrpc/xprtrdma/physical_ops.c | 6 ------
>> net/sunrpc/xprtrdma/verbs.c | 2 --
>> net/sunrpc/xprtrdma/xprt_rdma.h | 1 -
>> 5 files changed, 36 deletions(-)
>>
>> diff --git a/net/sunrpc/xprtrdma/fmr_ops.c b/net/sunrpc/xprtrdma/fmr_ops.c
>> index ad0055b..5dd77da 100644
>> --- a/net/sunrpc/xprtrdma/fmr_ops.c
>> +++ b/net/sunrpc/xprtrdma/fmr_ops.c
>> @@ -197,16 +197,6 @@ out_err:
>> return nsegs;
>> }
>>
>> -/* After a disconnect, unmap all FMRs.
>> - *
>> - * This is invoked only in the transport connect worker in order
>> - * to serialize with rpcrdma_register_fmr_external().
>> - */
>> -static void
>> -fmr_op_reset(struct rpcrdma_xprt *r_xprt)
>> -{
>> -}
>> -
>> static void
>> fmr_op_destroy(struct rpcrdma_buffer *buf)
>> {
>> @@ -230,7 +220,6 @@ const struct rpcrdma_memreg_ops rpcrdma_fmr_memreg_ops
>> = {
>> .ro_open = fmr_op_open,
>> .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 6f93a89..3fb609a 100644
>> --- a/net/sunrpc/xprtrdma/frwr_ops.c
>> +++ b/net/sunrpc/xprtrdma/frwr_ops.c
>> @@ -430,21 +430,6 @@ out_err:
>> return nsegs;
>> }
>>
>> -/* After a disconnect, a flushed FAST_REG_MR can leave an FRMR in
>> - * an unusable state. Find FRMRs in this state and dereg / reg
>> - * each. FRMRs that are VALID and attached to an rpcrdma_req are
>> - * also torn down.
>> - *
>> - * This gives all in-use FRMRs a fresh rkey and leaves them INVALID.
>> - *
>> - * This is invoked only in the transport connect worker in order
>> - * to serialize with rpcrdma_register_frmr_external().
>> - */
>> -static void
>> -frwr_op_reset(struct rpcrdma_xprt *r_xprt)
>> -{
>> -}
>> -
>> static void
>> frwr_op_destroy(struct rpcrdma_buffer *buf)
>> {
>> @@ -464,7 +449,6 @@ const struct rpcrdma_memreg_ops
>> rpcrdma_frwr_memreg_ops = {
>> .ro_open = frwr_op_open,
>> .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 da149e8..41985d0 100644
>> --- a/net/sunrpc/xprtrdma/physical_ops.c
>> +++ b/net/sunrpc/xprtrdma/physical_ops.c
>> @@ -69,11 +69,6 @@ physical_op_unmap(struct rpcrdma_xprt *r_xprt, struct
>> rpcrdma_mr_seg *seg)
>> }
>>
>> static void
>> -physical_op_reset(struct rpcrdma_xprt *r_xprt)
>> -{
>> -}
>> -
>> -static void
>> physical_op_destroy(struct rpcrdma_buffer *buf)
>> {
>> }
>> @@ -84,7 +79,6 @@ const struct rpcrdma_memreg_ops
>> rpcrdma_physical_memreg_ops = {
>> .ro_open = physical_op_open,
>> .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 5120a8e..eaf0b9d 100644
>> --- a/net/sunrpc/xprtrdma/verbs.c
>> +++ b/net/sunrpc/xprtrdma/verbs.c
>> @@ -897,8 +897,6 @@ retry:
>> rpcrdma_flush_cqs(ep);
>>
>> xprt = container_of(ia, struct rpcrdma_xprt, rx_ia);
>> - ia->ri_ops->ro_reset(xprt);
>> -
>> id = rpcrdma_create_id(xprt, ia,
>> (struct sockaddr *)&xprt->rx_data.addr);
>> if (IS_ERR(id)) {
>> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h
>> b/net/sunrpc/xprtrdma/xprt_rdma.h
>> index 98227d6..6a1e565 100644
>> --- a/net/sunrpc/xprtrdma/xprt_rdma.h
>> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h
>> @@ -353,7 +353,6 @@ struct rpcrdma_memreg_ops {
>> struct rpcrdma_create_data_internal *);
>> 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;
>> };
>>
>
> Looks good,
>
> Reviewed-by: Sagi Grimberg <sagig@xxxxxxxxxxxx>
>
> --
> 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
--
-Regards
Devesh
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
