Commit 91ca1866 authored by Chuck Lever's avatar Chuck Lever Committed by Anna Schumaker

xprtrdma: xprt_release_rqst_cong is called outside of transport_lock

Since commit ce7c252a ("SUNRPC: Add a separate spinlock to
protect the RPC request receive list") the RPC/RDMA reply handler
has been calling xprt_release_rqst_cong without holding
xprt->transport_lock.

I think the only way this call is ever made is if the credit grant
increases and there are RPCs pending. Current server implementations
do not change their credit grant during operation (except at
connect time).

Commit e7ce710a ("xprtrdma: Avoid deadlock when credit window is
reset") added the ->release_rqst call because UDP invokes
xprt_adjust_cwnd(), which calls __xprt_put_cong() after adjusting
xprt->cwnd. Both xprt_release() and ->xprt_release_xprt already wake
another task in this case, so it is safe to remove this call from
the reply handler.
Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
Signed-off-by: default avatarAnna Schumaker <Anna.Schumaker@Netapp.com>
parent 17b57b18
...@@ -1216,7 +1216,6 @@ void rpcrdma_complete_rqst(struct rpcrdma_rep *rep) ...@@ -1216,7 +1216,6 @@ void rpcrdma_complete_rqst(struct rpcrdma_rep *rep)
struct rpcrdma_xprt *r_xprt = rep->rr_rxprt; struct rpcrdma_xprt *r_xprt = rep->rr_rxprt;
struct rpc_xprt *xprt = &r_xprt->rx_xprt; struct rpc_xprt *xprt = &r_xprt->rx_xprt;
struct rpc_rqst *rqst = rep->rr_rqst; struct rpc_rqst *rqst = rep->rr_rqst;
unsigned long cwnd;
int status; int status;
xprt->reestablish_timeout = 0; xprt->reestablish_timeout = 0;
...@@ -1239,11 +1238,6 @@ void rpcrdma_complete_rqst(struct rpcrdma_rep *rep) ...@@ -1239,11 +1238,6 @@ void rpcrdma_complete_rqst(struct rpcrdma_rep *rep)
out: out:
spin_lock(&xprt->recv_lock); spin_lock(&xprt->recv_lock);
cwnd = xprt->cwnd;
xprt->cwnd = r_xprt->rx_buf.rb_credits << RPC_CWNDSHIFT;
if (xprt->cwnd > cwnd)
xprt_release_rqst_cong(rqst->rq_task);
xprt_complete_rqst(rqst->rq_task, status); xprt_complete_rqst(rqst->rq_task, status);
xprt_unpin_rqst(rqst); xprt_unpin_rqst(rqst);
spin_unlock(&xprt->recv_lock); spin_unlock(&xprt->recv_lock);
...@@ -1350,14 +1344,18 @@ void rpcrdma_reply_handler(struct rpcrdma_rep *rep) ...@@ -1350,14 +1344,18 @@ void rpcrdma_reply_handler(struct rpcrdma_rep *rep)
if (!rqst) if (!rqst)
goto out_norqst; goto out_norqst;
xprt_pin_rqst(rqst); xprt_pin_rqst(rqst);
spin_unlock(&xprt->recv_lock);
if (credits == 0) if (credits == 0)
credits = 1; /* don't deadlock */ credits = 1; /* don't deadlock */
else if (credits > buf->rb_max_requests) else if (credits > buf->rb_max_requests)
credits = buf->rb_max_requests; credits = buf->rb_max_requests;
buf->rb_credits = credits; if (buf->rb_credits != credits) {
spin_lock_bh(&xprt->transport_lock);
spin_unlock(&xprt->recv_lock); buf->rb_credits = credits;
xprt->cwnd = credits << RPC_CWNDSHIFT;
spin_unlock_bh(&xprt->transport_lock);
}
req = rpcr_to_rdmar(rqst); req = rpcr_to_rdmar(rqst);
req->rl_reply = rep; req->rl_reply = rep;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment