Since the ->xprt_ctxt pointer was added to svc_deferred_req, it has not
been sufficient to use kfree() to free a deferred request. We may need
to free the ctxt as well.
As freeing the ctxt is all that ->xpo_release_rqst() does, we repurpose
it to explicit do that even when the ctxt is not stored in an rqst.
So we now have ->xpo_release_ctxt() which is given an xprt and a ctxt,
which may have been taken either from an rqst or from a dreq. The
caller is now responsible for clearing that pointer after the call to
->xpo_release_ctxt.
We also clear dr->xprt_ctxt when the ctxt is moved into a new rqst when
revisiting a deferred request. This ensures there is only one pointer
to the ctxt, so the risk of double freeing in future is reduced. The
new code in svc_xprt_release which releases both the ctxt and any
rq_deferred depends on this.
Fixes: 773f91b2cf3f ("SUNRPC: Fix NFSD's request deferral on RDMA transports")
Signed-off-by: NeilBrown <[email protected]>
---
include/linux/sunrpc/svc_rdma.h | 2 +-
include/linux/sunrpc/svc_xprt.h | 2 +-
net/sunrpc/svc_xprt.c | 21 ++++++++++++-----
net/sunrpc/svcsock.c | 30 +++++++++++++-----------
net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 11 ++++-----
net/sunrpc/xprtrdma/svc_rdma_transport.c | 2 +-
6 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
index 24aa159d29a7..fbc4bd423b35 100644
--- a/include/linux/sunrpc/svc_rdma.h
+++ b/include/linux/sunrpc/svc_rdma.h
@@ -176,7 +176,7 @@ extern struct svc_rdma_recv_ctxt *
extern void svc_rdma_recv_ctxt_put(struct svcxprt_rdma *rdma,
struct svc_rdma_recv_ctxt *ctxt);
extern void svc_rdma_flush_recv_queues(struct svcxprt_rdma *rdma);
-extern void svc_rdma_release_rqst(struct svc_rqst *rqstp);
+extern void svc_rdma_release_ctxt(struct svc_xprt *xprt, void *ctxt);
extern int svc_rdma_recvfrom(struct svc_rqst *);
/* svc_rdma_rw.c */
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 867479204840..6f4473ee68e1 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -23,7 +23,7 @@ struct svc_xprt_ops {
int (*xpo_sendto)(struct svc_rqst *);
int (*xpo_result_payload)(struct svc_rqst *, unsigned int,
unsigned int);
- void (*xpo_release_rqst)(struct svc_rqst *);
+ void (*xpo_release_ctxt)(struct svc_xprt *, void *);
void (*xpo_detach)(struct svc_xprt *);
void (*xpo_free)(struct svc_xprt *);
void (*xpo_kill_temp_xprt)(struct svc_xprt *);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 5fd94f6bdc75..1e3bba433561 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -532,13 +532,21 @@ void svc_reserve(struct svc_rqst *rqstp, int space)
}
EXPORT_SYMBOL_GPL(svc_reserve);
+static void free_deferred(struct svc_xprt *xprt, struct svc_deferred_req *dr)
+{
+ if (dr)
+ xprt->xpt_ops->xpo_release_ctxt(xprt, dr->xprt_ctxt);
+ kfree(dr);
+}
+
static void svc_xprt_release(struct svc_rqst *rqstp)
{
struct svc_xprt *xprt = rqstp->rq_xprt;
- xprt->xpt_ops->xpo_release_rqst(rqstp);
+ xprt->xpt_ops->xpo_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
+ rqstp->rq_xprt_ctxt = NULL;
- kfree(rqstp->rq_deferred);
+ free_deferred(xprt, rqstp->rq_deferred);
rqstp->rq_deferred = NULL;
svc_rqst_release_pages(rqstp);
@@ -1054,7 +1062,7 @@ static void svc_delete_xprt(struct svc_xprt *xprt)
spin_unlock_bh(&serv->sv_lock);
while ((dr = svc_deferred_dequeue(xprt)) != NULL)
- kfree(dr);
+ free_deferred(xprt, dr);
call_xpt_users(xprt);
svc_xprt_put(xprt);
@@ -1176,8 +1184,8 @@ static void svc_revisit(struct cache_deferred_req *dreq, int too_many)
if (too_many || test_bit(XPT_DEAD, &xprt->xpt_flags)) {
spin_unlock(&xprt->xpt_lock);
trace_svc_defer_drop(dr);
+ free_deferred(xprt, dr);
svc_xprt_put(xprt);
- kfree(dr);
return;
}
dr->xprt = NULL;
@@ -1222,14 +1230,13 @@ static struct cache_deferred_req *svc_defer(struct cache_req *req)
dr->addrlen = rqstp->rq_addrlen;
dr->daddr = rqstp->rq_daddr;
dr->argslen = rqstp->rq_arg.len >> 2;
- dr->xprt_ctxt = rqstp->rq_xprt_ctxt;
/* back up head to the start of the buffer and copy */
skip = rqstp->rq_arg.len - rqstp->rq_arg.head[0].iov_len;
memcpy(dr->args, rqstp->rq_arg.head[0].iov_base - skip,
dr->argslen << 2);
}
- WARN_ON_ONCE(rqstp->rq_xprt_ctxt != dr->xprt_ctxt);
+ dr->xprt_ctxt = rqstp->rq_xprt_ctxt;
rqstp->rq_xprt_ctxt = NULL;
trace_svc_defer(rqstp);
svc_xprt_get(rqstp->rq_xprt);
@@ -1263,6 +1270,8 @@ static noinline int svc_deferred_recv(struct svc_rqst *rqstp)
rqstp->rq_daddr = dr->daddr;
rqstp->rq_respages = rqstp->rq_pages;
rqstp->rq_xprt_ctxt = dr->xprt_ctxt;
+
+ dr->xprt_ctxt = NULL;
svc_xprt_received(rqstp->rq_xprt);
return dr->argslen << 2;
}
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index a51c9b989d58..aa4f31a770e3 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -121,27 +121,27 @@ static void svc_reclassify_socket(struct socket *sock)
#endif
/**
- * svc_tcp_release_rqst - Release transport-related resources
- * @rqstp: request structure with resources to be released
+ * svc_tcp_release_ctxt - Release transport-related resources
+ * @xprt: the transport which owned the context
+ * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
*
*/
-static void svc_tcp_release_rqst(struct svc_rqst *rqstp)
+static void svc_tcp_release_ctxt(struct svc_xprt *xprt, void *ctxt)
{
}
/**
- * svc_udp_release_rqst - Release transport-related resources
- * @rqstp: request structure with resources to be released
+ * svc_udp_release_ctxt - Release transport-related resources
+ * @xprt: the transport which owned the context
+ * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
*
*/
-static void svc_udp_release_rqst(struct svc_rqst *rqstp)
+static void svc_udp_release_ctxt(struct svc_xprt *xprt, void *ctxt)
{
- struct sk_buff *skb = rqstp->rq_xprt_ctxt;
+ struct sk_buff *skb = ctxt;
- if (skb) {
- rqstp->rq_xprt_ctxt = NULL;
+ if (skb)
consume_skb(skb);
- }
}
union svc_pktinfo_u {
@@ -696,7 +696,8 @@ static int svc_udp_sendto(struct svc_rqst *rqstp)
unsigned int sent;
int err;
- svc_udp_release_rqst(rqstp);
+ svc_udp_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
+ rqstp->rq_xprt_ctxt = NULL;
svc_set_cmsg_data(rqstp, cmh);
@@ -768,7 +769,7 @@ static const struct svc_xprt_ops svc_udp_ops = {
.xpo_recvfrom = svc_udp_recvfrom,
.xpo_sendto = svc_udp_sendto,
.xpo_result_payload = svc_sock_result_payload,
- .xpo_release_rqst = svc_udp_release_rqst,
+ .xpo_release_ctxt = svc_udp_release_ctxt,
.xpo_detach = svc_sock_detach,
.xpo_free = svc_sock_free,
.xpo_has_wspace = svc_udp_has_wspace,
@@ -1298,7 +1299,8 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp)
unsigned int sent;
int err;
- svc_tcp_release_rqst(rqstp);
+ svc_tcp_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
+ rqstp->rq_xprt_ctxt = NULL;
atomic_inc(&svsk->sk_sendqlen);
mutex_lock(&xprt->xpt_mutex);
@@ -1343,7 +1345,7 @@ static const struct svc_xprt_ops svc_tcp_ops = {
.xpo_recvfrom = svc_tcp_recvfrom,
.xpo_sendto = svc_tcp_sendto,
.xpo_result_payload = svc_sock_result_payload,
- .xpo_release_rqst = svc_tcp_release_rqst,
+ .xpo_release_ctxt = svc_tcp_release_ctxt,
.xpo_detach = svc_tcp_sock_detach,
.xpo_free = svc_sock_free,
.xpo_has_wspace = svc_tcp_has_wspace,
diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
index 1c658fa43063..5c51e28b3111 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
@@ -239,21 +239,20 @@ void svc_rdma_recv_ctxt_put(struct svcxprt_rdma *rdma,
}
/**
- * svc_rdma_release_rqst - Release transport-specific per-rqst resources
- * @rqstp: svc_rqst being released
+ * svc_rdma_release_ctxt - Release transport-specific per-rqst resources
+ * @xprt: the transport which owned the context
+ * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
*
* Ensure that the recv_ctxt is released whether or not a Reply
* was sent. For example, the client could close the connection,
* or svc_process could drop an RPC, before the Reply is sent.
*/
-void svc_rdma_release_rqst(struct svc_rqst *rqstp)
+void svc_rdma_release_ctxt(struct svc_xprt *xprt, void *vctxt)
{
- struct svc_rdma_recv_ctxt *ctxt = rqstp->rq_xprt_ctxt;
- struct svc_xprt *xprt = rqstp->rq_xprt;
+ struct svc_rdma_recv_ctxt *ctxt = vctxt;
struct svcxprt_rdma *rdma =
container_of(xprt, struct svcxprt_rdma, sc_xprt);
- rqstp->rq_xprt_ctxt = NULL;
if (ctxt)
svc_rdma_recv_ctxt_put(rdma, ctxt);
}
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index 416b298f74dd..ca04f7a6a085 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -80,7 +80,7 @@ static const struct svc_xprt_ops svc_rdma_ops = {
.xpo_recvfrom = svc_rdma_recvfrom,
.xpo_sendto = svc_rdma_sendto,
.xpo_result_payload = svc_rdma_result_payload,
- .xpo_release_rqst = svc_rdma_release_rqst,
+ .xpo_release_ctxt = svc_rdma_release_ctxt,
.xpo_detach = svc_rdma_detach,
.xpo_free = svc_rdma_free,
.xpo_has_wspace = svc_rdma_has_wspace,
--
2.40.1
On Tue, 2023-05-09 at 09:42 +1000, NeilBrown wrote:
> Since the ->xprt_ctxt pointer was added to svc_deferred_req, it has not
> been sufficient to use kfree() to free a deferred request. We may need
> to free the ctxt as well.
>
> As freeing the ctxt is all that ->xpo_release_rqst() does, we repurpose
> it to explicit do that even when the ctxt is not stored in an rqst.
> So we now have ->xpo_release_ctxt() which is given an xprt and a ctxt,
> which may have been taken either from an rqst or from a dreq. The
> caller is now responsible for clearing that pointer after the call to
> ->xpo_release_ctxt.
>
> We also clear dr->xprt_ctxt when the ctxt is moved into a new rqst when
> revisiting a deferred request. This ensures there is only one pointer
> to the ctxt, so the risk of double freeing in future is reduced. The
> new code in svc_xprt_release which releases both the ctxt and any
> rq_deferred depends on this.
>
Thank you. Leaving stray pointers around like that is just asking for
trouble.
> Fixes: 773f91b2cf3f ("SUNRPC: Fix NFSD's request deferral on RDMA transports")
> Signed-off-by: NeilBrown <[email protected]>
> ---
> include/linux/sunrpc/svc_rdma.h | 2 +-
> include/linux/sunrpc/svc_xprt.h | 2 +-
> net/sunrpc/svc_xprt.c | 21 ++++++++++++-----
> net/sunrpc/svcsock.c | 30 +++++++++++++-----------
> net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 11 ++++-----
> net/sunrpc/xprtrdma/svc_rdma_transport.c | 2 +-
> 6 files changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
> index 24aa159d29a7..fbc4bd423b35 100644
> --- a/include/linux/sunrpc/svc_rdma.h
> +++ b/include/linux/sunrpc/svc_rdma.h
> @@ -176,7 +176,7 @@ extern struct svc_rdma_recv_ctxt *
> extern void svc_rdma_recv_ctxt_put(struct svcxprt_rdma *rdma,
> struct svc_rdma_recv_ctxt *ctxt);
> extern void svc_rdma_flush_recv_queues(struct svcxprt_rdma *rdma);
> -extern void svc_rdma_release_rqst(struct svc_rqst *rqstp);
> +extern void svc_rdma_release_ctxt(struct svc_xprt *xprt, void *ctxt);
> extern int svc_rdma_recvfrom(struct svc_rqst *);
>
> /* svc_rdma_rw.c */
> diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
> index 867479204840..6f4473ee68e1 100644
> --- a/include/linux/sunrpc/svc_xprt.h
> +++ b/include/linux/sunrpc/svc_xprt.h
> @@ -23,7 +23,7 @@ struct svc_xprt_ops {
> int (*xpo_sendto)(struct svc_rqst *);
> int (*xpo_result_payload)(struct svc_rqst *, unsigned int,
> unsigned int);
> - void (*xpo_release_rqst)(struct svc_rqst *);
> + void (*xpo_release_ctxt)(struct svc_xprt *, void *);
> void (*xpo_detach)(struct svc_xprt *);
> void (*xpo_free)(struct svc_xprt *);
> void (*xpo_kill_temp_xprt)(struct svc_xprt *);
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 5fd94f6bdc75..1e3bba433561 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -532,13 +532,21 @@ void svc_reserve(struct svc_rqst *rqstp, int space)
> }
> EXPORT_SYMBOL_GPL(svc_reserve);
>
> +static void free_deferred(struct svc_xprt *xprt, struct svc_deferred_req *dr)
> +{
> + if (dr)
> + xprt->xpt_ops->xpo_release_ctxt(xprt, dr->xprt_ctxt);
> + kfree(dr);
nit: might as well put the kfree inside the if block to avoid it in the
common case of dr == NULL.
> +}
> +
> static void svc_xprt_release(struct svc_rqst *rqstp)
> {
> struct svc_xprt *xprt = rqstp->rq_xprt;
>
> - xprt->xpt_ops->xpo_release_rqst(rqstp);
> + xprt->xpt_ops->xpo_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
> + rqstp->rq_xprt_ctxt = NULL;
>
> - kfree(rqstp->rq_deferred);
> + free_deferred(xprt, rqstp->rq_deferred);
> rqstp->rq_deferred = NULL;
>
> svc_rqst_release_pages(rqstp);
> @@ -1054,7 +1062,7 @@ static void svc_delete_xprt(struct svc_xprt *xprt)
> spin_unlock_bh(&serv->sv_lock);
>
> while ((dr = svc_deferred_dequeue(xprt)) != NULL)
> - kfree(dr);
> + free_deferred(xprt, dr);
>
> call_xpt_users(xprt);
> svc_xprt_put(xprt);
> @@ -1176,8 +1184,8 @@ static void svc_revisit(struct cache_deferred_req *dreq, int too_many)
> if (too_many || test_bit(XPT_DEAD, &xprt->xpt_flags)) {
> spin_unlock(&xprt->xpt_lock);
> trace_svc_defer_drop(dr);
> + free_deferred(xprt, dr);
> svc_xprt_put(xprt);
> - kfree(dr);
> return;
> }
> dr->xprt = NULL;
> @@ -1222,14 +1230,13 @@ static struct cache_deferred_req *svc_defer(struct cache_req *req)
> dr->addrlen = rqstp->rq_addrlen;
> dr->daddr = rqstp->rq_daddr;
> dr->argslen = rqstp->rq_arg.len >> 2;
> - dr->xprt_ctxt = rqstp->rq_xprt_ctxt;
>
> /* back up head to the start of the buffer and copy */
> skip = rqstp->rq_arg.len - rqstp->rq_arg.head[0].iov_len;
> memcpy(dr->args, rqstp->rq_arg.head[0].iov_base - skip,
> dr->argslen << 2);
> }
> - WARN_ON_ONCE(rqstp->rq_xprt_ctxt != dr->xprt_ctxt);
> + dr->xprt_ctxt = rqstp->rq_xprt_ctxt;
> rqstp->rq_xprt_ctxt = NULL;
> trace_svc_defer(rqstp);
> svc_xprt_get(rqstp->rq_xprt);
> @@ -1263,6 +1270,8 @@ static noinline int svc_deferred_recv(struct svc_rqst *rqstp)
> rqstp->rq_daddr = dr->daddr;
> rqstp->rq_respages = rqstp->rq_pages;
> rqstp->rq_xprt_ctxt = dr->xprt_ctxt;
> +
> + dr->xprt_ctxt = NULL;
> svc_xprt_received(rqstp->rq_xprt);
> return dr->argslen << 2;
> }
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index a51c9b989d58..aa4f31a770e3 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -121,27 +121,27 @@ static void svc_reclassify_socket(struct socket *sock)
> #endif
>
> /**
> - * svc_tcp_release_rqst - Release transport-related resources
> - * @rqstp: request structure with resources to be released
> + * svc_tcp_release_ctxt - Release transport-related resources
> + * @xprt: the transport which owned the context
> + * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
> *
> */
> -static void svc_tcp_release_rqst(struct svc_rqst *rqstp)
> +static void svc_tcp_release_ctxt(struct svc_xprt *xprt, void *ctxt)
> {
> }
>
> /**
> - * svc_udp_release_rqst - Release transport-related resources
> - * @rqstp: request structure with resources to be released
> + * svc_udp_release_ctxt - Release transport-related resources
> + * @xprt: the transport which owned the context
> + * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
> *
> */
> -static void svc_udp_release_rqst(struct svc_rqst *rqstp)
> +static void svc_udp_release_ctxt(struct svc_xprt *xprt, void *ctxt)
> {
> - struct sk_buff *skb = rqstp->rq_xprt_ctxt;
> + struct sk_buff *skb = ctxt;
>
> - if (skb) {
> - rqstp->rq_xprt_ctxt = NULL;
> + if (skb)
> consume_skb(skb);
> - }
> }
>
> union svc_pktinfo_u {
> @@ -696,7 +696,8 @@ static int svc_udp_sendto(struct svc_rqst *rqstp)
> unsigned int sent;
> int err;
>
> - svc_udp_release_rqst(rqstp);
> + svc_udp_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
> + rqstp->rq_xprt_ctxt = NULL;
>
> svc_set_cmsg_data(rqstp, cmh);
>
> @@ -768,7 +769,7 @@ static const struct svc_xprt_ops svc_udp_ops = {
> .xpo_recvfrom = svc_udp_recvfrom,
> .xpo_sendto = svc_udp_sendto,
> .xpo_result_payload = svc_sock_result_payload,
> - .xpo_release_rqst = svc_udp_release_rqst,
> + .xpo_release_ctxt = svc_udp_release_ctxt,
> .xpo_detach = svc_sock_detach,
> .xpo_free = svc_sock_free,
> .xpo_has_wspace = svc_udp_has_wspace,
> @@ -1298,7 +1299,8 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp)
> unsigned int sent;
> int err;
>
> - svc_tcp_release_rqst(rqstp);
> + svc_tcp_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
> + rqstp->rq_xprt_ctxt = NULL;
>
> atomic_inc(&svsk->sk_sendqlen);
> mutex_lock(&xprt->xpt_mutex);
> @@ -1343,7 +1345,7 @@ static const struct svc_xprt_ops svc_tcp_ops = {
> .xpo_recvfrom = svc_tcp_recvfrom,
> .xpo_sendto = svc_tcp_sendto,
> .xpo_result_payload = svc_sock_result_payload,
> - .xpo_release_rqst = svc_tcp_release_rqst,
> + .xpo_release_ctxt = svc_tcp_release_ctxt,
> .xpo_detach = svc_tcp_sock_detach,
> .xpo_free = svc_sock_free,
> .xpo_has_wspace = svc_tcp_has_wspace,
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> index 1c658fa43063..5c51e28b3111 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> @@ -239,21 +239,20 @@ void svc_rdma_recv_ctxt_put(struct svcxprt_rdma *rdma,
> }
>
> /**
> - * svc_rdma_release_rqst - Release transport-specific per-rqst resources
> - * @rqstp: svc_rqst being released
> + * svc_rdma_release_ctxt - Release transport-specific per-rqst resources
> + * @xprt: the transport which owned the context
> + * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
> *
> * Ensure that the recv_ctxt is released whether or not a Reply
> * was sent. For example, the client could close the connection,
> * or svc_process could drop an RPC, before the Reply is sent.
> */
> -void svc_rdma_release_rqst(struct svc_rqst *rqstp)
> +void svc_rdma_release_ctxt(struct svc_xprt *xprt, void *vctxt)
> {
> - struct svc_rdma_recv_ctxt *ctxt = rqstp->rq_xprt_ctxt;
> - struct svc_xprt *xprt = rqstp->rq_xprt;
> + struct svc_rdma_recv_ctxt *ctxt = vctxt;
> struct svcxprt_rdma *rdma =
> container_of(xprt, struct svcxprt_rdma, sc_xprt);
>
> - rqstp->rq_xprt_ctxt = NULL;
> if (ctxt)
> svc_rdma_recv_ctxt_put(rdma, ctxt);
> }
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> index 416b298f74dd..ca04f7a6a085 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> @@ -80,7 +80,7 @@ static const struct svc_xprt_ops svc_rdma_ops = {
> .xpo_recvfrom = svc_rdma_recvfrom,
> .xpo_sendto = svc_rdma_sendto,
> .xpo_result_payload = svc_rdma_result_payload,
> - .xpo_release_rqst = svc_rdma_release_rqst,
> + .xpo_release_ctxt = svc_rdma_release_ctxt,
> .xpo_detach = svc_rdma_detach,
> .xpo_free = svc_rdma_free,
> .xpo_has_wspace = svc_rdma_has_wspace,
Reviewed-by: Jeff Layton <[email protected]>
> On May 8, 2023, at 4:42 PM, NeilBrown <[email protected]> wrote:
>
>
> Since the ->xprt_ctxt pointer was added to svc_deferred_req, it has not
> been sufficient to use kfree() to free a deferred request. We may need
> to free the ctxt as well.
>
> As freeing the ctxt is all that ->xpo_release_rqst() does, we repurpose
> it to explicit do that even when the ctxt is not stored in an rqst.
> So we now have ->xpo_release_ctxt() which is given an xprt and a ctxt,
> which may have been taken either from an rqst or from a dreq. The
> caller is now responsible for clearing that pointer after the call to
> ->xpo_release_ctxt.
>
> We also clear dr->xprt_ctxt when the ctxt is moved into a new rqst when
> revisiting a deferred request. This ensures there is only one pointer
> to the ctxt, so the risk of double freeing in future is reduced. The
> new code in svc_xprt_release which releases both the ctxt and any
> rq_deferred depends on this.
>
> Fixes: 773f91b2cf3f ("SUNRPC: Fix NFSD's request deferral on RDMA transports")
> Signed-off-by: NeilBrown <[email protected]>
I haven't yet looked closely at these, but they look clean
and sensible at first glance. I'll apply them to nfsd-fixes
with Jeff's suggestion and Reviewed-by.
> ---
> include/linux/sunrpc/svc_rdma.h | 2 +-
> include/linux/sunrpc/svc_xprt.h | 2 +-
> net/sunrpc/svc_xprt.c | 21 ++++++++++++-----
> net/sunrpc/svcsock.c | 30 +++++++++++++-----------
> net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 11 ++++-----
> net/sunrpc/xprtrdma/svc_rdma_transport.c | 2 +-
> 6 files changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
> index 24aa159d29a7..fbc4bd423b35 100644
> --- a/include/linux/sunrpc/svc_rdma.h
> +++ b/include/linux/sunrpc/svc_rdma.h
> @@ -176,7 +176,7 @@ extern struct svc_rdma_recv_ctxt *
> extern void svc_rdma_recv_ctxt_put(struct svcxprt_rdma *rdma,
> struct svc_rdma_recv_ctxt *ctxt);
> extern void svc_rdma_flush_recv_queues(struct svcxprt_rdma *rdma);
> -extern void svc_rdma_release_rqst(struct svc_rqst *rqstp);
> +extern void svc_rdma_release_ctxt(struct svc_xprt *xprt, void *ctxt);
> extern int svc_rdma_recvfrom(struct svc_rqst *);
>
> /* svc_rdma_rw.c */
> diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
> index 867479204840..6f4473ee68e1 100644
> --- a/include/linux/sunrpc/svc_xprt.h
> +++ b/include/linux/sunrpc/svc_xprt.h
> @@ -23,7 +23,7 @@ struct svc_xprt_ops {
> int (*xpo_sendto)(struct svc_rqst *);
> int (*xpo_result_payload)(struct svc_rqst *, unsigned int,
> unsigned int);
> - void (*xpo_release_rqst)(struct svc_rqst *);
> + void (*xpo_release_ctxt)(struct svc_xprt *, void *);
> void (*xpo_detach)(struct svc_xprt *);
> void (*xpo_free)(struct svc_xprt *);
> void (*xpo_kill_temp_xprt)(struct svc_xprt *);
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 5fd94f6bdc75..1e3bba433561 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -532,13 +532,21 @@ void svc_reserve(struct svc_rqst *rqstp, int space)
> }
> EXPORT_SYMBOL_GPL(svc_reserve);
>
> +static void free_deferred(struct svc_xprt *xprt, struct svc_deferred_req *dr)
> +{
> + if (dr)
> + xprt->xpt_ops->xpo_release_ctxt(xprt, dr->xprt_ctxt);
> + kfree(dr);
> +}
> +
> static void svc_xprt_release(struct svc_rqst *rqstp)
> {
> struct svc_xprt *xprt = rqstp->rq_xprt;
>
> - xprt->xpt_ops->xpo_release_rqst(rqstp);
> + xprt->xpt_ops->xpo_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
> + rqstp->rq_xprt_ctxt = NULL;
>
> - kfree(rqstp->rq_deferred);
> + free_deferred(xprt, rqstp->rq_deferred);
> rqstp->rq_deferred = NULL;
>
> svc_rqst_release_pages(rqstp);
> @@ -1054,7 +1062,7 @@ static void svc_delete_xprt(struct svc_xprt *xprt)
> spin_unlock_bh(&serv->sv_lock);
>
> while ((dr = svc_deferred_dequeue(xprt)) != NULL)
> - kfree(dr);
> + free_deferred(xprt, dr);
>
> call_xpt_users(xprt);
> svc_xprt_put(xprt);
> @@ -1176,8 +1184,8 @@ static void svc_revisit(struct cache_deferred_req *dreq, int too_many)
> if (too_many || test_bit(XPT_DEAD, &xprt->xpt_flags)) {
> spin_unlock(&xprt->xpt_lock);
> trace_svc_defer_drop(dr);
> + free_deferred(xprt, dr);
> svc_xprt_put(xprt);
> - kfree(dr);
> return;
> }
> dr->xprt = NULL;
> @@ -1222,14 +1230,13 @@ static struct cache_deferred_req *svc_defer(struct cache_req *req)
> dr->addrlen = rqstp->rq_addrlen;
> dr->daddr = rqstp->rq_daddr;
> dr->argslen = rqstp->rq_arg.len >> 2;
> - dr->xprt_ctxt = rqstp->rq_xprt_ctxt;
>
> /* back up head to the start of the buffer and copy */
> skip = rqstp->rq_arg.len - rqstp->rq_arg.head[0].iov_len;
> memcpy(dr->args, rqstp->rq_arg.head[0].iov_base - skip,
> dr->argslen << 2);
> }
> - WARN_ON_ONCE(rqstp->rq_xprt_ctxt != dr->xprt_ctxt);
> + dr->xprt_ctxt = rqstp->rq_xprt_ctxt;
> rqstp->rq_xprt_ctxt = NULL;
> trace_svc_defer(rqstp);
> svc_xprt_get(rqstp->rq_xprt);
> @@ -1263,6 +1270,8 @@ static noinline int svc_deferred_recv(struct svc_rqst *rqstp)
> rqstp->rq_daddr = dr->daddr;
> rqstp->rq_respages = rqstp->rq_pages;
> rqstp->rq_xprt_ctxt = dr->xprt_ctxt;
> +
> + dr->xprt_ctxt = NULL;
> svc_xprt_received(rqstp->rq_xprt);
> return dr->argslen << 2;
> }
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index a51c9b989d58..aa4f31a770e3 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -121,27 +121,27 @@ static void svc_reclassify_socket(struct socket *sock)
> #endif
>
> /**
> - * svc_tcp_release_rqst - Release transport-related resources
> - * @rqstp: request structure with resources to be released
> + * svc_tcp_release_ctxt - Release transport-related resources
> + * @xprt: the transport which owned the context
> + * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
> *
> */
> -static void svc_tcp_release_rqst(struct svc_rqst *rqstp)
> +static void svc_tcp_release_ctxt(struct svc_xprt *xprt, void *ctxt)
> {
> }
>
> /**
> - * svc_udp_release_rqst - Release transport-related resources
> - * @rqstp: request structure with resources to be released
> + * svc_udp_release_ctxt - Release transport-related resources
> + * @xprt: the transport which owned the context
> + * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
> *
> */
> -static void svc_udp_release_rqst(struct svc_rqst *rqstp)
> +static void svc_udp_release_ctxt(struct svc_xprt *xprt, void *ctxt)
> {
> - struct sk_buff *skb = rqstp->rq_xprt_ctxt;
> + struct sk_buff *skb = ctxt;
>
> - if (skb) {
> - rqstp->rq_xprt_ctxt = NULL;
> + if (skb)
> consume_skb(skb);
> - }
> }
>
> union svc_pktinfo_u {
> @@ -696,7 +696,8 @@ static int svc_udp_sendto(struct svc_rqst *rqstp)
> unsigned int sent;
> int err;
>
> - svc_udp_release_rqst(rqstp);
> + svc_udp_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
> + rqstp->rq_xprt_ctxt = NULL;
>
> svc_set_cmsg_data(rqstp, cmh);
>
> @@ -768,7 +769,7 @@ static const struct svc_xprt_ops svc_udp_ops = {
> .xpo_recvfrom = svc_udp_recvfrom,
> .xpo_sendto = svc_udp_sendto,
> .xpo_result_payload = svc_sock_result_payload,
> - .xpo_release_rqst = svc_udp_release_rqst,
> + .xpo_release_ctxt = svc_udp_release_ctxt,
> .xpo_detach = svc_sock_detach,
> .xpo_free = svc_sock_free,
> .xpo_has_wspace = svc_udp_has_wspace,
> @@ -1298,7 +1299,8 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp)
> unsigned int sent;
> int err;
>
> - svc_tcp_release_rqst(rqstp);
> + svc_tcp_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
> + rqstp->rq_xprt_ctxt = NULL;
>
> atomic_inc(&svsk->sk_sendqlen);
> mutex_lock(&xprt->xpt_mutex);
> @@ -1343,7 +1345,7 @@ static const struct svc_xprt_ops svc_tcp_ops = {
> .xpo_recvfrom = svc_tcp_recvfrom,
> .xpo_sendto = svc_tcp_sendto,
> .xpo_result_payload = svc_sock_result_payload,
> - .xpo_release_rqst = svc_tcp_release_rqst,
> + .xpo_release_ctxt = svc_tcp_release_ctxt,
> .xpo_detach = svc_tcp_sock_detach,
> .xpo_free = svc_sock_free,
> .xpo_has_wspace = svc_tcp_has_wspace,
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> index 1c658fa43063..5c51e28b3111 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> @@ -239,21 +239,20 @@ void svc_rdma_recv_ctxt_put(struct svcxprt_rdma *rdma,
> }
>
> /**
> - * svc_rdma_release_rqst - Release transport-specific per-rqst resources
> - * @rqstp: svc_rqst being released
> + * svc_rdma_release_ctxt - Release transport-specific per-rqst resources
> + * @xprt: the transport which owned the context
> + * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
> *
> * Ensure that the recv_ctxt is released whether or not a Reply
> * was sent. For example, the client could close the connection,
> * or svc_process could drop an RPC, before the Reply is sent.
> */
> -void svc_rdma_release_rqst(struct svc_rqst *rqstp)
> +void svc_rdma_release_ctxt(struct svc_xprt *xprt, void *vctxt)
> {
> - struct svc_rdma_recv_ctxt *ctxt = rqstp->rq_xprt_ctxt;
> - struct svc_xprt *xprt = rqstp->rq_xprt;
> + struct svc_rdma_recv_ctxt *ctxt = vctxt;
> struct svcxprt_rdma *rdma =
> container_of(xprt, struct svcxprt_rdma, sc_xprt);
>
> - rqstp->rq_xprt_ctxt = NULL;
> if (ctxt)
> svc_rdma_recv_ctxt_put(rdma, ctxt);
> }
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> index 416b298f74dd..ca04f7a6a085 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> @@ -80,7 +80,7 @@ static const struct svc_xprt_ops svc_rdma_ops = {
> .xpo_recvfrom = svc_rdma_recvfrom,
> .xpo_sendto = svc_rdma_sendto,
> .xpo_result_payload = svc_rdma_result_payload,
> - .xpo_release_rqst = svc_rdma_release_rqst,
> + .xpo_release_ctxt = svc_rdma_release_ctxt,
> .xpo_detach = svc_rdma_detach,
> .xpo_free = svc_rdma_free,
> .xpo_has_wspace = svc_rdma_has_wspace,
> --
> 2.40.1
>
--
Chuck Lever
On Wed, 10 May 2023, Jeff Layton wrote:
> On Tue, 2023-05-09 at 09:42 +1000, NeilBrown wrote:
> > Since the ->xprt_ctxt pointer was added to svc_deferred_req, it has not
> > been sufficient to use kfree() to free a deferred request. We may need
> > to free the ctxt as well.
> >
> > As freeing the ctxt is all that ->xpo_release_rqst() does, we repurpose
> > it to explicit do that even when the ctxt is not stored in an rqst.
> > So we now have ->xpo_release_ctxt() which is given an xprt and a ctxt,
> > which may have been taken either from an rqst or from a dreq. The
> > caller is now responsible for clearing that pointer after the call to
> > ->xpo_release_ctxt.
> >
> > We also clear dr->xprt_ctxt when the ctxt is moved into a new rqst when
> > revisiting a deferred request. This ensures there is only one pointer
> > to the ctxt, so the risk of double freeing in future is reduced. The
> > new code in svc_xprt_release which releases both the ctxt and any
> > rq_deferred depends on this.
> >
>
> Thank you. Leaving stray pointers around like that is just asking for
> trouble.
>
> > Fixes: 773f91b2cf3f ("SUNRPC: Fix NFSD's request deferral on RDMA transports")
> > Signed-off-by: NeilBrown <[email protected]>
> > ---
> > include/linux/sunrpc/svc_rdma.h | 2 +-
> > include/linux/sunrpc/svc_xprt.h | 2 +-
> > net/sunrpc/svc_xprt.c | 21 ++++++++++++-----
> > net/sunrpc/svcsock.c | 30 +++++++++++++-----------
> > net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 11 ++++-----
> > net/sunrpc/xprtrdma/svc_rdma_transport.c | 2 +-
> > 6 files changed, 39 insertions(+), 29 deletions(-)
> >
> > diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
> > index 24aa159d29a7..fbc4bd423b35 100644
> > --- a/include/linux/sunrpc/svc_rdma.h
> > +++ b/include/linux/sunrpc/svc_rdma.h
> > @@ -176,7 +176,7 @@ extern struct svc_rdma_recv_ctxt *
> > extern void svc_rdma_recv_ctxt_put(struct svcxprt_rdma *rdma,
> > struct svc_rdma_recv_ctxt *ctxt);
> > extern void svc_rdma_flush_recv_queues(struct svcxprt_rdma *rdma);
> > -extern void svc_rdma_release_rqst(struct svc_rqst *rqstp);
> > +extern void svc_rdma_release_ctxt(struct svc_xprt *xprt, void *ctxt);
> > extern int svc_rdma_recvfrom(struct svc_rqst *);
> >
> > /* svc_rdma_rw.c */
> > diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
> > index 867479204840..6f4473ee68e1 100644
> > --- a/include/linux/sunrpc/svc_xprt.h
> > +++ b/include/linux/sunrpc/svc_xprt.h
> > @@ -23,7 +23,7 @@ struct svc_xprt_ops {
> > int (*xpo_sendto)(struct svc_rqst *);
> > int (*xpo_result_payload)(struct svc_rqst *, unsigned int,
> > unsigned int);
> > - void (*xpo_release_rqst)(struct svc_rqst *);
> > + void (*xpo_release_ctxt)(struct svc_xprt *, void *);
> > void (*xpo_detach)(struct svc_xprt *);
> > void (*xpo_free)(struct svc_xprt *);
> > void (*xpo_kill_temp_xprt)(struct svc_xprt *);
> > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> > index 5fd94f6bdc75..1e3bba433561 100644
> > --- a/net/sunrpc/svc_xprt.c
> > +++ b/net/sunrpc/svc_xprt.c
> > @@ -532,13 +532,21 @@ void svc_reserve(struct svc_rqst *rqstp, int space)
> > }
> > EXPORT_SYMBOL_GPL(svc_reserve);
> >
> > +static void free_deferred(struct svc_xprt *xprt, struct svc_deferred_req *dr)
> > +{
> > + if (dr)
> > + xprt->xpt_ops->xpo_release_ctxt(xprt, dr->xprt_ctxt);
> > + kfree(dr);
>
> nit: might as well put the kfree inside the if block to avoid it in the
> common case of dr == NULL.
I did that at first. Then I took it out of the if block - more vague
aesthetics than anything else. I'm perfectly happy for it to go back in.
>
> > +}
> > +
> > static void svc_xprt_release(struct svc_rqst *rqstp)
> > {
> > struct svc_xprt *xprt = rqstp->rq_xprt;
> >
> > - xprt->xpt_ops->xpo_release_rqst(rqstp);
> > + xprt->xpt_ops->xpo_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
> > + rqstp->rq_xprt_ctxt = NULL;
> >
> > - kfree(rqstp->rq_deferred);
> > + free_deferred(xprt, rqstp->rq_deferred);
> > rqstp->rq_deferred = NULL;
> >
> > svc_rqst_release_pages(rqstp);
> > @@ -1054,7 +1062,7 @@ static void svc_delete_xprt(struct svc_xprt *xprt)
> > spin_unlock_bh(&serv->sv_lock);
> >
> > while ((dr = svc_deferred_dequeue(xprt)) != NULL)
> > - kfree(dr);
> > + free_deferred(xprt, dr);
> >
> > call_xpt_users(xprt);
> > svc_xprt_put(xprt);
> > @@ -1176,8 +1184,8 @@ static void svc_revisit(struct cache_deferred_req *dreq, int too_many)
> > if (too_many || test_bit(XPT_DEAD, &xprt->xpt_flags)) {
> > spin_unlock(&xprt->xpt_lock);
> > trace_svc_defer_drop(dr);
> > + free_deferred(xprt, dr);
> > svc_xprt_put(xprt);
> > - kfree(dr);
> > return;
> > }
> > dr->xprt = NULL;
> > @@ -1222,14 +1230,13 @@ static struct cache_deferred_req *svc_defer(struct cache_req *req)
> > dr->addrlen = rqstp->rq_addrlen;
> > dr->daddr = rqstp->rq_daddr;
> > dr->argslen = rqstp->rq_arg.len >> 2;
> > - dr->xprt_ctxt = rqstp->rq_xprt_ctxt;
> >
> > /* back up head to the start of the buffer and copy */
> > skip = rqstp->rq_arg.len - rqstp->rq_arg.head[0].iov_len;
> > memcpy(dr->args, rqstp->rq_arg.head[0].iov_base - skip,
> > dr->argslen << 2);
> > }
> > - WARN_ON_ONCE(rqstp->rq_xprt_ctxt != dr->xprt_ctxt);
> > + dr->xprt_ctxt = rqstp->rq_xprt_ctxt;
> > rqstp->rq_xprt_ctxt = NULL;
> > trace_svc_defer(rqstp);
> > svc_xprt_get(rqstp->rq_xprt);
> > @@ -1263,6 +1270,8 @@ static noinline int svc_deferred_recv(struct svc_rqst *rqstp)
> > rqstp->rq_daddr = dr->daddr;
> > rqstp->rq_respages = rqstp->rq_pages;
> > rqstp->rq_xprt_ctxt = dr->xprt_ctxt;
> > +
> > + dr->xprt_ctxt = NULL;
> > svc_xprt_received(rqstp->rq_xprt);
> > return dr->argslen << 2;
> > }
> > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> > index a51c9b989d58..aa4f31a770e3 100644
> > --- a/net/sunrpc/svcsock.c
> > +++ b/net/sunrpc/svcsock.c
> > @@ -121,27 +121,27 @@ static void svc_reclassify_socket(struct socket *sock)
> > #endif
> >
> > /**
> > - * svc_tcp_release_rqst - Release transport-related resources
> > - * @rqstp: request structure with resources to be released
> > + * svc_tcp_release_ctxt - Release transport-related resources
> > + * @xprt: the transport which owned the context
> > + * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
> > *
> > */
> > -static void svc_tcp_release_rqst(struct svc_rqst *rqstp)
> > +static void svc_tcp_release_ctxt(struct svc_xprt *xprt, void *ctxt)
> > {
> > }
> >
> > /**
> > - * svc_udp_release_rqst - Release transport-related resources
> > - * @rqstp: request structure with resources to be released
> > + * svc_udp_release_ctxt - Release transport-related resources
> > + * @xprt: the transport which owned the context
> > + * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
> > *
> > */
> > -static void svc_udp_release_rqst(struct svc_rqst *rqstp)
> > +static void svc_udp_release_ctxt(struct svc_xprt *xprt, void *ctxt)
> > {
> > - struct sk_buff *skb = rqstp->rq_xprt_ctxt;
> > + struct sk_buff *skb = ctxt;
> >
> > - if (skb) {
> > - rqstp->rq_xprt_ctxt = NULL;
> > + if (skb)
> > consume_skb(skb);
> > - }
> > }
> >
> > union svc_pktinfo_u {
> > @@ -696,7 +696,8 @@ static int svc_udp_sendto(struct svc_rqst *rqstp)
> > unsigned int sent;
> > int err;
> >
> > - svc_udp_release_rqst(rqstp);
> > + svc_udp_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
> > + rqstp->rq_xprt_ctxt = NULL;
> >
> > svc_set_cmsg_data(rqstp, cmh);
> >
> > @@ -768,7 +769,7 @@ static const struct svc_xprt_ops svc_udp_ops = {
> > .xpo_recvfrom = svc_udp_recvfrom,
> > .xpo_sendto = svc_udp_sendto,
> > .xpo_result_payload = svc_sock_result_payload,
> > - .xpo_release_rqst = svc_udp_release_rqst,
> > + .xpo_release_ctxt = svc_udp_release_ctxt,
> > .xpo_detach = svc_sock_detach,
> > .xpo_free = svc_sock_free,
> > .xpo_has_wspace = svc_udp_has_wspace,
> > @@ -1298,7 +1299,8 @@ static int svc_tcp_sendto(struct svc_rqst *rqstp)
> > unsigned int sent;
> > int err;
> >
> > - svc_tcp_release_rqst(rqstp);
> > + svc_tcp_release_ctxt(xprt, rqstp->rq_xprt_ctxt);
> > + rqstp->rq_xprt_ctxt = NULL;
> >
> > atomic_inc(&svsk->sk_sendqlen);
> > mutex_lock(&xprt->xpt_mutex);
> > @@ -1343,7 +1345,7 @@ static const struct svc_xprt_ops svc_tcp_ops = {
> > .xpo_recvfrom = svc_tcp_recvfrom,
> > .xpo_sendto = svc_tcp_sendto,
> > .xpo_result_payload = svc_sock_result_payload,
> > - .xpo_release_rqst = svc_tcp_release_rqst,
> > + .xpo_release_ctxt = svc_tcp_release_ctxt,
> > .xpo_detach = svc_tcp_sock_detach,
> > .xpo_free = svc_sock_free,
> > .xpo_has_wspace = svc_tcp_has_wspace,
> > diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> > index 1c658fa43063..5c51e28b3111 100644
> > --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> > +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> > @@ -239,21 +239,20 @@ void svc_rdma_recv_ctxt_put(struct svcxprt_rdma *rdma,
> > }
> >
> > /**
> > - * svc_rdma_release_rqst - Release transport-specific per-rqst resources
> > - * @rqstp: svc_rqst being released
> > + * svc_rdma_release_ctxt - Release transport-specific per-rqst resources
> > + * @xprt: the transport which owned the context
> > + * @ctxt: the context from rqstp->rq_xprt_ctxt or dr->xprt_ctxt
> > *
> > * Ensure that the recv_ctxt is released whether or not a Reply
> > * was sent. For example, the client could close the connection,
> > * or svc_process could drop an RPC, before the Reply is sent.
> > */
> > -void svc_rdma_release_rqst(struct svc_rqst *rqstp)
> > +void svc_rdma_release_ctxt(struct svc_xprt *xprt, void *vctxt)
> > {
> > - struct svc_rdma_recv_ctxt *ctxt = rqstp->rq_xprt_ctxt;
> > - struct svc_xprt *xprt = rqstp->rq_xprt;
> > + struct svc_rdma_recv_ctxt *ctxt = vctxt;
> > struct svcxprt_rdma *rdma =
> > container_of(xprt, struct svcxprt_rdma, sc_xprt);
> >
> > - rqstp->rq_xprt_ctxt = NULL;
> > if (ctxt)
> > svc_rdma_recv_ctxt_put(rdma, ctxt);
> > }
> > diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > index 416b298f74dd..ca04f7a6a085 100644
> > --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > @@ -80,7 +80,7 @@ static const struct svc_xprt_ops svc_rdma_ops = {
> > .xpo_recvfrom = svc_rdma_recvfrom,
> > .xpo_sendto = svc_rdma_sendto,
> > .xpo_result_payload = svc_rdma_result_payload,
> > - .xpo_release_rqst = svc_rdma_release_rqst,
> > + .xpo_release_ctxt = svc_rdma_release_ctxt,
> > .xpo_detach = svc_rdma_detach,
> > .xpo_free = svc_rdma_free,
> > .xpo_has_wspace = svc_rdma_has_wspace,
>
> Reviewed-by: Jeff Layton <[email protected]>
Thanks,
NeilBrown
>