2009-01-05 17:12:53

by Tom Tucker

Subject: [PATCH] svc: Move kfree of deferral record to common code

The rqstp structure has a pointer to a svc_deferred_req record
that is allocated when requests are deferred. This record is common
to all transports and can be freed in common code.

Move the kfree of the rq_deferred to the common svc_xprt_release
function.

This also fixes a memory leak in the RDMA transport which does not
kfree the dr structure in it's version of the xpo_release_rqst callback.

Signed-off-by: Tom Tucker <[email protected]>

---
net/sunrpc/svc_xprt.c | 3 +++
net/sunrpc/svcsock.c | 5 -----
2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 375a695..de16a97 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -448,6 +448,9 @@ static void svc_xprt_release(struct svc_rqst *rqstp)

rqstp->rq_xprt->xpt_ops->xpo_release_rqst(rqstp);

+ kfree(rqstp->rq_deferred);
+ rqstp->rq_deferred = NULL;
+
svc_free_res_pages(rqstp);
rqstp->rq_res.page_len = 0;
rqstp->rq_res.page_base = 0;
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index ef3238d..e59a88e 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -102,7 +102,6 @@ static void svc_reclassify_socket(struct socket *sock)
static void svc_release_skb(struct svc_rqst *rqstp)
{
struct sk_buff *skb = rqstp->rq_xprt_ctxt;
- struct svc_deferred_req *dr = rqstp->rq_deferred;

if (skb) {
struct svc_sock *svsk =
@@ -112,10 +111,6 @@ static void svc_release_skb(struct svc_rqst *rqstp)
dprintk("svc: service %p, releasing skb %p\n", rqstp, skb);
skb_free_datagram(svsk->sk_sk, skb);
}
- if (dr) {
- rqstp->rq_deferred = NULL;
- kfree(dr);
- }
}

union svc_pktinfo_u {

2009-01-05 18:34:55

by J. Bruce Fields

Subject: Re: [PATCH] svc: Move kfree of deferral record to common code

On Mon, Jan 05, 2009 at 11:12:52AM -0600, Tom Tucker wrote:
>
> The rqstp structure has a pointer to a svc_deferred_req record
> that is allocated when requests are deferred. This record is common
> to all transports and can be freed in common code.
>
> Move the kfree of the rq_deferred to the common svc_xprt_release
> function.
>
> This also fixes a memory leak in the RDMA transport which does not
> kfree the dr structure in it's version of the xpo_release_rqst callback.
>
> Signed-off-by: Tom Tucker <[email protected]>

Looks good, thanks--applied. (But, minor whine: your mailer seems to
have changed all the tabs in the messages to spaces, so I had to apply
the patch by hand.)

--b.

>
> ---
> net/sunrpc/svc_xprt.c | 3 +++
> net/sunrpc/svcsock.c | 5 -----
> 2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 375a695..de16a97 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -448,6 +448,9 @@ static void svc_xprt_release(struct svc_rqst *rqstp)
>
> rqstp->rq_xprt->xpt_ops->xpo_release_rqst(rqstp);
>
> + kfree(rqstp->rq_deferred);
> + rqstp->rq_deferred = NULL;
> +
> svc_free_res_pages(rqstp);
> rqstp->rq_res.page_len = 0;
> rqstp->rq_res.page_base = 0;
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index ef3238d..e59a88e 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -102,7 +102,6 @@ static void svc_reclassify_socket(struct socket *sock)
> static void svc_release_skb(struct svc_rqst *rqstp)
> {
> struct sk_buff *skb = rqstp->rq_xprt_ctxt;
> - struct svc_deferred_req *dr = rqstp->rq_deferred;
>
> if (skb) {
> struct svc_sock *svsk =
> @@ -112,10 +111,6 @@ static void svc_release_skb(struct svc_rqst *rqstp)
> dprintk("svc: service %p, releasing skb %p\n", rqstp, skb);
> skb_free_datagram(svsk->sk_sk, skb);
> }
> - if (dr) {
> - rqstp->rq_deferred = NULL;
> - kfree(dr);
> - }
> }
>
> union svc_pktinfo_u {