NFSD/SUNRPC: Fix memory leak for the backchannel
Subsystems (3)
cover NFSD/SUNRPC: Fix memory leak for the backchannel
When using pynfs test-site, I found some memory leak for the backchannel.
Kinglong Mee (5):
NFSD: Using free_conn free connection
NFSD: Free backchannel xprt in bc_destroy
SUNRPC: New helper for creating client with rpc_xprt NFSD/SUNRPC: Check rpc_xprt out of xs_setup_bc_tcp
SUNRPC: Clear xpt_bc_xprt if xs_setup_bc_tcp failed
fs/nfsd/nfs4callback.c | 19 ++++++++++++++- fs/nfsd/nfs4state.c | 3 ++- include/linux/sunrpc/clnt.h | 2 ++ include/linux/sunrpc/xprt.h | 13 +++++++++- net/sunrpc/clnt.c | 58 ++++++++++++++++++++++++++------------------- net/sunrpc/xprt.c | 12 ---------- net/sunrpc/xprtsock.c | 15 +++++------- 7 files changed, 73 insertions(+), 49 deletions(-)
Signature · 1 line
1.8.5.3
PATCH RESEND 1/5 NFSD: Using free_conn free connection
Connection from alloc_conn must be freed through free_conn, otherwise, the reference of svc_xprt will never be put.
--- fs/nfsd/nfs4state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
fs/nfsd/nfs4state.c +2 -1
@@ -2287,7 +2287,8 @@ out:if (!list_empty (&clp->cl_revoked)) seq->status_flags |=SEQ4_STATUS_RECALLABLE_STATE_REVOKED ; out_no_session: -kfree (conn); +if (conn) +free_conn (conn);spin_unlock (&nn->client_lock);return status; out_put_session:
Signature · 1 line
1.8.5.3
PATCH RESEND 2/5 NFSD: Free backchannel xprt in bc_destroy
In reply to Kinglong MeeBackchannel xprt isn't freed right now.
Free it in bc_destroy, and put the reference of THIS_MODULE.
--- net/sunrpc/xprtsock.c | 4 ++++ 1 file changed, 4 insertions(+)
net/sunrpc/xprtsock.c +4 -0
@@ -2549,6 +2549,10 @@ static void bc_close(struct rpc_xprt *xprt)static void bc_destroy (struct rpc_xprt *xprt) { +dprintk ("RPC: bc_destroy xprt %p\n" , xprt); + +xs_xprt_free (xprt); +module_put (THIS_MODULE ); }static struct rpc_xprt_ops xs_local_ops = {
Signature · 1 line
1.8.5.3
PATCH RESEND 3/5 SUNRPC: New helper for creating client with rpc_xprt
In reply to Kinglong Mee--- include/linux/sunrpc/clnt.h | 2 ++ net/sunrpc/clnt.c | 58 ++++++++++++++++++++++++++------------------- 2 files changed, 35 insertions(+), 25 deletions(-)
include/linux/sunrpc/clnt.h +2 -0
@@ -130,6 +130,8 @@ struct rpc_create_args {#define RPC_CLNT_CREATE_NO_RETRANS_TIMEOUT (1UL << 9)struct rpc_clnt *rpc_create (struct rpc_create_args *args); +struct rpc_clnt *rpc_create_xprt (struct rpc_create_args *args, +struct rpc_xprt *xprt);struct rpc_clnt *rpc_bind_new_program (struct rpc_clnt *,const struct rpc_program *,u32 );void rpc_task_reset_client (struct rpc_task *task,struct rpc_clnt *clnt);
net/sunrpc/clnt.c +33 -25
76 lines not on this page · raw message
Signature · 1 line
1.8.5.3
PATCH RESEND 4/5 NFSD/SUNRPC: Check rpc_xprt out of xs_setup_bc_tcp
In reply to Kinglong MeeBesides checking rpc_xprt out of xs_setup_bc_tcp,
increase it's reference (it's important).
--- fs/nfsd/nfs4callback.c | 19 ++++++++++++++++++- include/linux/sunrpc/xprt.h | 13 ++++++++++++- net/sunrpc/xprt.c | 12 ------------ net/sunrpc/xprtsock.c | 9 --------- 4 files changed, 30 insertions(+), 23 deletions(-)
fs/nfsd/nfs4callback.c +18 -1
@@ -32,6 +32,7 @@*/ #include <linux/sunrpc/clnt.h> +#include <linux/sunrpc/xprt.h> #include <linux/sunrpc/svc_xprt.h> #include <linux/slab.h> #include "nfsd.h"
@@ -635,6 +636,22 @@ static struct rpc_cred *get_backchannel_cred(struct nfs4_client *clp, struct rpc } } +static struct rpc_clnt *create_backchannel_client (struct rpc_create_args *args) +{ +struct rpc_xprt *xprt; + +if (args->protocol !=XPRT_TRANSPORT_BC_TCP ) +return rpc_create (args); + + xprt = args->bc_xprt->xpt_bc_xprt; +if (xprt) { +xprt_get (xprt); +return rpc_create_xprt (args, xprt); + } + +return rpc_create (args); +} +static int setup_callback_client (struct nfs4_client *clp,struct nfs4_cb_conn *conn,struct nfsd4_session *ses) {struct rpc_timeout timeparms = {
@@ -674,7 +691,7 @@ static int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *c args.authflavor = ses->se_cb_sec.flavor; }/* Create RPC client */ - client =rpc_create (&args); + client =create_backchannel_client (&args);if (IS_ERR (client)) {dprintk ("NFSD: couldn't create callback client: %ld\n" ,PTR_ERR (client));
include/linux/sunrpc/xprt.h +12 -1
@@ -295,13 +295,24 @@ int xprt_adjust_timeout(struct rpc_rqst *req);void xprt_release_xprt (struct rpc_xprt *xprt,struct rpc_task *task);void xprt_release_xprt_cong (struct rpc_xprt *xprt,struct rpc_task *task);void xprt_release (struct rpc_task *task); -struct rpc_xprt *xprt_get (struct rpc_xprt *xprt);void xprt_put (struct rpc_xprt *xprt);struct rpc_xprt *xprt_alloc (struct net *net,size_t size,unsigned int num_prealloc,unsigned int max_req);void xprt_free (struct rpc_xprt *); +/** +* xprt_get - return a reference to an RPC transport. +* @xprt: pointer to the transport +* +*/ +static inline struct rpc_xprt *xprt_get (struct rpc_xprt *xprt) +{ +if (atomic_inc_not_zero (&xprt->count)) +return xprt; +return NULL ; +} +static inline __be32 *xprt_skip_transport_header (struct rpc_xprt *xprt,__be32 *p) {return p + xprt->tsh_size;
net/sunrpc/xprt.c +0 -12
@@ -1383,15 +1383,3 @@ void xprt_put(struct rpc_xprt *xprt)if (atomic_dec_and_test (&xprt->count))xprt_destroy (xprt); } - -/** -* xprt_get - return a reference to an RPC transport. -* @xprt: pointer to the transport -* -*/ -struct rpc_xprt *xprt_get (struct rpc_xprt *xprt) -{ -if (atomic_inc_not_zero (&xprt->count)) -return xprt; -return NULL ; -}
net/sunrpc/xprtsock.c +0 -9
@@ -2927,15 +2927,6 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)struct svc_sock *bc_sock;struct rpc_xprt *ret; -if (args->bc_xprt->xpt_bc_xprt) { -/* -* This server connection already has a backchannel -* transport; we can't create a new one, as we wouldn't -* be able to match replies based on xid any more. So, -* reuse the already-existing one: -*/ -return args->bc_xprt->xpt_bc_xprt; - } xprt =xs_setup_xprt (args, xprt_tcp_slot_table_entries, xprt_tcp_slot_table_entries);if (IS_ERR (xprt))
Signature · 1 line
1.8.5.3
PATCH RESEND 5/5 SUNRPC: Clear xpt_bc_xprt if xs_setup_bc_tcp failed
In reply to Kinglong MeeDon't move the assign of args->bc_xprt->xpt_bc_xprt out of xs_setup_bc_tcp, because rpc_ping (which is in rpc_create) will using it.
--- net/sunrpc/xprtsock.c | 2 ++ 1 file changed, 2 insertions(+)
net/sunrpc/xprtsock.c +2 -0
@@ -2986,6 +2986,8 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)if (try_module_get (THIS_MODULE ))return xprt; + + args->bc_xprt->xpt_bc_xprt =NULL ;xprt_put (xprt); ret =ERR_PTR (-EINVAL ); out_err:
Signature · 1 line
1.8.5.3
Quoted 1 line · Kinglong Mee
When using pynfs test-site, I found some memory leak for the backchannel.
All applied, thanks.--b.
Quoted 19 lines
Kinglong Mee (5):
NFSD: Usingfree_connfree connection
NFSD: Free backchannel xprt inbc_destroy
SUNRPC: New helper for creating client withrpc_xprtNFSD/SUNRPC: Checkrpc_xprtout ofxs_setup_bc_tcp
SUNRPC: Clearxpt_bc_xprtifxs_setup_bc_tcpfailedfs/nfsd/nfs4callback.c | 19 ++++++++++++++- fs/nfsd/nfs4state.c | 3 ++- include/linux/sunrpc/clnt.h | 2 ++ include/linux/sunrpc/xprt.h | 13 +++++++++- net/sunrpc/clnt.c | 58 ++++++++++++++++++++++++++------------------- net/sunrpc/xprt.c | 12 ---------- net/sunrpc/xprtsock.c | 15 +++++------- 7 files changed, 73 insertions(+), 49 deletions(-)Signature · 1 line
1.8.5.3