Skip to contentSkip to conversation

nfs: Fix a memory leak when meeting an unsupported state protect

PATCH In review Kinglong Mee 2 participants 11 messages Jul 1, 2015

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Kinglong Mee

author
#0 lore raw

PATCH 1/8 nfs: Fix a memory leak when meeting an unsupported state protect

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/nfs4proc.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
fs/nfs/nfs4proc.c +8 -6
index 55e1e3a..f1e0843 100644
@@ -6885,7 +6885,7 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 		/* unsupported! */
 		WARN_ON_ONCE(1);
 		status = -EINVAL;
-		goto out_server_scope;
+		goto out_impl_id;
 	}
 
 	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);

@@ -6913,6 +6913,7 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 		/* use the most recent implementation id */
 		kfree(clp->cl_implid);
 		clp->cl_implid = res.impl_id;
+		res.impl_id = NULL;
 
 		if (clp->cl_serverscope != NULL &&
 		    !nfs41_same_server_scope(clp->cl_serverscope,

@@ -6926,15 +6927,16 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 
 		if (clp->cl_serverscope == NULL) {
 			clp->cl_serverscope = res.server_scope;
-			goto out;
+			res.server_scope = NULL;
 		}
-	} else
-		kfree(res.impl_id);
+	}
 
-out_server_owner:
-	kfree(res.server_owner);
+out_impl_id:
+	kfree(res.impl_id);
 out_server_scope:
 	kfree(res.server_scope);
+out_server_owner:
+	kfree(res.server_owner);
 out:
 	if (clp->cl_implid != NULL)
 		dprintk("NFS reply exchange_id: Server Implementation ID: "
Signature · 1 line
2.4.3
author
#1 lore raw

PATCH 2/8 nfs: Remove unused argument in nfs_server_set_fsinfo()

Commit e38eb6506f "NFS: set_pnfs_layoutdriver() from nfs4_proc_fsinfo()" have remove the using of mntfh from nfs_server_set_fsinfo().

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/client.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
fs/nfs/client.c +1 -2
index 892aeff..a13fada 100644
@@ -825,7 +825,6 @@ error:
  * Load up the server record from information gained in an fsinfo record
  */
 static void nfs_server_set_fsinfo(struct nfs_server *server,
-				  struct nfs_fh *mntfh,
 				  struct nfs_fsinfo *fsinfo)
 {
 	unsigned long max_rpc_payload;

@@ -901,7 +900,7 @@ int nfs_probe_fsinfo(struct nfs_server *server, struct nfs_fh *mntfh, struct nfs
 	if (error < 0)
 		goto out_error;
 
-	nfs_server_set_fsinfo(server, mntfh, &fsinfo);
+	nfs_server_set_fsinfo(server, &fsinfo);
 
 	/* Get some general file system info */
 	if (server->namelen == 0) {
Signature · 1 line
2.4.3
author
#2 lore raw

PATCH 3/8 nfs: Use remove_proc_subtree() instead remove_proc_entry()

In reply to Kinglong Mee
Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/client.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
fs/nfs/client.c +3 -7
index a13fada..52488ec 100644
@@ -1447,13 +1447,11 @@ int __init nfs_fs_proc_init(void)
 	/* a file of volumes that we have mounted */
 	p = proc_symlink("volumes", proc_fs_nfs, "../../net/nfsfs/volumes");
 	if (!p)
-		goto error_2;
+		goto error_1;
 	return 0;
 
-error_2:
-	remove_proc_entry("servers", proc_fs_nfs);
 error_1:
-	remove_proc_entry("fs/nfsfs", NULL);
+	remove_proc_subtree("fs/nfsfs", NULL);
 error_0:
 	return -ENOMEM;
 }

@@ -1463,9 +1461,7 @@ error_0:
  */
 void nfs_fs_proc_exit(void)
 {
-	remove_proc_entry("volumes", proc_fs_nfs);
-	remove_proc_entry("servers", proc_fs_nfs);
-	remove_proc_entry("fs/nfsfs", NULL);
+	remove_proc_subtree("fs/nfsfs", NULL);
 }
 
 #endif /* CONFIG_PROC_FS */
Signature · 1 line
2.4.3
author
#6 lore raw

PATCH 4/8 nfs: Don't setting FILE_CREATED flags always

In reply to Kinglong Mee

Commit 5bc2afc2b5 "NFSv4: Honour the 'opened' parameter in the atomic_open()
filesystem method" have support the opened arguments now.

Also,
Commit 03da633aa7 "atomic_open: take care of EEXIST in no-open case with
O_CREAT|O_EXCL in fs/namei.c" have change vfs's logical.

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/dir.c | 3 ---
 1 file changed, 3 deletions(-)
fs/nfs/dir.c +0 -3
index b2c8b31..21457bb 100644
@@ -1470,9 +1470,6 @@ static int nfs_finish_open(struct nfs_open_context *ctx,
 {
 	int err;
 
-	if ((open_flags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))
-		*opened |= FILE_CREATED;
-
 	err = finish_open(file, dentry, do_open, opened);
 	if (err)
 		goto out;
Signature · 1 line
2.4.3
author
#7 lore raw

PATCH 5/8 nfs: Remove unneeded micro checking of CONFIG_PROC_FS

In reply to Kinglong Mee

Have checking CONFIG_PROC_FS in include/linux/sunrpc/stats.h.

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/inode.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
fs/nfs/inode.c +3 -7
index f734562..d1d2b11 100644
@@ -2008,17 +2008,15 @@ static int __init init_nfs_fs(void)
 	if (err)
 		goto out1;
 
-#ifdef CONFIG_PROC_FS
 	rpc_proc_register(&init_net, &nfs_rpcstat);
-#endif
-	if ((err = register_nfs_fs()) != 0)
+
+	err = register_nfs_fs();
+	if (err)
 		goto out0;
 
 	return 0;
 out0:
-#ifdef CONFIG_PROC_FS
 	rpc_proc_unregister(&init_net, "nfs");
-#endif
 	nfs_destroy_directcache();
 out1:
 	nfs_destroy_writepagecache();

@@ -2049,9 +2047,7 @@ static void __exit exit_nfs_fs(void)
 	nfs_destroy_nfspagecache();
 	nfs_fscache_unregister();
 	unregister_pernet_subsys(&nfs_net_ops);
-#ifdef CONFIG_PROC_FS
 	rpc_proc_unregister(&init_net, "nfs");
-#endif
 	unregister_nfs_fs();
 	nfs_fs_proc_exit();
 	nfsiod_stop();
Signature · 1 line
2.4.3
author
#8 lore raw

PATCH 6/8 nfs: Drop bad comment in nfs41_walk_client_list()

In reply to Kinglong Mee

Commit 7b1f1fd184 "NFSv4/4.1: Fix bugs in nfs4[01]_walk_client_list" have change the logical of the list_for_each_entry().

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/nfs4client.c | 1 -
 1 file changed, 1 deletion(-)
fs/nfs/nfs4client.c +0 -1
index e42be52..3aa6a9b 100644
@@ -676,7 +676,6 @@ found:
 		break;
 	}
 
-	/* No matching nfs_client found. */
 	spin_unlock(&nn->nfs_client_lock);
 	dprintk("NFS: <-- %s status = %d\n", __func__, status);
 	nfs_put_client(prev);
Signature · 1 line
2.4.3
author
#9 lore raw

PATCH 7/8 nfs: Remove invalid NFS_ATTR_FATTR_V4_REFERRAL checking in nfs4_get_rootfh

In reply to Kinglong Mee

NFS_ATTR_FATTR_V4_REFERRAL is only set in nfs4_proc_lookup_common.

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/nfs4getroot.c | 7 -------
 1 file changed, 7 deletions(-)
fs/nfs/nfs4getroot.c +0 -7
index c0b3a16..039b3eb 100644
@@ -35,13 +35,6 @@ int nfs4_get_rootfh(struct nfs_server *server, struct nfs_fh *mntfh, bool auth_p
 		goto out;
 	}
 
-	if (fsinfo.fattr->valid & NFS_ATTR_FATTR_V4_REFERRAL) {
-		printk(KERN_ERR "nfs4_get_rootfh:"
-		       " getroot obtained referral\n");
-		ret = -EREMOTE;
-		goto out;
-	}
-
 	memcpy(&server->fsid, &fsinfo.fattr->fsid, sizeof(server->fsid));
 out:
 	nfs_free_fattr(fsinfo.fattr);
Signature · 1 line
2.4.3
author
#10 lore raw

PATCH 8/8 nfs: Remove invalid tk_pid from debug message

In reply to Kinglong Mee

Before rpc_run_task(), tk_pid is uninitiated as 0 always.

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/nfs4proc.c | 5 ++---
 fs/nfs/pagelist.c | 3 +--
 fs/nfs/write.c    | 2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)
fs/nfs/nfs4proc.c +2 -3
index f1e0843..c3bb09f 100644
@@ -8063,9 +8063,8 @@ nfs4_proc_layoutcommit(struct nfs4_layoutcommit_data *data, bool sync)
 	struct rpc_task *task;
 	int status = 0;
 
-	dprintk("NFS: %4d initiating layoutcommit call. sync %d "
-		"lbw: %llu inode %lu\n",
-		data->task.tk_pid, sync,
+	dprintk("NFS: initiating layoutcommit call. sync %d "
+		"lbw: %llu inode %lu\n", sync,
 		data->args.lastbytewritten,
 		data->args.inode->i_ino);
fs/nfs/pagelist.c +1 -2
index 282b393..776f8f8 100644
@@ -636,9 +636,8 @@ int nfs_initiate_pgio(struct rpc_clnt *clnt, struct nfs_pgio_header *hdr,
 
 	hdr->rw_ops->rw_initiate(hdr, &msg, rpc_ops, &task_setup_data, how);
 
-	dprintk("NFS: %5u initiated pgio call "
+	dprintk("NFS: initiated pgio call "
 		"(req %s/%llu, %u bytes @ offset %llu)\n",
-		hdr->task.tk_pid,
 		hdr->inode->i_sb->s_id,
 		(unsigned long long)NFS_FILEID(hdr->inode),
 		hdr->args.count,
fs/nfs/write.c +1 -1
index e6c2625..9354980 100644
@@ -1556,7 +1556,7 @@ int nfs_initiate_commit(struct rpc_clnt *clnt, struct nfs_commit_data *data,
 	/* Set up the initial task struct.  */
 	nfs_ops->commit_setup(data, &msg);
 
-	dprintk("NFS: %5u initiated commit call\n", data->task.tk_pid);
+	dprintk("NFS: initiated commit call\n");
 
 	nfs4_state_protect(NFS_SERVER(data->inode)->nfs_client,
 		NFS_SP4_MACH_CRED_COMMIT, &task_setup_data.rpc_client, &msg);
Signature · 1 line
2.4.3
#3 lore raw

Re: [PATCH 3/8] nfs: Use remove_proc_subtree() instead remove_proc_entry()

In reply to Kinglong Mee

fs/nfs/client.c int __init nfs_fs_proc_init(void) · line 1448

Quoted 12 lines · Kinglong Mee
Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/client.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
fs/nfs/client.c +0 -0
index a13fada..52488ec 100644
@@ -1447,13 +1447,11 @@ int __init nfs_fs_proc_init(void)
 	/* a file of volumes that we have mounted */
 	p = proc_symlink("volumes", proc_fs_nfs, "../../net/nfsfs/volumes");

Why not make that

	if (!proc_symlink("fs/nfs/volumes", NULL, "../../net/nfsfs/volumes"))

instead, and kill proc_fs_nfs completely?

author
#4 lore raw

Re: [PATCH 3/8] nfs: Use remove_proc_subtree() instead remove_proc_entry()

Quoted 15 lines · Al Viro

fs/nfs/client.c int __init nfs_fs_proc_init(void) · line 1448

Quoted 12 lines · Kinglong Mee
Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/client.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
fs/nfs/client.c +0 -0
index a13fada..52488ec 100644
@@ -1447,13 +1447,11 @@ int __init nfs_fs_proc_init(void)
 	/* a file of volumes that we have mounted */
 	p = proc_symlink("volumes", proc_fs_nfs, "../../net/nfsfs/volumes");

Why not make that

Quoted 2 lines
	if (!proc_symlink("fs/nfs/volumes", NULL, "../../net/nfsfs/volumes"))

instead, and kill proc_fs_nfs completely?

That's great. Thank you!

thanks,
Kinglong Mee

author
#5 lore raw

PATCH v2 3/8 nfs: Use remove_proc_subtree() instead remove_proc_entry()

In reply to Al Viro

Thanks for Al Viro's comments of killing proc_fs_nfs completely.

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfs/client.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)
fs/nfs/client.c +7 -18
index 892aeff..3e8781d 100644
@@ -1193,8 +1193,6 @@ void nfs_clients_init(struct net *net)
 }
 
 #ifdef CONFIG_PROC_FS
-static struct proc_dir_entry *proc_fs_nfs;
-
 static int nfs_server_list_open(struct inode *inode, struct file *file);
 static void *nfs_server_list_start(struct seq_file *p, loff_t *pos);
 static void *nfs_server_list_next(struct seq_file *p, void *v, loff_t *pos);

@@ -1434,27 +1432,20 @@ void nfs_fs_proc_net_exit(struct net *net)
  */
 int __init nfs_fs_proc_init(void)
 {
-	struct proc_dir_entry *p;
-
-	proc_fs_nfs = proc_mkdir("fs/nfsfs", NULL);
-	if (!proc_fs_nfs)
+	if (!proc_mkdir("fs/nfsfs", NULL))
 		goto error_0;
 
 	/* a file of servers with which we're dealing */
-	p = proc_symlink("servers", proc_fs_nfs, "../../net/nfsfs/servers");
-	if (!p)
+	if (!proc_symlink("fs/nfsfs/servers", NULL, "../../net/nfsfs/servers"))
 		goto error_1;
 
 	/* a file of volumes that we have mounted */
-	p = proc_symlink("volumes", proc_fs_nfs, "../../net/nfsfs/volumes");
-	if (!p)
-		goto error_2;
-	return 0;
+	if (!proc_symlink("fs/nfsfs/volumes", NULL, "../../net/nfsfs/volumes"))
+		goto error_1;
 
-error_2:
-	remove_proc_entry("servers", proc_fs_nfs);
+	return 0;
 error_1:
-	remove_proc_entry("fs/nfsfs", NULL);
+	remove_proc_subtree("fs/nfsfs", NULL);
 error_0:
 	return -ENOMEM;
 }

@@ -1464,9 +1455,7 @@ error_0:
  */
 void nfs_fs_proc_exit(void)
 {
-	remove_proc_entry("volumes", proc_fs_nfs);
-	remove_proc_entry("servers", proc_fs_nfs);
-	remove_proc_entry("fs/nfsfs", NULL);
+	remove_proc_subtree("fs/nfsfs", NULL);
 }
 
 #endif /* CONFIG_PROC_FS */
Signature · 1 line
2.4.3