NFS: fix reference counting for NFSv4 callback thread

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: <git-commits-head@...>
Date: Friday, February 15, 2008 - 6:01 pm

Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=8e6002...
Commit:     8e60029f403781b8a63b7ffdb7dc1faff6ca651e
Parent:     e760e716d47b48caf98da348368fd41b4a9b9e7e
Author:     Jeff Layton <jlayton@redhat.com>
AuthorDate: Mon Feb 11 10:00:20 2008 -0500
Committer:  Trond Myklebust <Trond.Myklebust@netapp.com>
CommitDate: Wed Feb 13 23:24:04 2008 -0500

    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>
---
 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 bd185a5..ecc06c6 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -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();
@@ -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;
 }
-
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
NFS: fix reference counting for NFSv4 callback thread, Linux Kernel Mailing List..., (Fri Feb 15, 6:01 pm)