diff options
author | Jeff Layton <jlayton@redhat.com> | 2008-02-11 10:00:20 -0500 |
---|---|---|
committer | Trond Myklebust <Trond.Myklebust@netapp.com> | 2008-02-13 23:24:04 -0500 |
commit | 8e60029f403781b8a63b7ffdb7dc1faff6ca651e (patch) | |
tree | 9eb13d36a8951ef160250bb973648426b456c46b /fs/nfs | |
parent | e760e716d47b48caf98da348368fd41b4a9b9e7e (diff) |
NFS: fix reference counting for NFSv4 callback thread
The reference counting for the NFSv4 callback thread stays artificially
high. When this thread comes down, it doesn't properly tear down the
svc_serv, causing a memory leak. In my testing on an older kernel on
x86_64, memory would leak out of the 8k kmalloc slab. So, we're leaking
at least a page of memory every time the thread comes down.
svc_create() creates the svc_serv with a sv_nrthreads count of 1, and
then svc_create_thread() increments that count. Whenever the callback
thread is started it has a sv_nrthreads count of 2. When coming down, it
calls svc_exit_thread() which decrements that count and if it hits 0, it
tears everything down. That never happens here since the count is always
at 2 when the thread exits.
The problem is that nfs_callback_up() should be calling svc_destroy() on
the svc_serv on both success and failure. This is how lockd_up_proto()
handles the reference counting, and doing that here fixes the leak.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Diffstat (limited to 'fs/nfs')
-rw-r--r-- | fs/nfs/callback.c | 18 |
1 files changed, 12 insertions, 6 deletions
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c index bd185a572a23..ecc06c619494 100644 --- a/fs/nfs/callback.c +++ b/fs/nfs/callback.c | |||
@@ -105,7 +105,7 @@ static void nfs_callback_svc(struct svc_rqst *rqstp) | |||
105 | */ | 105 | */ |
106 | int nfs_callback_up(void) | 106 | int nfs_callback_up(void) |
107 | { | 107 | { |
108 | struct svc_serv *serv; | 108 | struct svc_serv *serv = NULL; |
109 | int ret = 0; | 109 | int ret = 0; |
110 | 110 | ||
111 | lock_kernel(); | 111 | lock_kernel(); |
@@ -122,24 +122,30 @@ int nfs_callback_up(void) | |||
122 | ret = svc_create_xprt(serv, "tcp", nfs_callback_set_tcpport, | 122 | ret = svc_create_xprt(serv, "tcp", nfs_callback_set_tcpport, |
123 | SVC_SOCK_ANONYMOUS); | 123 | SVC_SOCK_ANONYMOUS); |
124 | if (ret <= 0) | 124 | if (ret <= 0) |
125 | goto out_destroy; | 125 | goto out_err; |
126 | nfs_callback_tcpport = ret; | 126 | nfs_callback_tcpport = ret; |
127 | dprintk("Callback port = 0x%x\n", nfs_callback_tcpport); | 127 | dprintk("Callback port = 0x%x\n", nfs_callback_tcpport); |
128 | 128 | ||
129 | ret = svc_create_thread(nfs_callback_svc, serv); | 129 | ret = svc_create_thread(nfs_callback_svc, serv); |
130 | if (ret < 0) | 130 | if (ret < 0) |
131 | goto out_destroy; | 131 | goto out_err; |
132 | nfs_callback_info.serv = serv; | 132 | nfs_callback_info.serv = serv; |
133 | wait_for_completion(&nfs_callback_info.started); | 133 | wait_for_completion(&nfs_callback_info.started); |
134 | out: | 134 | out: |
135 | /* | ||
136 | * svc_create creates the svc_serv with sv_nrthreads == 1, and then | ||
137 | * svc_create_thread increments that. So we need to call svc_destroy | ||
138 | * on both success and failure so that the refcount is 1 when the | ||
139 | * thread exits. | ||
140 | */ | ||
141 | if (serv) | ||
142 | svc_destroy(serv); | ||
135 | mutex_unlock(&nfs_callback_mutex); | 143 | mutex_unlock(&nfs_callback_mutex); |
136 | unlock_kernel(); | 144 | unlock_kernel(); |
137 | return ret; | 145 | return ret; |
138 | out_destroy: | 146 | out_err: |
139 | dprintk("Couldn't create callback socket or server thread; err = %d\n", | 147 | dprintk("Couldn't create callback socket or server thread; err = %d\n", |
140 | ret); | 148 | ret); |
141 | svc_destroy(serv); | ||
142 | out_err: | ||
143 | nfs_callback_info.users--; | 149 | nfs_callback_info.users--; |
144 | goto out; | 150 | goto out; |
145 | } | 151 | } |