[PATCH 0/4] NFS: make internal list network namespace aware

[PATCH 0/4] NFS: make internal list network namespace aware

[Stanislav Kinsbursky]

This patch set moves static internal lists of NFS clients and NFS servers, as
well as static IDR's structure and lock, protecting all that stuff, to network
namespace context.

The following series consists of:

---

Stanislav Kinsbursky (4):
      NFS: make nfs_client_list per net ns
      NFS: make nfs_volume_list per net ns
      NFS: make cb_ident_idr per net ns
      NFS: make nfs_client_lock per net ns


 fs/nfs/callback_xdr.c |    2 -
 fs/nfs/client.c       |  137 +++++++++++++++++++++++++++++++------------------
 fs/nfs/idmap.c        |    9 +--
 fs/nfs/inode.c        |    3 +
 fs/nfs/internal.h     |    9 +--
 fs/nfs/netns.h        |    6 ++
 6 files changed, 104 insertions(+), 62 deletions(-)

[PATCH 1/4] NFS: make nfs_client_list per net ns

[Stanislav Kinsbursky]

This patch splits global list of NFS clients into per-net-ns array of lists.
This looks more strict and clearer.
BTW, this patch also makes "/proc/fs/nfsfs/servers" entry content depends on
/proc mount owner pid namespace. See below for details.

NOTE: few words about how was /proc/fs/nfsfs/ entries content show per network
namespace done. This is a little bit tricky and not the best is could be. But
it's cheap (proper fix for /proc conteinerization is a hard nut to crack).
The idea is simple: take proper network namespace from pid namespace
child reaper nsproxy of /proc/ mount creator.
This actually means, that if there are 2 containers with different net
namespace sharing pid namespace, then read of /proc/fs/nfsfs/ entries will
always return content, taken from net namespace of pid namespace creator task
(and thus second namespace set wil be unvisible).

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>

---
 fs/nfs/client.c   |   38 +++++++++++++++++++++++++++-----------
 fs/nfs/idmap.c    |    5 ++---
 fs/nfs/inode.c    |    1 +
 fs/nfs/internal.h |    2 +-
 fs/nfs/netns.h    |    1 +
 5 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 98af1cb..058eb9b 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -39,6 +39,8 @@
 #include <net/ipv6.h>
 #include <linux/nfs_xdr.h>
 #include <linux/sunrpc/bc_xprt.h>
+#include <linux/nsproxy.h>
+#include <linux/pid_namespace.h>
 
 #include <asm/system.h>
 
@@ -49,11 +51,11 @@
 #include "internal.h"
 #include "fscache.h"
 #include "pnfs.h"
+#include "netns.h"
 
 #define NFSDBG_FACILITY		NFSDBG_CLIENT
 
 DEFINE_SPINLOCK(nfs_client_lock);
-LIST_HEAD(nfs_client_list);
 static LIST_HEAD(nfs_volume_list);
 static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
 #ifdef CONFIG_NFS_V4
@@ -464,8 +466,9 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
 {
 	struct nfs_client *clp;
 	const struct sockaddr *sap = data->addr;
+	struct nfs_net *nn = net_generic(data->net, nfs_net_id);
 
-	list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
+	list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
 	        const struct sockaddr *clap = (struct sockaddr *)&clp->cl_addr;
 		/* Don't match clients that failed to initialise properly */
 		if (clp->cl_cons_state < 0)
@@ -483,9 +486,6 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
 		/* Match the full socket address */
 		if (!nfs_sockaddr_cmp(sap, clap))
 			continue;
-		/* Match network namespace */
-		if (clp->net != data->net)
-			continue;
 
 		atomic_inc(&clp->cl_count);
 		return clp;
@@ -506,6 +506,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
 {
 	struct nfs_client *clp, *new = NULL;
 	int error;
+	struct nfs_net *nn = net_generic(cl_init->net, nfs_net_id);
 
 	dprintk("--> nfs_get_client(%s,v%u)\n",
 		cl_init->hostname ?: "", cl_init->rpc_ops->version);
@@ -531,7 +532,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
 	/* install a new client and return with it unready */
 install_client:
 	clp = new;
-	list_add(&clp->cl_share_link, &nfs_client_list);
+	list_add(&clp->cl_share_link, &nn->nfs_client_list);
 	spin_unlock(&nfs_client_lock);
 
 	error = cl_init->rpc_ops->init_client(clp, timeparms, ip_addr,
@@ -1227,9 +1228,10 @@ nfs4_find_client_sessionid(const struct sockaddr *addr,
 			   struct nfs4_sessionid *sid)
 {
 	struct nfs_client *clp;
+	struct nfs_net *nn = net_generic(&init_net, nfs_net_id);
 
 	spin_lock(&nfs_client_lock);
-	list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
+	list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
 		if (nfs4_cb_match_client(addr, clp, 1) == false)
 			continue;
 
@@ -1757,6 +1759,13 @@ out_free_server:
 	return ERR_PTR(error);
 }
 
+void nfs_clients_init(struct net *net)
+{
+	struct nfs_net *nn = net_generic(net, nfs_net_id);
+
+	INIT_LIST_HEAD(&nn->nfs_client_list);
+}
+
 #ifdef CONFIG_PROC_FS
 static struct proc_dir_entry *proc_fs_nfs;
 
@@ -1810,13 +1819,15 @@ static int nfs_server_list_open(struct inode *inode, struct file *file)
 {
 	struct seq_file *m;
 	int ret;
+	struct pid_namespace *pid_ns = file->f_dentry->d_sb->s_fs_info;
+	struct net *net = pid_ns->child_reaper->nsproxy->net_ns;
 
 	ret = seq_open(file, &nfs_server_list_ops);
 	if (ret < 0)
 		return ret;
 
 	m = file->private_data;
-	m->private = PDE(inode)->data;
+	m->private = net;
 
 	return 0;
 }
@@ -1826,9 +1837,11 @@ static int nfs_server_list_open(struct inode *inode, struct file *file)
  */
 static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos)
 {
+	struct nfs_net *nn = net_generic(m->private, nfs_net_id);
+
 	/* lock the list against modification */
 	spin_lock(&nfs_client_lock);
-	return seq_list_start_head(&nfs_client_list, *_pos);
+	return seq_list_start_head(&nn->nfs_client_list, *_pos);
 }
 
 /*
@@ -1836,7 +1849,9 @@ static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos)
  */
 static void *nfs_server_list_next(struct seq_file *p, void *v, loff_t *pos)
 {
-	return seq_list_next(v, &nfs_client_list, pos);
+	struct nfs_net *nn = net_generic(p->private, nfs_net_id);
+
+	return seq_list_next(v, &nn->nfs_client_list, pos);
 }
 
 /*
@@ -1853,9 +1868,10 @@ static void nfs_server_list_stop(struct seq_file *p, void *v)
 static int nfs_server_list_show(struct seq_file *m, void *v)
 {
 	struct nfs_client *clp;
+	struct nfs_net *nn = net_generic(m->private, nfs_net_id);
 
 	/* display header on line 1 */
-	if (v == &nfs_client_list) {
+	if (v == &nn->nfs_client_list) {
 		seq_puts(m, "NV SERVER   PORT USE HOSTNAME\n");
 		return 0;
 	}
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index ff084d2..92deaf8 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -571,13 +571,12 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
 			    void *ptr)
 {
 	struct super_block *sb = ptr;
+	struct nfs_net *nn = net_generic(sb->s_fs_info, nfs_net_id);
 	struct nfs_client *clp;
 	int error = 0;
 
 	spin_lock(&nfs_client_lock);
-	list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
-		if (clp->net != sb->s_fs_info)
-			continue;
+	list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
 		if (clp->rpc_ops != &nfs_v4_clientops)
 			continue;
 		error = __rpc_pipefs_event(clp, event, sb);
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index d2c760e..e8f81e5 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1558,6 +1558,7 @@ EXPORT_SYMBOL_GPL(nfs_net_id);
 
 static int nfs_net_init(struct net *net)
 {
+	nfs_clients_init(net);
 	return nfs_dns_resolver_cache_init(net);
 }
 
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index cdb121d..a9ae806 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -146,6 +146,7 @@ extern void nfs_umount(const struct nfs_mount_request *info);
 
 /* client.c */
 extern const struct rpc_program nfs_program;
+extern void nfs_clients_init(struct net *net);
 
 extern void nfs_cleanup_cb_ident_idr(void);
 extern void nfs_put_client(struct nfs_client *);
@@ -183,7 +184,6 @@ static inline void nfs_fs_proc_exit(void)
 #endif
 #ifdef CONFIG_NFS_V4
 extern spinlock_t nfs_client_lock;
-extern struct list_head nfs_client_list;
 #endif
 
 /* nfs4namespace.c */
diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
index 39ae4ca..feb33c3 100644
--- a/fs/nfs/netns.h
+++ b/fs/nfs/netns.h
@@ -7,6 +7,7 @@
 struct nfs_net {
 	struct cache_detail *nfs_dns_resolve;
 	struct rpc_pipe *bl_device_pipe;
+	struct list_head nfs_client_list;
 };
 
 extern int nfs_net_id;

[PATCH 2/4] NFS: make nfs_volume_list per net ns

[Stanislav Kinsbursky]

This patch splits global list of NFS servers into per-net-ns array of lists.
This looks more strict and clearer.
BTW, this patch also makes "/proc/fs/nfsfs/volumes" content depends on /proc
mount owner pid namespace. See below for details.

NOTE: few words about how was /proc/fs/nfsfs/ entries content show per network
namespace done. This is a little bit tricky and not the best is could be. But
it's cheap (proper fix for /proc conteinerization is a hard nut to crack).
The idea is simple: take proper network namespace from pid namespace
child reaper nsproxy of /proc/ mount creator.
This actually means, that if there are 2 containers with different net
namespace sharing pid namespace, then read of /proc/fs/nfsfs/ entries will
always return content, taken from net namespace of pid namespace creator task
(and thus second namespace set wil be unvisible).

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>

---
 fs/nfs/client.c |   20 ++++++++++++++------
 fs/nfs/netns.h  |    1 +
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 058eb9b..d58e838 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -56,7 +56,6 @@
 #define NFSDBG_FACILITY		NFSDBG_CLIENT
 
 DEFINE_SPINLOCK(nfs_client_lock);
-static LIST_HEAD(nfs_volume_list);
 static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
 #ifdef CONFIG_NFS_V4
 static DEFINE_IDR(cb_ident_idr); /* Protected by nfs_client_lock */
@@ -1036,10 +1035,11 @@ static void nfs_server_copy_userdata(struct nfs_server *target, struct nfs_serve
 static void nfs_server_insert_lists(struct nfs_server *server)
 {
 	struct nfs_client *clp = server->nfs_client;
+	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
 
 	spin_lock(&nfs_client_lock);
 	list_add_tail_rcu(&server->client_link, &clp->cl_superblocks);
-	list_add_tail(&server->master_link, &nfs_volume_list);
+	list_add_tail(&server->master_link, &nn->nfs_volume_list);
 	clear_bit(NFS_CS_STOP_RENEW, &clp->cl_res_state);
 	spin_unlock(&nfs_client_lock);
 
@@ -1764,6 +1764,7 @@ void nfs_clients_init(struct net *net)
 	struct nfs_net *nn = net_generic(net, nfs_net_id);
 
 	INIT_LIST_HEAD(&nn->nfs_client_list);
+	INIT_LIST_HEAD(&nn->nfs_volume_list);
 }
 
 #ifdef CONFIG_PROC_FS
@@ -1900,13 +1901,15 @@ static int nfs_volume_list_open(struct inode *inode, struct file *file)
 {
 	struct seq_file *m;
 	int ret;
+	struct pid_namespace *pid_ns = file->f_dentry->d_sb->s_fs_info;
+	struct net *net = pid_ns->child_reaper->nsproxy->net_ns;
 
 	ret = seq_open(file, &nfs_volume_list_ops);
 	if (ret < 0)
 		return ret;
 
 	m = file->private_data;
-	m->private = PDE(inode)->data;
+	m->private = net;
 
 	return 0;
 }
@@ -1916,9 +1919,11 @@ static int nfs_volume_list_open(struct inode *inode, struct file *file)
  */
 static void *nfs_volume_list_start(struct seq_file *m, loff_t *_pos)
 {
+	struct nfs_net *nn = net_generic(m->private, nfs_net_id);
+
 	/* lock the list against modification */
 	spin_lock(&nfs_client_lock);
-	return seq_list_start_head(&nfs_volume_list, *_pos);
+	return seq_list_start_head(&nn->nfs_volume_list, *_pos);
 }
 
 /*
@@ -1926,7 +1931,9 @@ static void *nfs_volume_list_start(struct seq_file *m, loff_t *_pos)
  */
 static void *nfs_volume_list_next(struct seq_file *p, void *v, loff_t *pos)
 {
-	return seq_list_next(v, &nfs_volume_list, pos);
+	struct nfs_net *nn = net_generic(p->private, nfs_net_id);
+
+	return seq_list_next(v, &nn->nfs_volume_list, pos);
 }
 
 /*
@@ -1945,9 +1952,10 @@ static int nfs_volume_list_show(struct seq_file *m, void *v)
 	struct nfs_server *server;
 	struct nfs_client *clp;
 	char dev[8], fsid[17];
+	struct nfs_net *nn = net_generic(m->private, nfs_net_id);
 
 	/* display header on line 1 */
-	if (v == &nfs_volume_list) {
+	if (v == &nn->nfs_volume_list) {
 		seq_puts(m, "NV SERVER   PORT DEV     FSID              FSC\n");
 		return 0;
 	}
diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
index feb33c3..0fbd4e0 100644
--- a/fs/nfs/netns.h
+++ b/fs/nfs/netns.h
@@ -8,6 +8,7 @@ struct nfs_net {
 	struct cache_detail *nfs_dns_resolve;
 	struct rpc_pipe *bl_device_pipe;
 	struct list_head nfs_client_list;
+	struct list_head nfs_volume_list;
 };
 
 extern int nfs_net_id;

[PATCH 3/4] NFS: make cb_ident_idr per net ns

[Stanislav Kinsbursky]

This patch makes ID's infrastructure network namespace aware. This was done
mainly because of nfs_client_lock, which is desired to be per network
namespace, but protects NFS clients ID's.

NOTE: NFS client's net pointer have to be set prior to ID initialization,
proper assignment was moved.

Signed-off-by: Stanislav Kinsbursky <skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

---
 fs/nfs/callback_xdr.c |    2 +-
 fs/nfs/client.c       |   28 ++++++++++++++++++----------
 fs/nfs/inode.c        |    2 +-
 fs/nfs/internal.h     |    4 ++--
 fs/nfs/netns.h        |    3 +++
 5 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index d50b274..f2be3e1e 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -876,7 +876,7 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp, void *argp, void *r
 		return rpc_garbage_args;
 
 	if (hdr_arg.minorversion == 0) {
-		cps.clp = nfs4_find_client_ident(hdr_arg.cb_ident);
+		cps.clp = nfs4_find_client_ident(rqstp->rq_xprt->xpt_net, hdr_arg.cb_ident);
 		if (!cps.clp || !check_gss_callback_principal(cps.clp, rqstp))
 			return rpc_drop_reply;
 	}
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index d58e838..f51b279 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -58,7 +58,6 @@
 DEFINE_SPINLOCK(nfs_client_lock);
 static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
 #ifdef CONFIG_NFS_V4
-static DEFINE_IDR(cb_ident_idr); /* Protected by nfs_client_lock */
 
 /*
  * Get a unique NFSv4.0 callback identifier which will be used
@@ -67,14 +66,15 @@ static DEFINE_IDR(cb_ident_idr); /* Protected by nfs_client_lock */
 static int nfs_get_cb_ident_idr(struct nfs_client *clp, int minorversion)
 {
 	int ret = 0;
+	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
 
 	if (clp->rpc_ops->version != 4 || minorversion != 0)
 		return ret;
 retry:
-	if (!idr_pre_get(&cb_ident_idr, GFP_KERNEL))
+	if (!idr_pre_get(&nn->cb_ident_idr, GFP_KERNEL))
 		return -ENOMEM;
 	spin_lock(&nfs_client_lock);
-	ret = idr_get_new(&cb_ident_idr, clp, &clp->cl_cb_ident);
+	ret = idr_get_new(&nn->cb_ident_idr, clp, &clp->cl_cb_ident);
 	spin_unlock(&nfs_client_lock);
 	if (ret == -EAGAIN)
 		goto retry;
@@ -173,6 +173,7 @@ static struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_
 	clp->cl_rpcclient = ERR_PTR(-EINVAL);
 
 	clp->cl_proto = cl_init->proto;
+	clp->net = cl_init->net;
 
 #ifdef CONFIG_NFS_V4
 	err = nfs_get_cb_ident_idr(clp, cl_init->minorversion);
@@ -191,7 +192,6 @@ static struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_
 	if (!IS_ERR(cred))
 		clp->cl_machine_cred = cred;
 	nfs_fscache_get_client_cookie(clp);
-	clp->net = cl_init->net;
 
 	return clp;
 
@@ -236,16 +236,20 @@ static void nfs4_shutdown_client(struct nfs_client *clp)
 }
 
 /* idr_remove_all is not needed as all id's are removed by nfs_put_client */
-void nfs_cleanup_cb_ident_idr(void)
+void nfs_cleanup_cb_ident_idr(struct net *net)
 {
-	idr_destroy(&cb_ident_idr);
+	struct nfs_net *nn = net_generic(net, nfs_net_id);
+
+	idr_destroy(&nn->cb_ident_idr);
 }
 
 /* nfs_client_lock held */
 static void nfs_cb_idr_remove_locked(struct nfs_client *clp)
 {
+	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
+
 	if (clp->cl_cb_ident)
-		idr_remove(&cb_ident_idr, clp->cl_cb_ident);
+		idr_remove(&nn->cb_ident_idr, clp->cl_cb_ident);
 }
 
 static void pnfs_init_server(struct nfs_server *server)
@@ -263,7 +267,7 @@ static void nfs4_shutdown_client(struct nfs_client *clp)
 {
 }
 
-void nfs_cleanup_cb_ident_idr(void)
+void nfs_cleanup_cb_ident_idr(struct net *net)
 {
 }
 
@@ -1203,12 +1207,13 @@ error:
  * Find a client by callback identifier
  */
 struct nfs_client *
-nfs4_find_client_ident(int cb_ident)
+nfs4_find_client_ident(struct net *net, int cb_ident)
 {
 	struct nfs_client *clp;
+	struct nfs_net *nn = net_generic(net, nfs_net_id);
 
 	spin_lock(&nfs_client_lock);
-	clp = idr_find(&cb_ident_idr, cb_ident);
+	clp = idr_find(&nn->cb_ident_idr, cb_ident);
 	if (clp)
 		atomic_inc(&clp->cl_count);
 	spin_unlock(&nfs_client_lock);
@@ -1765,6 +1770,9 @@ void nfs_clients_init(struct net *net)
 
 	INIT_LIST_HEAD(&nn->nfs_client_list);
 	INIT_LIST_HEAD(&nn->nfs_volume_list);
+#ifdef CONFIG_NFS_V4
+	idr_init(&nn->cb_ident_idr);
+#endif
 }
 
 #ifdef CONFIG_PROC_FS
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index e8f81e5..d245064 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1565,6 +1565,7 @@ static int nfs_net_init(struct net *net)
 static void nfs_net_exit(struct net *net)
 {
 	nfs_dns_resolver_cache_destroy(net);
+	nfs_cleanup_cb_ident_idr(net);
 }
 
 static struct pernet_operations nfs_net_ops = {
@@ -1674,7 +1675,6 @@ static void __exit exit_nfs_fs(void)
 #ifdef CONFIG_PROC_FS
 	rpc_proc_unregister(&init_net, "nfs");
 #endif
-	nfs_cleanup_cb_ident_idr();
 	unregister_nfs_fs();
 	nfs_fs_proc_exit();
 	nfsiod_stop();
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index a9ae806..958fff2 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -148,9 +148,9 @@ extern void nfs_umount(const struct nfs_mount_request *info);
 extern const struct rpc_program nfs_program;
 extern void nfs_clients_init(struct net *net);
 
-extern void nfs_cleanup_cb_ident_idr(void);
+extern void nfs_cleanup_cb_ident_idr(struct net *);
 extern void nfs_put_client(struct nfs_client *);
-extern struct nfs_client *nfs4_find_client_ident(int);
+extern struct nfs_client *nfs4_find_client_ident(struct net *, int);
 extern struct nfs_client *
 nfs4_find_client_sessionid(const struct sockaddr *, struct nfs4_sessionid *);
 extern struct nfs_server *nfs_create_server(
diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
index 0fbd4e0..547cc95 100644
--- a/fs/nfs/netns.h
+++ b/fs/nfs/netns.h
@@ -9,6 +9,9 @@ struct nfs_net {
 	struct rpc_pipe *bl_device_pipe;
 	struct list_head nfs_client_list;
 	struct list_head nfs_volume_list;
+#ifdef CONFIG_NFS_V4
+	struct idr cb_ident_idr; /* Protected by nfs_client_lock */
+#endif
 };
 
 extern int nfs_net_id;

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] NFS: make nfs_client_lock per net ns

[Stanislav Kinsbursky]

This patch makes nfs_clients_lock allocated per network namespace. All items it
protects are already network namespace aware.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>

---
 fs/nfs/client.c   |   51 +++++++++++++++++++++++++++++----------------------
 fs/nfs/idmap.c    |    4 ++--
 fs/nfs/internal.h |    3 ---
 fs/nfs/netns.h    |    1 +
 4 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index f51b279..9e11d29 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -55,7 +55,6 @@
 
 #define NFSDBG_FACILITY		NFSDBG_CLIENT
 
-DEFINE_SPINLOCK(nfs_client_lock);
 static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
 #ifdef CONFIG_NFS_V4
 
@@ -73,9 +72,9 @@ static int nfs_get_cb_ident_idr(struct nfs_client *clp, int minorversion)
 retry:
 	if (!idr_pre_get(&nn->cb_ident_idr, GFP_KERNEL))
 		return -ENOMEM;
-	spin_lock(&nfs_client_lock);
+	spin_lock(&nn->nfs_client_lock);
 	ret = idr_get_new(&nn->cb_ident_idr, clp, &clp->cl_cb_ident);
-	spin_unlock(&nfs_client_lock);
+	spin_unlock(&nn->nfs_client_lock);
 	if (ret == -EAGAIN)
 		goto retry;
 	return ret;
@@ -313,15 +312,18 @@ static void nfs_free_client(struct nfs_client *clp)
  */
 void nfs_put_client(struct nfs_client *clp)
 {
+	struct nfs_net *nn;
+
 	if (!clp)
 		return;
 
 	dprintk("--> nfs_put_client({%d})\n", atomic_read(&clp->cl_count));
+	nn = net_generic(clp->net, nfs_net_id);
 
-	if (atomic_dec_and_lock(&clp->cl_count, &nfs_client_lock)) {
+	if (atomic_dec_and_lock(&clp->cl_count, &nn->nfs_client_lock)) {
 		list_del(&clp->cl_share_link);
 		nfs_cb_idr_remove_locked(clp);
-		spin_unlock(&nfs_client_lock);
+		spin_unlock(&nn->nfs_client_lock);
 
 		BUG_ON(!list_empty(&clp->cl_superblocks));
 
@@ -516,7 +518,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
 
 	/* see if the client already exists */
 	do {
-		spin_lock(&nfs_client_lock);
+		spin_lock(&nn->nfs_client_lock);
 
 		clp = nfs_match_client(cl_init);
 		if (clp)
@@ -524,7 +526,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
 		if (new)
 			goto install_client;
 
-		spin_unlock(&nfs_client_lock);
+		spin_unlock(&nn->nfs_client_lock);
 
 		new = nfs_alloc_client(cl_init);
 	} while (!IS_ERR(new));
@@ -536,7 +538,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
 install_client:
 	clp = new;
 	list_add(&clp->cl_share_link, &nn->nfs_client_list);
-	spin_unlock(&nfs_client_lock);
+	spin_unlock(&nn->nfs_client_lock);
 
 	error = cl_init->rpc_ops->init_client(clp, timeparms, ip_addr,
 					      authflavour, noresvport);
@@ -551,7 +553,7 @@ install_client:
 	 * - make sure it's ready before returning
 	 */
 found_client:
-	spin_unlock(&nfs_client_lock);
+	spin_unlock(&nn->nfs_client_lock);
 
 	if (new)
 		nfs_free_client(new);
@@ -1041,24 +1043,25 @@ static void nfs_server_insert_lists(struct nfs_server *server)
 	struct nfs_client *clp = server->nfs_client;
 	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
 
-	spin_lock(&nfs_client_lock);
+	spin_lock(&nn->nfs_client_lock);
 	list_add_tail_rcu(&server->client_link, &clp->cl_superblocks);
 	list_add_tail(&server->master_link, &nn->nfs_volume_list);
 	clear_bit(NFS_CS_STOP_RENEW, &clp->cl_res_state);
-	spin_unlock(&nfs_client_lock);
+	spin_unlock(&nn->nfs_client_lock);
 
 }
 
 static void nfs_server_remove_lists(struct nfs_server *server)
 {
 	struct nfs_client *clp = server->nfs_client;
+	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
 
-	spin_lock(&nfs_client_lock);
+	spin_lock(&nn->nfs_client_lock);
 	list_del_rcu(&server->client_link);
 	if (clp && list_empty(&clp->cl_superblocks))
 		set_bit(NFS_CS_STOP_RENEW, &clp->cl_res_state);
 	list_del(&server->master_link);
-	spin_unlock(&nfs_client_lock);
+	spin_unlock(&nn->nfs_client_lock);
 
 	synchronize_rcu();
 }
@@ -1212,11 +1215,11 @@ nfs4_find_client_ident(struct net *net, int cb_ident)
 	struct nfs_client *clp;
 	struct nfs_net *nn = net_generic(net, nfs_net_id);
 
-	spin_lock(&nfs_client_lock);
+	spin_lock(&nn->nfs_client_lock);
 	clp = idr_find(&nn->cb_ident_idr, cb_ident);
 	if (clp)
 		atomic_inc(&clp->cl_count);
-	spin_unlock(&nfs_client_lock);
+	spin_unlock(&nn->nfs_client_lock);
 	return clp;
 }
 
@@ -1235,7 +1238,7 @@ nfs4_find_client_sessionid(const struct sockaddr *addr,
 	struct nfs_client *clp;
 	struct nfs_net *nn = net_generic(&init_net, nfs_net_id);
 
-	spin_lock(&nfs_client_lock);
+	spin_lock(&nn->nfs_client_lock);
 	list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
 		if (nfs4_cb_match_client(addr, clp, 1) == false)
 			continue;
@@ -1249,10 +1252,10 @@ nfs4_find_client_sessionid(const struct sockaddr *addr,
 			continue;
 
 		atomic_inc(&clp->cl_count);
-		spin_unlock(&nfs_client_lock);
+		spin_unlock(&nn->nfs_client_lock);
 		return clp;
 	}
-	spin_unlock(&nfs_client_lock);
+	spin_unlock(&nn->nfs_client_lock);
 	return NULL;
 }
 
@@ -1849,7 +1852,7 @@ static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos)
 	struct nfs_net *nn = net_generic(m->private, nfs_net_id);
 
 	/* lock the list against modification */
-	spin_lock(&nfs_client_lock);
+	spin_lock(&nn->nfs_client_lock);
 	return seq_list_start_head(&nn->nfs_client_list, *_pos);
 }
 
@@ -1868,7 +1871,9 @@ static void *nfs_server_list_next(struct seq_file *p, void *v, loff_t *pos)
  */
 static void nfs_server_list_stop(struct seq_file *p, void *v)
 {
-	spin_unlock(&nfs_client_lock);
+	struct nfs_net *nn = net_generic(p->private, nfs_net_id);
+
+	spin_unlock(&nn->nfs_client_lock);
 }
 
 /*
@@ -1930,7 +1935,7 @@ static void *nfs_volume_list_start(struct seq_file *m, loff_t *_pos)
 	struct nfs_net *nn = net_generic(m->private, nfs_net_id);
 
 	/* lock the list against modification */
-	spin_lock(&nfs_client_lock);
+	spin_lock(&nn->nfs_client_lock);
 	return seq_list_start_head(&nn->nfs_volume_list, *_pos);
 }
 
@@ -1949,7 +1954,9 @@ static void *nfs_volume_list_next(struct seq_file *p, void *v, loff_t *pos)
  */
 static void nfs_volume_list_stop(struct seq_file *p, void *v)
 {
-	spin_unlock(&nfs_client_lock);
+	struct nfs_net *nn = net_generic(p->private, nfs_net_id);
+
+	spin_unlock(&nn->nfs_client_lock);
 }
 
 /*
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 92deaf8..aed3d2e 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -575,7 +575,7 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
 	struct nfs_client *clp;
 	int error = 0;
 
-	spin_lock(&nfs_client_lock);
+	spin_lock(&nn->nfs_client_lock);
 	list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
 		if (clp->rpc_ops != &nfs_v4_clientops)
 			continue;
@@ -583,7 +583,7 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
 		if (error)
 			break;
 	}
-	spin_unlock(&nfs_client_lock);
+	spin_unlock(&nn->nfs_client_lock);
 	return error;
 }
 
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 958fff2..b38b733 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -182,9 +182,6 @@ static inline void nfs_fs_proc_exit(void)
 {
 }
 #endif
-#ifdef CONFIG_NFS_V4
-extern spinlock_t nfs_client_lock;
-#endif
 
 /* nfs4namespace.c */
 #ifdef CONFIG_NFS_V4
diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
index 547cc95..7baad89 100644
--- a/fs/nfs/netns.h
+++ b/fs/nfs/netns.h
@@ -12,6 +12,7 @@ struct nfs_net {
 #ifdef CONFIG_NFS_V4
 	struct idr cb_ident_idr; /* Protected by nfs_client_lock */
 #endif
+	spinlock_t nfs_client_lock;
 };
 
 extern int nfs_net_id;

Re: [PATCH 4/4] NFS: make nfs_client_lock per net ns

[Myklebust, Trond]

On Mon, 2012-01-23 at 17:26 +0000, Stanislav Kinsbursky wrote:
> This patch makes nfs_clients_lock allocated per network namespace. All items it
> protects are already network namespace aware.
> 
> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
> 
> ---
>  fs/nfs/client.c   |   51 +++++++++++++++++++++++++++++----------------------
>  fs/nfs/idmap.c    |    4 ++--
>  fs/nfs/internal.h |    3 ---
>  fs/nfs/netns.h    |    1 +
>  4 files changed, 32 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index f51b279..9e11d29 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -551,7 +553,7 @@ install_client:
>  	 * - make sure it's ready before returning
>  	 */
>  found_client:
> -	spin_unlock(&nfs_client_lock);
> +	spin_unlock(&nn->nfs_client_lock);
>  
>  	if (new)
>  		nfs_free_client(new);
> @@ -1041,24 +1043,25 @@ static void nfs_server_insert_lists(struct nfs_server *server)
>  	struct nfs_client *clp = server->nfs_client;
>  	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
>  
> -	spin_lock(&nfs_client_lock);
> +	spin_lock(&nn->nfs_client_lock);
>  	list_add_tail_rcu(&server->client_link, &clp->cl_superblocks);
>  	list_add_tail(&server->master_link, &nn->nfs_volume_list);
>  	clear_bit(NFS_CS_STOP_RENEW, &clp->cl_res_state);
> -	spin_unlock(&nfs_client_lock);
> +	spin_unlock(&nn->nfs_client_lock);
>  
>  }
>  
>  static void nfs_server_remove_lists(struct nfs_server *server)
>  {
>  	struct nfs_client *clp = server->nfs_client;
> +	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
>  
> -	spin_lock(&nfs_client_lock);
> +	spin_lock(&nn->nfs_client_lock);
>  	list_del_rcu(&server->client_link);
>  	if (clp && list_empty(&clp->cl_superblocks))
>  		set_bit(NFS_CS_STOP_RENEW, &clp->cl_res_state);
>  	list_del(&server->master_link);
> -	spin_unlock(&nfs_client_lock);
> +	spin_unlock(&nn->nfs_client_lock);
>  
>  	synchronize_rcu();
>  }

This hunk causes an Oops when nfs_server_remove_lists gets called from
nfs4_create_server(). I've applied the following patch to fix it up.

Cheers
  Trond
8<-------------------------------------------------------------------------
From 5a489156da4fd15dd143f2b21dd9657b97dcef88 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <Trond.Myklebust@netapp.com>
Date: Tue, 7 Feb 2012 00:05:11 -0500
Subject: [PATCH] NFS: Initialise the nfs_net->nfs_client_lock

Ensure that we initialise the nfs_net->nfs_client_lock spinlock.
Also ensure that nfs_server_remove_lists() doesn't try to
dereference server->nfs_client before that is initialised.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 fs/nfs/client.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 1a5cd49..f0dacad 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1055,8 +1055,11 @@ static void nfs_server_insert_lists(struct nfs_server *server)
 static void nfs_server_remove_lists(struct nfs_server *server)
 {
 	struct nfs_client *clp = server->nfs_client;
-	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
+	struct nfs_net *nn;
 
+	if (clp == NULL)
+		return;
+	nn = net_generic(clp->net, nfs_net_id);
 	spin_lock(&nn->nfs_client_lock);
 	list_del_rcu(&server->client_link);
 	if (clp && list_empty(&clp->cl_superblocks))
@@ -1777,6 +1780,7 @@ void nfs_clients_init(struct net *net)
 #ifdef CONFIG_NFS_V4
 	idr_init(&nn->cb_ident_idr);
 #endif
+	spin_lock_init(&nn->nfs_client_lock);
 }
 
 #ifdef CONFIG_PROC_FS
-- 
1.7.7.6



-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 4/4] NFS: make nfs_client_lock per net ns

[Stanislav Kinsbursky]

07.02.2012 17:51, Myklebust, Trond пишет:
> 8<-------------------------------------------------------------------------
>  From 5a489156da4fd15dd143f2b21dd9657b97dcef88 Mon Sep 17 00:00:00 2001
> From: Trond Myklebust<Trond.Myklebust@netapp.com>
> Date: Tue, 7 Feb 2012 00:05:11 -0500
> Subject: [PATCH] NFS: Initialise the nfs_net->nfs_client_lock
>
> Ensure that we initialise the nfs_net->nfs_client_lock spinlock.
> Also ensure that nfs_server_remove_lists() doesn't try to
> dereference server->nfs_client before that is initialised.
>

Sorry.
Patch looks nice. Except one notice below.

> Signed-off-by: Trond Myklebust<Trond.Myklebust@netapp.com>
> Cc: Stanislav Kinsbursky<skinsbursky@parallels.com>
> ---
>   fs/nfs/client.c |    6 +++++-
>   1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 1a5cd49..f0dacad 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -1055,8 +1055,11 @@ static void nfs_server_insert_lists(struct nfs_server *server)
>   static void nfs_server_remove_lists(struct nfs_server *server)
>   {
>   	struct nfs_client *clp = server->nfs_client;
> -	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
> +	struct nfs_net *nn;
>
> +	if (clp == NULL)
> +		return;
> +	nn = net_generic(clp->net, nfs_net_id);
>   	spin_lock(&nn->nfs_client_lock);
>   	list_del_rcu(&server->client_link);
>   	if (clp&&  list_empty(&clp->cl_superblocks))

This check for clp != NULL can be removed.



-- 
Best regards,
Stanislav Kinsbursky

Re: [PATCH 4/4] NFS: make nfs_client_lock per net ns

[Myklebust, Trond]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1944 bytes --]

On Tue, 2012-02-07 at 18:09 +0400, Stanislav Kinsbursky wrote:
> 07.02.2012 17:51, Myklebust, Trond пишет:
> > 8<-------------------------------------------------------------------------
> >  From 5a489156da4fd15dd143f2b21dd9657b97dcef88 Mon Sep 17 00:00:00 2001
> > From: Trond Myklebust<Trond.Myklebust@netapp.com>
> > Date: Tue, 7 Feb 2012 00:05:11 -0500
> > Subject: [PATCH] NFS: Initialise the nfs_net->nfs_client_lock
> >
> > Ensure that we initialise the nfs_net->nfs_client_lock spinlock.
> > Also ensure that nfs_server_remove_lists() doesn't try to
> > dereference server->nfs_client before that is initialised.
> >
> 
> Sorry.
> Patch looks nice. Except one notice below.
> 
> > Signed-off-by: Trond Myklebust<Trond.Myklebust@netapp.com>
> > Cc: Stanislav Kinsbursky<skinsbursky@parallels.com>
> > ---
> >   fs/nfs/client.c |    6 +++++-
> >   1 files changed, 5 insertions(+), 1 deletions(-)
> >
> > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> > index 1a5cd49..f0dacad 100644
> > --- a/fs/nfs/client.c
> > +++ b/fs/nfs/client.c
> > @@ -1055,8 +1055,11 @@ static void nfs_server_insert_lists(struct nfs_server *server)
> >   static void nfs_server_remove_lists(struct nfs_server *server)
> >   {
> >   	struct nfs_client *clp = server->nfs_client;
> > -	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
> > +	struct nfs_net *nn;
> >
> > +	if (clp == NULL)
> > +		return;
> > +	nn = net_generic(clp->net, nfs_net_id);
> >   	spin_lock(&nn->nfs_client_lock);
> >   	list_del_rcu(&server->client_link);
> >   	if (clp&&  list_empty(&clp->cl_superblocks))
> 
> This check for clp != NULL can be removed.
> 

Yep... I'll add that in...

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±û"žØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br	šê+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹\x1e®w¥¢¸?™¨è­Ú&¢)ߢ^[f

Re: [PATCH 4/4] NFS: make nfs_client_lock per net ns

[Bryan Schumaker]

On 02/07/12 09:11, Myklebust, Trond wrote:

> On Tue, 2012-02-07 at 18:09 +0400, Stanislav Kinsbursky wrote:
>> 07.02.2012 17:51, Myklebust, Trond пишет:
>>> 8<-------------------------------------------------------------------------
>>>  From 5a489156da4fd15dd143f2b21dd9657b97dcef88 Mon Sep 17 00:00:00 2001
>>> From: Trond Myklebust<Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
>>> Date: Tue, 7 Feb 2012 00:05:11 -0500
>>> Subject: [PATCH] NFS: Initialise the nfs_net->nfs_client_lock
>>>
>>> Ensure that we initialise the nfs_net->nfs_client_lock spinlock.
>>> Also ensure that nfs_server_remove_lists() doesn't try to
>>> dereference server->nfs_client before that is initialised.
>>>
>>
>> Sorry.
>> Patch looks nice. Except one notice below.
>>
>>> Signed-off-by: Trond Myklebust<Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
>>> Cc: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>> ---
>>>   fs/nfs/client.c |    6 +++++-
>>>   1 files changed, 5 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>>> index 1a5cd49..f0dacad 100644
>>> --- a/fs/nfs/client.c
>>> +++ b/fs/nfs/client.c
>>> @@ -1055,8 +1055,11 @@ static void nfs_server_insert_lists(struct nfs_server *server)
>>>   static void nfs_server_remove_lists(struct nfs_server *server)
>>>   {
>>>   	struct nfs_client *clp = server->nfs_client;
>>> -	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
>>> +	struct nfs_net *nn;
>>>
>>> +	if (clp == NULL)
>>> +		return;
>>> +	nn = net_generic(clp->net, nfs_net_id);
>>>   	spin_lock(&nn->nfs_client_lock);
>>>   	list_del_rcu(&server->client_link);
>>>   	if (clp&&  list_empty(&clp->cl_superblocks))
>>
>> This check for clp != NULL can be removed.
>>
> 
> Yep... I'll add that in...



When I compile Trond's devel branch I get this:

make[1]: Nothing to be done for `all'.
  CHK     include/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  CHK     kernel/config_data.h
  CC [M]  fs/nfs/idmap.o
fs/nfs/idmap.c: In function 'rpc_pipefs_event':
fs/nfs/idmap.c:535:9: error: implicit declaration of function 'net_generic' [-Werror=implicit-function-declaration]
fs/nfs/idmap.c:535:50: error: 'nfs_net_id' undeclared (first use in this function)
fs/nfs/idmap.c:535:50: note: each undeclared identifier is reported only once for each function it appears in
fs/nfs/idmap.c:540:82: error: dereferencing pointer to incomplete type
fs/nfs/idmap.c:540:224: error: dereferencing pointer to incomplete type
cc1: some warnings being treated as errors

make[2]: *** [fs/nfs/idmap.o] Error 1
make[1]: *** [fs/nfs] Error 2
make: *** [fs] Error 2

I bisected it to this patch, probably a missing #include?

- Bryan

> 


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] NFS: make nfs_client_lock per net ns

[Bryan Schumaker]

On 02/07/12 09:30, Bryan Schumaker wrote:

> On 02/07/12 09:11, Myklebust, Trond wrote:
> 
>> On Tue, 2012-02-07 at 18:09 +0400, Stanislav Kinsbursky wrote:
>>> 07.02.2012 17:51, Myklebust, Trond пишет:
>>>> 8<-------------------------------------------------------------------------
>>>>  From 5a489156da4fd15dd143f2b21dd9657b97dcef88 Mon Sep 17 00:00:00 2001
>>>> From: Trond Myklebust<Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
>>>> Date: Tue, 7 Feb 2012 00:05:11 -0500
>>>> Subject: [PATCH] NFS: Initialise the nfs_net->nfs_client_lock
>>>>
>>>> Ensure that we initialise the nfs_net->nfs_client_lock spinlock.
>>>> Also ensure that nfs_server_remove_lists() doesn't try to
>>>> dereference server->nfs_client before that is initialised.
>>>>
>>>
>>> Sorry.
>>> Patch looks nice. Except one notice below.
>>>
>>>> Signed-off-by: Trond Myklebust<Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
>>>> Cc: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>>> ---
>>>>   fs/nfs/client.c |    6 +++++-
>>>>   1 files changed, 5 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>>>> index 1a5cd49..f0dacad 100644
>>>> --- a/fs/nfs/client.c
>>>> +++ b/fs/nfs/client.c
>>>> @@ -1055,8 +1055,11 @@ static void nfs_server_insert_lists(struct nfs_server *server)
>>>>   static void nfs_server_remove_lists(struct nfs_server *server)
>>>>   {
>>>>   	struct nfs_client *clp = server->nfs_client;
>>>> -	struct nfs_net *nn = net_generic(clp->net, nfs_net_id);
>>>> +	struct nfs_net *nn;
>>>>
>>>> +	if (clp == NULL)
>>>> +		return;
>>>> +	nn = net_generic(clp->net, nfs_net_id);
>>>>   	spin_lock(&nn->nfs_client_lock);
>>>>   	list_del_rcu(&server->client_link);
>>>>   	if (clp&&  list_empty(&clp->cl_superblocks))
>>>
>>> This check for clp != NULL can be removed.
>>>
>>
>> Yep... I'll add that in...
> 
> 
> 
> When I compile Trond's devel branch I get this:
> 
> make[1]: Nothing to be done for `all'.
>   CHK     include/linux/version.h
>   CHK     include/generated/utsrelease.h
>   CALL    scripts/checksyscalls.sh
>   CHK     include/generated/compile.h
>   CHK     kernel/config_data.h
>   CC [M]  fs/nfs/idmap.o
> fs/nfs/idmap.c: In function 'rpc_pipefs_event':
> fs/nfs/idmap.c:535:9: error: implicit declaration of function 'net_generic' [-Werror=implicit-function-declaration]
> fs/nfs/idmap.c:535:50: error: 'nfs_net_id' undeclared (first use in this function)
> fs/nfs/idmap.c:535:50: note: each undeclared identifier is reported only once for each function it appears in
> fs/nfs/idmap.c:540:82: error: dereferencing pointer to incomplete type
> fs/nfs/idmap.c:540:224: error: dereferencing pointer to incomplete type
> cc1: some warnings being treated as errors
> 
> make[2]: *** [fs/nfs/idmap.o] Error 1
> make[1]: *** [fs/nfs] Error 2
> make: *** [fs] Error 2
> 
> I bisected it to this patch, probably a missing #include?


Ignore that, I was in the middle of bisecting something else and got confused.  This was still something I kept seeing during the bisect, though.  I'll figure out which patch causes it in a few minutes...

> 
> - Bryan
> 
>>
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] NFS: make nfs_client_list per net ns

[Bryan Schumaker]

I fixed up my bisect and identified this patch instead.  The problem I'm seeing is:

  CC [M]  fs/nfs/delegation.o
  CC [M]  fs/nfs/idmap.o
fs/nfs/idmap.c: In function 'rpc_pipefs_event':
fs/nfs/idmap.c:535:9: error: implicit declaration of function 'net_generic' [-Werror=implicit-function-declaration]
fs/nfs/idmap.c:535:50: error: 'nfs_net_id' undeclared (first use in this function)
fs/nfs/idmap.c:535:50: note: each undeclared identifier is reported only once for each function it appears in
fs/nfs/idmap.c:540:82: error: dereferencing pointer to incomplete type
fs/nfs/idmap.c:540:224: error: dereferencing pointer to incomplete type
cc1: some warnings being treated as errors

make[2]: *** [fs/nfs/idmap.o] Error 1
make[1]: *** [fs/nfs] Error 2
make: *** [fs] Error 2

I think you need to add: #include "netns.h" to idmap.c.

- Bryan

On 01/23/12 12:26, Stanislav Kinsbursky wrote:

> This patch splits global list of NFS clients into per-net-ns array of lists.
> This looks more strict and clearer.
> BTW, this patch also makes "/proc/fs/nfsfs/servers" entry content depends on
> /proc mount owner pid namespace. See below for details.
> 
> NOTE: few words about how was /proc/fs/nfsfs/ entries content show per network
> namespace done. This is a little bit tricky and not the best is could be. But
> it's cheap (proper fix for /proc conteinerization is a hard nut to crack).
> The idea is simple: take proper network namespace from pid namespace
> child reaper nsproxy of /proc/ mount creator.
> This actually means, that if there are 2 containers with different net
> namespace sharing pid namespace, then read of /proc/fs/nfsfs/ entries will
> always return content, taken from net namespace of pid namespace creator task
> (and thus second namespace set wil be unvisible).
> 
> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
> 
> ---
>  fs/nfs/client.c   |   38 +++++++++++++++++++++++++++-----------
>  fs/nfs/idmap.c    |    5 ++---
>  fs/nfs/inode.c    |    1 +
>  fs/nfs/internal.h |    2 +-
>  fs/nfs/netns.h    |    1 +
>  5 files changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 98af1cb..058eb9b 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -39,6 +39,8 @@
>  #include <net/ipv6.h>
>  #include <linux/nfs_xdr.h>
>  #include <linux/sunrpc/bc_xprt.h>
> +#include <linux/nsproxy.h>
> +#include <linux/pid_namespace.h>
>  
>  #include <asm/system.h>
>  
> @@ -49,11 +51,11 @@
>  #include "internal.h"
>  #include "fscache.h"
>  #include "pnfs.h"
> +#include "netns.h"
>  
>  #define NFSDBG_FACILITY		NFSDBG_CLIENT
>  
>  DEFINE_SPINLOCK(nfs_client_lock);
> -LIST_HEAD(nfs_client_list);
>  static LIST_HEAD(nfs_volume_list);
>  static DECLARE_WAIT_QUEUE_HEAD(nfs_client_active_wq);
>  #ifdef CONFIG_NFS_V4
> @@ -464,8 +466,9 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
>  {
>  	struct nfs_client *clp;
>  	const struct sockaddr *sap = data->addr;
> +	struct nfs_net *nn = net_generic(data->net, nfs_net_id);
>  
> -	list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
> +	list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
>  	        const struct sockaddr *clap = (struct sockaddr *)&clp->cl_addr;
>  		/* Don't match clients that failed to initialise properly */
>  		if (clp->cl_cons_state < 0)
> @@ -483,9 +486,6 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
>  		/* Match the full socket address */
>  		if (!nfs_sockaddr_cmp(sap, clap))
>  			continue;
> -		/* Match network namespace */
> -		if (clp->net != data->net)
> -			continue;
>  
>  		atomic_inc(&clp->cl_count);
>  		return clp;
> @@ -506,6 +506,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
>  {
>  	struct nfs_client *clp, *new = NULL;
>  	int error;
> +	struct nfs_net *nn = net_generic(cl_init->net, nfs_net_id);
>  
>  	dprintk("--> nfs_get_client(%s,v%u)\n",
>  		cl_init->hostname ?: "", cl_init->rpc_ops->version);
> @@ -531,7 +532,7 @@ nfs_get_client(const struct nfs_client_initdata *cl_init,
>  	/* install a new client and return with it unready */
>  install_client:
>  	clp = new;
> -	list_add(&clp->cl_share_link, &nfs_client_list);
> +	list_add(&clp->cl_share_link, &nn->nfs_client_list);
>  	spin_unlock(&nfs_client_lock);
>  
>  	error = cl_init->rpc_ops->init_client(clp, timeparms, ip_addr,
> @@ -1227,9 +1228,10 @@ nfs4_find_client_sessionid(const struct sockaddr *addr,
>  			   struct nfs4_sessionid *sid)
>  {
>  	struct nfs_client *clp;
> +	struct nfs_net *nn = net_generic(&init_net, nfs_net_id);
>  
>  	spin_lock(&nfs_client_lock);
> -	list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
> +	list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
>  		if (nfs4_cb_match_client(addr, clp, 1) == false)
>  			continue;
>  
> @@ -1757,6 +1759,13 @@ out_free_server:
>  	return ERR_PTR(error);
>  }
>  
> +void nfs_clients_init(struct net *net)
> +{
> +	struct nfs_net *nn = net_generic(net, nfs_net_id);
> +
> +	INIT_LIST_HEAD(&nn->nfs_client_list);
> +}
> +
>  #ifdef CONFIG_PROC_FS
>  static struct proc_dir_entry *proc_fs_nfs;
>  
> @@ -1810,13 +1819,15 @@ static int nfs_server_list_open(struct inode *inode, struct file *file)
>  {
>  	struct seq_file *m;
>  	int ret;
> +	struct pid_namespace *pid_ns = file->f_dentry->d_sb->s_fs_info;
> +	struct net *net = pid_ns->child_reaper->nsproxy->net_ns;
>  
>  	ret = seq_open(file, &nfs_server_list_ops);
>  	if (ret < 0)
>  		return ret;
>  
>  	m = file->private_data;
> -	m->private = PDE(inode)->data;
> +	m->private = net;
>  
>  	return 0;
>  }
> @@ -1826,9 +1837,11 @@ static int nfs_server_list_open(struct inode *inode, struct file *file)
>   */
>  static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos)
>  {
> +	struct nfs_net *nn = net_generic(m->private, nfs_net_id);
> +
>  	/* lock the list against modification */
>  	spin_lock(&nfs_client_lock);
> -	return seq_list_start_head(&nfs_client_list, *_pos);
> +	return seq_list_start_head(&nn->nfs_client_list, *_pos);
>  }
>  
>  /*
> @@ -1836,7 +1849,9 @@ static void *nfs_server_list_start(struct seq_file *m, loff_t *_pos)
>   */
>  static void *nfs_server_list_next(struct seq_file *p, void *v, loff_t *pos)
>  {
> -	return seq_list_next(v, &nfs_client_list, pos);
> +	struct nfs_net *nn = net_generic(p->private, nfs_net_id);
> +
> +	return seq_list_next(v, &nn->nfs_client_list, pos);
>  }
>  
>  /*
> @@ -1853,9 +1868,10 @@ static void nfs_server_list_stop(struct seq_file *p, void *v)
>  static int nfs_server_list_show(struct seq_file *m, void *v)
>  {
>  	struct nfs_client *clp;
> +	struct nfs_net *nn = net_generic(m->private, nfs_net_id);
>  
>  	/* display header on line 1 */
> -	if (v == &nfs_client_list) {
> +	if (v == &nn->nfs_client_list) {
>  		seq_puts(m, "NV SERVER   PORT USE HOSTNAME\n");
>  		return 0;
>  	}
> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
> index ff084d2..92deaf8 100644
> --- a/fs/nfs/idmap.c
> +++ b/fs/nfs/idmap.c
> @@ -571,13 +571,12 @@ static int rpc_pipefs_event(struct notifier_block *nb, unsigned long event,
>  			    void *ptr)
>  {
>  	struct super_block *sb = ptr;
> +	struct nfs_net *nn = net_generic(sb->s_fs_info, nfs_net_id);
>  	struct nfs_client *clp;
>  	int error = 0;
>  
>  	spin_lock(&nfs_client_lock);
> -	list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
> -		if (clp->net != sb->s_fs_info)
> -			continue;
> +	list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) {
>  		if (clp->rpc_ops != &nfs_v4_clientops)
>  			continue;
>  		error = __rpc_pipefs_event(clp, event, sb);
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index d2c760e..e8f81e5 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -1558,6 +1558,7 @@ EXPORT_SYMBOL_GPL(nfs_net_id);
>  
>  static int nfs_net_init(struct net *net)
>  {
> +	nfs_clients_init(net);
>  	return nfs_dns_resolver_cache_init(net);
>  }
>  
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index cdb121d..a9ae806 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -146,6 +146,7 @@ extern void nfs_umount(const struct nfs_mount_request *info);
>  
>  /* client.c */
>  extern const struct rpc_program nfs_program;
> +extern void nfs_clients_init(struct net *net);
>  
>  extern void nfs_cleanup_cb_ident_idr(void);
>  extern void nfs_put_client(struct nfs_client *);
> @@ -183,7 +184,6 @@ static inline void nfs_fs_proc_exit(void)
>  #endif
>  #ifdef CONFIG_NFS_V4
>  extern spinlock_t nfs_client_lock;
> -extern struct list_head nfs_client_list;
>  #endif
>  
>  /* nfs4namespace.c */
> diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
> index 39ae4ca..feb33c3 100644
> --- a/fs/nfs/netns.h
> +++ b/fs/nfs/netns.h
> @@ -7,6 +7,7 @@
>  struct nfs_net {
>  	struct cache_detail *nfs_dns_resolve;
>  	struct rpc_pipe *bl_device_pipe;
> +	struct list_head nfs_client_list;
>  };
>  
>  extern int nfs_net_id;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] NFS: make nfs_client_list per net ns

[Myklebust, Trond]

On Tue, 2012-02-07 at 10:32 -0500, Bryan Schumaker wrote:
> I fixed up my bisect and identified this patch instead.  The problem I'm seeing is:
> 
>   CC [M]  fs/nfs/delegation.o
>   CC [M]  fs/nfs/idmap.o
> fs/nfs/idmap.c: In function 'rpc_pipefs_event':
> fs/nfs/idmap.c:535:9: error: implicit declaration of function 'net_generic' [-Werror=implicit-function-declaration]
> fs/nfs/idmap.c:535:50: error: 'nfs_net_id' undeclared (first use in this function)
> fs/nfs/idmap.c:535:50: note: each undeclared identifier is reported only once for each function it appears in
> fs/nfs/idmap.c:540:82: error: dereferencing pointer to incomplete type
> fs/nfs/idmap.c:540:224: error: dereferencing pointer to incomplete type
> cc1: some warnings being treated as errors
> 
> make[2]: *** [fs/nfs/idmap.o] Error 1
> make[1]: *** [fs/nfs] Error 2
> make: *** [fs] Error 2
> 
> I think you need to add: #include "netns.h" to idmap.c.
> 
> - Bryan

That should already be there in the 'devel' branch. See commit
17347d03c008e2f504c33bb4905cdad0abc01319.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 1/4] NFS: make nfs_client_list per net ns

[Bryan Schumaker]

On 02/07/12 10:34, Myklebust, Trond wrote:

> On Tue, 2012-02-07 at 10:32 -0500, Bryan Schumaker wrote:
>> I fixed up my bisect and identified this patch instead.  The problem I'm seeing is:
>>
>>   CC [M]  fs/nfs/delegation.o
>>   CC [M]  fs/nfs/idmap.o
>> fs/nfs/idmap.c: In function 'rpc_pipefs_event':
>> fs/nfs/idmap.c:535:9: error: implicit declaration of function 'net_generic' [-Werror=implicit-function-declaration]
>> fs/nfs/idmap.c:535:50: error: 'nfs_net_id' undeclared (first use in this function)
>> fs/nfs/idmap.c:535:50: note: each undeclared identifier is reported only once for each function it appears in
>> fs/nfs/idmap.c:540:82: error: dereferencing pointer to incomplete type
>> fs/nfs/idmap.c:540:224: error: dereferencing pointer to incomplete type
>> cc1: some warnings being treated as errors
>>
>> make[2]: *** [fs/nfs/idmap.o] Error 1
>> make[1]: *** [fs/nfs] Error 2
>> make: *** [fs] Error 2
>>
>> I think you need to add: #include "netns.h" to idmap.c.
>>
>> - Bryan
> 
> That should already be there in the 'devel' branch. See commit
> 17347d03c008e2f504c33bb4905cdad0abc01319.
> 


Yes, but once I fixed this I was able to find the initial problem I was searching for (I have no idea how git led me to the wrong patch on my first attempt).

- Bryan