From 8e60029f403781b8a63b7ffdb7dc1faff6ca651e Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 11 Feb 2008 10:00:20 -0500 Subject: 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 Signed-off-by: Trond Myklebust --- fs/nfs/callback.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) (limited to 'fs/nfs') 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; } -- cgit v1.1 From 497799e7c0ac7e82164a510ebf8beed7b3635e34 Mon Sep 17 00:00:00 2001 From: Dan Muntz Date: Wed, 13 Feb 2008 13:09:35 -0800 Subject: NFS: missing spaces in KERN_WARNING The warning message for a v4 server returning various bad sequence-ids is missing spaces. Signed-off-by: Dan Muntz Signed-off-by: Trond Myklebust --- fs/nfs/nfs4state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/nfs') diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index f9c7432..6233eb5 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -682,8 +682,8 @@ static void nfs_increment_seqid(int status, struct nfs_seqid *seqid) if (seqid->sequence->flags & NFS_SEQID_CONFIRMED) return; printk(KERN_WARNING "NFS: v4 server returned a bad" - "sequence-id error on an" - "unconfirmed sequence %p!\n", + " sequence-id error on an" + " unconfirmed sequence %p!\n", seqid->sequence); case -NFS4ERR_STALE_CLIENTID: case -NFS4ERR_STALE_STATEID: -- cgit v1.1 From 25606656b19a38bbece914c4c67101f674908f49 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 12 Feb 2008 06:49:01 -0500 Subject: NFS: remove error field from nfs_readdir_descriptor_t The error field in nfs_readdir_descriptor_t is never used outside of the function in which it is set. Remove the field and change the place that does use it to use an existing local variable. Signed-off-by: Jeff Layton Signed-off-by: Trond Myklebust --- fs/nfs/dir.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'fs/nfs') diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 476cb0f..ae04892 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -154,7 +154,6 @@ typedef struct { struct nfs_entry *entry; decode_dirent_t decode; int plus; - int error; unsigned long timestamp; int timestamp_valid; } nfs_readdir_descriptor_t; @@ -213,7 +212,6 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page *page) return 0; error: unlock_page(page); - desc->error = error; return -EIO; } @@ -483,13 +481,13 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent, goto out; } timestamp = jiffies; - desc->error = NFS_PROTO(inode)->readdir(file->f_path.dentry, cred, *desc->dir_cookie, - page, + status = NFS_PROTO(inode)->readdir(file->f_path.dentry, cred, + *desc->dir_cookie, page, NFS_SERVER(inode)->dtsize, desc->plus); desc->page = page; desc->ptr = kmap(page); /* matching kunmap in nfs_do_filldir */ - if (desc->error >= 0) { + if (status >= 0) { desc->timestamp = timestamp; desc->timestamp_valid = 1; if ((status = dir_decode(desc)) == 0) -- cgit v1.1 From 8d042218b075de3cdbe066198515b3521553746e Mon Sep 17 00:00:00 2001 From: Olga Kornievskaia Date: Wed, 13 Feb 2008 16:47:06 -0500 Subject: NFS: add missing spkm3 strings to mount option parser This patch adds previous missing spkm3 string values that are needed to parse mount options in the kernel. --- fs/nfs/super.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'fs/nfs') diff --git a/fs/nfs/super.c b/fs/nfs/super.c index 7f4505f..1fb3818 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -190,6 +190,10 @@ static match_table_t nfs_secflavor_tokens = { { Opt_sec_lkeyi, "lkeyi" }, { Opt_sec_lkeyp, "lkeyp" }, + { Opt_sec_spkm, "spkm3" }, + { Opt_sec_spkmi, "spkm3i" }, + { Opt_sec_spkmp, "spkm3p" }, + { Opt_sec_err, NULL } }; -- cgit v1.1 From 4ac9137858e08a19f29feac4e1f4df7c268b0ba5 Mon Sep 17 00:00:00 2001 From: Jan Blunck Date: Thu, 14 Feb 2008 19:34:32 -0800 Subject: Embed a struct path into struct nameidata instead of nd->{dentry,mnt} This is the central patch of a cleanup series. In most cases there is no good reason why someone would want to use a dentry for itself. This series reflects that fact and embeds a struct path into nameidata. Together with the other patches of this series - it enforced the correct order of getting/releasing the reference count on pairs - it prepares the VFS for stacking support since it is essential to have a struct path in every place where the stack can be traversed - it reduces the overall code size: without patch series: text data bss dec hex filename 5321639 858418 715768 6895825 6938d1 vmlinux with patch series: text data bss dec hex filename 5320026 858418 715768 6894212 693284 vmlinux This patch: Switch from nd->{dentry,mnt} to nd->path.{dentry,mnt} everywhere. [akpm@linux-foundation.org: coding-style fixes] [akpm@linux-foundation.org: fix cifs] [akpm@linux-foundation.org: fix smack] Signed-off-by: Jan Blunck Signed-off-by: Andreas Gruenbacher Acked-by: Christoph Hellwig Cc: Al Viro Cc: Casey Schaufler Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/nfs/namespace.c | 27 +++++++++++++++------------ fs/nfs/nfs4proc.c | 8 ++++---- 2 files changed, 19 insertions(+), 16 deletions(-) (limited to 'fs/nfs') diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c index be4ce1c..3b6d83d 100644 --- a/fs/nfs/namespace.c +++ b/fs/nfs/namespace.c @@ -107,38 +107,40 @@ static void * nfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd) BUG_ON(IS_ROOT(dentry)); dprintk("%s: enter\n", __FUNCTION__); - dput(nd->dentry); - nd->dentry = dget(dentry); + dput(nd->path.dentry); + nd->path.dentry = dget(dentry); /* Look it up again */ - parent = dget_parent(nd->dentry); + parent = dget_parent(nd->path.dentry); err = server->nfs_client->rpc_ops->lookup(parent->d_inode, - &nd->dentry->d_name, + &nd->path.dentry->d_name, &fh, &fattr); dput(parent); if (err != 0) goto out_err; if (fattr.valid & NFS_ATTR_FATTR_V4_REFERRAL) - mnt = nfs_do_refmount(nd->mnt, nd->dentry); + mnt = nfs_do_refmount(nd->path.mnt, nd->path.dentry); else - mnt = nfs_do_submount(nd->mnt, nd->dentry, &fh, &fattr); + mnt = nfs_do_submount(nd->path.mnt, nd->path.dentry, &fh, + &fattr); err = PTR_ERR(mnt); if (IS_ERR(mnt)) goto out_err; mntget(mnt); - err = do_add_mount(mnt, nd, nd->mnt->mnt_flags|MNT_SHRINKABLE, &nfs_automount_list); + err = do_add_mount(mnt, nd, nd->path.mnt->mnt_flags|MNT_SHRINKABLE, + &nfs_automount_list); if (err < 0) { mntput(mnt); if (err == -EBUSY) goto out_follow; goto out_err; } - mntput(nd->mnt); - dput(nd->dentry); - nd->mnt = mnt; - nd->dentry = dget(mnt->mnt_root); + mntput(nd->path.mnt); + dput(nd->path.dentry); + nd->path.mnt = mnt; + nd->path.dentry = dget(mnt->mnt_root); schedule_delayed_work(&nfs_automount_task, nfs_mountpoint_expiry_timeout); out: dprintk("%s: done, returned %d\n", __FUNCTION__, err); @@ -149,7 +151,8 @@ out_err: path_release(nd); goto out; out_follow: - while(d_mountpoint(nd->dentry) && follow_down(&nd->mnt, &nd->dentry)) + while (d_mountpoint(nd->path.dentry) && + follow_down(&nd->path.mnt, &nd->path.dentry)) ; err = 0; goto out; diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 027e109..7ce0786 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1384,11 +1384,11 @@ out_close: struct dentry * nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd) { - struct dentry *parent; struct path path = { - .mnt = nd->mnt, + .mnt = nd->path.mnt, .dentry = dentry, }; + struct dentry *parent; struct iattr attr; struct rpc_cred *cred; struct nfs4_state *state; @@ -1433,7 +1433,7 @@ int nfs4_open_revalidate(struct inode *dir, struct dentry *dentry, int openflags, struct nameidata *nd) { struct path path = { - .mnt = nd->mnt, + .mnt = nd->path.mnt, .dentry = dentry, }; struct rpc_cred *cred; @@ -1885,7 +1885,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr, int flags, struct nameidata *nd) { struct path path = { - .mnt = nd->mnt, + .mnt = nd->path.mnt, .dentry = dentry, }; struct nfs4_state *state; -- cgit v1.1 From 1d957f9bf87da74f420424d16ece005202bbebd3 Mon Sep 17 00:00:00 2001 From: Jan Blunck Date: Thu, 14 Feb 2008 19:34:35 -0800 Subject: Introduce path_put() * Add path_put() functions for releasing a reference to the dentry and vfsmount of a struct path in the right order * Switch from path_release(nd) to path_put(&nd->path) * Rename dput_path() to path_put_conditional() [akpm@linux-foundation.org: fix cifs] Signed-off-by: Jan Blunck Signed-off-by: Andreas Gruenbacher Acked-by: Christoph Hellwig Cc: Cc: Al Viro Cc: Steven French Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/nfs/namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/nfs') diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c index 3b6d83d..607f6eb 100644 --- a/fs/nfs/namespace.c +++ b/fs/nfs/namespace.c @@ -148,7 +148,7 @@ out: dprintk("<-- nfs_follow_mountpoint() = %d\n", err); return ERR_PTR(err); out_err: - path_release(nd); + path_put(&nd->path); goto out; out_follow: while (d_mountpoint(nd->path.dentry) && -- cgit v1.1