Skip to content

Commit

Permalink
NFS: fix reference counting for NFSv4 callback thread
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
  • Loading branch information
jtlayton authored and Trond Myklebust committed Feb 14, 2008
1 parent e760e71 commit 8e60029
Showing 1 changed file with 12 additions and 6 deletions.
18 changes: 12 additions & 6 deletions fs/nfs/callback.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ static void nfs_callback_svc(struct svc_rqst *rqstp)
*/
int nfs_callback_up(void)
{
struct svc_serv *serv;
struct svc_serv *serv = NULL;
int ret = 0;

lock_kernel();
Expand All @@ -122,24 +122,30 @@ int nfs_callback_up(void)
ret = svc_create_xprt(serv, "tcp", nfs_callback_set_tcpport,
SVC_SOCK_ANONYMOUS);
if (ret <= 0)
goto out_destroy;
goto out_err;
nfs_callback_tcpport = ret;
dprintk("Callback port = 0x%x\n", nfs_callback_tcpport);

ret = svc_create_thread(nfs_callback_svc, serv);
if (ret < 0)
goto out_destroy;
goto out_err;
nfs_callback_info.serv = serv;
wait_for_completion(&nfs_callback_info.started);
out:
/*
* svc_create creates the svc_serv with sv_nrthreads == 1, and then
* svc_create_thread increments that. So we need to call svc_destroy
* on both success and failure so that the refcount is 1 when the
* thread exits.
*/
if (serv)
svc_destroy(serv);
mutex_unlock(&nfs_callback_mutex);
unlock_kernel();
return ret;
out_destroy:
out_err:
dprintk("Couldn't create callback socket or server thread; err = %d\n",
ret);
svc_destroy(serv);
out_err:
nfs_callback_info.users--;
goto out;
}
Expand Down

0 comments on commit 8e60029

Please sign in to comment.