Skip to content

Commit 21fce0d

Browse files
chucklevergregkh
authored andcommitted
xprtrdma: Clear receive-side ownership pointers on release
[ Upstream commit 2ae8e7a ] Three small ownership-state cleanups land the transport in a state that lets future reviewers reason about each pointer locally rather than tracing the whole reply path: rpcrdma_rep_put() clears rep->rr_rqst before the rep enters rb_free_reps so that no rep on the free list still carries a stale rqst pointer. rpcrdma_reply_handler() and rpcrdma_unpin_rqst() are the only sites that set rr_rqst; rpcrdma_reply_handler() hands the rep through rpcrdma_rep_put(), and rpcrdma_unpin_rqst() NULLs rr_rqst directly because its error path abandons the rep for teardown cleanup rather than returning it to rb_free_reps. rpcrdma_reply_put() NULLs req->rl_reply before calling rpcrdma_rep_put(). The previous order placed the rep on rb_free_reps while req->rl_reply still pointed at it; the window was harmless because xprt_rdma_free_slot() holds the req exclusively across the pair, but closing it makes the invariant 'rep on rb_free_reps implies no req references it' strictly checkable. rpcrdma_sendctx_unmap() and rpcrdma_sendctx_cancel() clear req->rl_sendctx after dropping the sendctx pointer in the sendctx ring. Without this, req->rl_sendctx survives across Send completion and points at a sendctx that may already have been reassigned by rpcrdma_sendctx_get_locked() to a different req. No caller dereferences the stale pointer today -- rpcrdma_prepare_send_sges() overwrites it before the next Send -- but a NULL is a more honest representation of 'the Send is no longer outstanding' and lets the assertion patch that follows trip on any future regression. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent e472df3 commit 21fce0d

2 files changed

Lines changed: 14 additions & 2 deletions

File tree

net/sunrpc/xprtrdma/rpc_rdma.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,7 @@ void rpcrdma_sendctx_unmap(struct rpcrdma_sendctx *sc)
542542

543543
rpcrdma_sendctx_dma_unmap(sc);
544544
sc->sc_req = NULL;
545+
req->rl_sendctx = NULL;
545546
rpcrdma_req_put(req);
546547
}
547548

@@ -550,8 +551,11 @@ void rpcrdma_sendctx_unmap(struct rpcrdma_sendctx *sc)
550551
*/
551552
static void rpcrdma_sendctx_cancel(struct rpcrdma_sendctx *sc)
552553
{
554+
struct rpcrdma_req *req = sc->sc_req;
555+
553556
rpcrdma_sendctx_dma_unmap(sc);
554557
sc->sc_req = NULL;
558+
req->rl_sendctx = NULL;
555559
}
556560

557561
/* Prepare an SGE for the RPC-over-RDMA transport header.

net/sunrpc/xprtrdma/verbs.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,9 +1080,15 @@ static struct rpcrdma_rep *rpcrdma_rep_get_locked(struct rpcrdma_buffer *buf)
10801080
* @buf: buffer pool
10811081
* @rep: rep to release
10821082
*
1083+
* The rep's transient association with an rpc_rqst, established
1084+
* by rpcrdma_reply_handler() and torn down here, must not survive
1085+
* onto rb_free_reps: rpcrdma_post_recvs() pulls reps from the free
1086+
* list to re-post them, and a non-NULL rr_rqst on a free-listed rep
1087+
* would imply the rep is still referenced by a req.
10831088
*/
10841089
void rpcrdma_rep_put(struct rpcrdma_buffer *buf, struct rpcrdma_rep *rep)
10851090
{
1091+
rep->rr_rqst = NULL;
10861092
llist_add(&rep->rr_node, &buf->rb_free_reps);
10871093
}
10881094

@@ -1265,9 +1271,11 @@ rpcrdma_mr_get(struct rpcrdma_xprt *r_xprt)
12651271
*/
12661272
void rpcrdma_reply_put(struct rpcrdma_buffer *buffers, struct rpcrdma_req *req)
12671273
{
1268-
if (req->rl_reply) {
1269-
rpcrdma_rep_put(buffers, req->rl_reply);
1274+
struct rpcrdma_rep *rep = req->rl_reply;
1275+
1276+
if (rep) {
12701277
req->rl_reply = NULL;
1278+
rpcrdma_rep_put(buffers, rep);
12711279
}
12721280
/* I2: rl_reply NULL after the put closes the
12731281
* 'rep on rb_free_reps still referenced by req' window.

0 commit comments

Comments
 (0)