Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v1 3/8] xprtrdma: Put flushed Receives on free list
 instead of destroying them
To:     Chuck Lever <chuck.lever@oracle.com>, linux-rdma@vger.kernel.org,
        linux-nfs@vger.kernel.org
References: <161721926778.515226.9805598788670386587.stgit@manet.1015granger.net>
 <161721937732.515226.2170674299158077377.stgit@manet.1015granger.net>
From:   Tom Talpey <tom@talpey.com>
Message-ID: <8dc8bdd3-b006-46ed-cc9f-75fae4df3f40@talpey.com>
Date:   Wed, 31 Mar 2021 16:02:43 -0400
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101
 Thunderbird/68.12.1
MIME-Version: 1.0
In-Reply-To: <161721937732.515226.2170674299158077377.stgit@manet.1015granger.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/31/2021 3:36 PM, Chuck Lever wrote:
> Defer destruction of an rpcrdma_rep until transport tear-down to
> avoid races between Receive completion and rpcrdma_reps_unmap().

This seems sketchy, but it's a good approach to destroy in one
place, and off the hot path. You might restate the description
in a positive way, not strictly to avoid a race.

Reviewed-By: Tom Talpey <tom@talpey.com>

> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>   net/sunrpc/xprtrdma/verbs.c |    4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index 1d88685badbe..92af272f9cc9 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -80,6 +80,8 @@ static void rpcrdma_sendctx_put_locked(struct rpcrdma_xprt *r_xprt,
>   				       struct rpcrdma_sendctx *sc);
>   static int rpcrdma_reqs_setup(struct rpcrdma_xprt *r_xprt);
>   static void rpcrdma_reqs_reset(struct rpcrdma_xprt *r_xprt);
> +static void rpcrdma_rep_put(struct rpcrdma_buffer *buf,
> +			    struct rpcrdma_rep *rep);
>   static void rpcrdma_rep_destroy(struct rpcrdma_rep *rep);
>   static void rpcrdma_reps_unmap(struct rpcrdma_xprt *r_xprt);
>   static void rpcrdma_mrs_create(struct rpcrdma_xprt *r_xprt);
> @@ -205,7 +207,7 @@ static void rpcrdma_wc_receive(struct ib_cq *cq, struct ib_wc *wc)
>   
>   out_flushed:
>   	rpcrdma_flush_disconnect(r_xprt, wc);
> -	rpcrdma_rep_destroy(rep);
> +	rpcrdma_rep_put(&r_xprt->rx_buf, rep);
>   }
>   
>   static void rpcrdma_update_cm_private(struct rpcrdma_ep *ep,
> 
> 
>