aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChuck Lever <chuck.lever@oracle.com>2018-10-01 14:25:09 -0400
committerAnna Schumaker <Anna.Schumaker@Netapp.com>2018-10-02 15:26:30 -0400
commit91ca18660e195df426522b29190940abb3010425 (patch)
tree15282ee417a2595e8cb463f6a7cbec4d133a5273
parent17b57b1883c1285f3d0dc2266e8f79286a7bef38 (diff)
xprtrdma: xprt_release_rqst_cong is called outside of transport_lock
Since commit ce7c252a8c74 ("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 e7ce710a8802 ("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: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
-rw-r--r--net/sunrpc/xprtrdma/rpc_rdma.c16
1 files changed, 7 insertions, 9 deletions
diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
index c8ae983c6cc0..293b3d3e3e65 100644
--- a/net/sunrpc/xprtrdma/rpc_rdma.c
+++ b/net/sunrpc/xprtrdma/rpc_rdma.c
@@ -1216,7 +1216,6 @@ void rpcrdma_complete_rqst(struct rpcrdma_rep *rep)
1216 struct rpcrdma_xprt *r_xprt = rep->rr_rxprt; 1216 struct rpcrdma_xprt *r_xprt = rep->rr_rxprt;
1217 struct rpc_xprt *xprt = &r_xprt->rx_xprt; 1217 struct rpc_xprt *xprt = &r_xprt->rx_xprt;
1218 struct rpc_rqst *rqst = rep->rr_rqst; 1218 struct rpc_rqst *rqst = rep->rr_rqst;
1219 unsigned long cwnd;
1220 int status; 1219 int status;
1221 1220
1222 xprt->reestablish_timeout = 0; 1221 xprt->reestablish_timeout = 0;
@@ -1239,11 +1238,6 @@ void rpcrdma_complete_rqst(struct rpcrdma_rep *rep)
1239 1238
1240out: 1239out:
1241 spin_lock(&xprt->recv_lock); 1240 spin_lock(&xprt->recv_lock);
1242 cwnd = xprt->cwnd;
1243 xprt->cwnd = r_xprt->rx_buf.rb_credits << RPC_CWNDSHIFT;
1244 if (xprt->cwnd > cwnd)
1245 xprt_release_rqst_cong(rqst->rq_task);
1246
1247 xprt_complete_rqst(rqst->rq_task, status); 1241 xprt_complete_rqst(rqst->rq_task, status);
1248 xprt_unpin_rqst(rqst); 1242 xprt_unpin_rqst(rqst);
1249 spin_unlock(&xprt->recv_lock); 1243 spin_unlock(&xprt->recv_lock);
@@ -1350,14 +1344,18 @@ void rpcrdma_reply_handler(struct rpcrdma_rep *rep)
1350 if (!rqst) 1344 if (!rqst)
1351 goto out_norqst; 1345 goto out_norqst;
1352 xprt_pin_rqst(rqst); 1346 xprt_pin_rqst(rqst);
1347 spin_unlock(&xprt->recv_lock);
1353 1348
1354 if (credits == 0) 1349 if (credits == 0)
1355 credits = 1; /* don't deadlock */ 1350 credits = 1; /* don't deadlock */
1356 else if (credits > buf->rb_max_requests) 1351 else if (credits > buf->rb_max_requests)
1357 credits = buf->rb_max_requests; 1352 credits = buf->rb_max_requests;
1358 buf->rb_credits = credits; 1353 if (buf->rb_credits != credits) {
1359 1354 spin_lock_bh(&xprt->transport_lock);
1360 spin_unlock(&xprt->recv_lock); 1355 buf->rb_credits = credits;
1356 xprt->cwnd = credits << RPC_CWNDSHIFT;
1357 spin_unlock_bh(&xprt->transport_lock);
1358 }
1361 1359
1362 req = rpcr_to_rdmar(rqst); 1360 req = rpcr_to_rdmar(rqst);
1363 req->rl_reply = rep; 1361 req->rl_reply = rep;