public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/9] nsproxy: incorporate fs namespace
       [not found] <29vfyljM-1.2006059-s@us.ibm.com>
@ 2006-05-10  2:11 ` Serge E. Hallyn
  2006-05-10 12:46   ` Eric W. Biederman
  0 siblings, 1 reply; 11+ messages in thread
From: Serge E. Hallyn @ 2006-05-10  2:11 UTC (permalink / raw)
  To: Andi Kleen
  Cc: linux-kernel, Eric W. Biederman, herbert, dev, sam, xemul,
	haveblue, clg, frankeh

Incorporate fs namespace into nsproxy.

Signed-off-by: Serge Hallyn <serue@us.ibm.com>

---

 fs/namespace.c            |   21 +++++++++++++--------
 fs/proc/base.c            |    5 +++--
 include/linux/init_task.h |    1 +
 include/linux/namespace.h |    6 ++----
 include/linux/nsproxy.h   |    3 +++
 include/linux/sched.h     |    4 +---
 kernel/exit.c             |    7 +++----
 kernel/fork.c             |    6 +++---
 8 files changed, 29 insertions(+), 24 deletions(-)

8095ae8032ce2164e7177fb2d254629e7851947c
diff --git a/fs/namespace.c b/fs/namespace.c
index 2c5f1f8..851a02d 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -133,7 +133,7 @@ struct vfsmount *lookup_mnt(struct vfsmo
 
 static inline int check_mnt(struct vfsmount *mnt)
 {
-	return mnt->mnt_namespace == current->namespace;
+	return mnt->mnt_namespace == current->nsproxy->namespace;
 }
 
 static void touch_namespace(struct namespace *ns)
@@ -832,7 +832,7 @@ static int attach_recursive_mnt(struct v
 	if (parent_nd) {
 		detach_mnt(source_mnt, parent_nd);
 		attach_mnt(source_mnt, nd);
-		touch_namespace(current->namespace);
+		touch_namespace(current->nsproxy->namespace);
 	} else {
 		mnt_set_mountpoint(dest_mnt, dest_dentry, source_mnt);
 		commit_tree(source_mnt);
@@ -1372,7 +1372,7 @@ dput_out:
  */
 struct namespace *dup_namespace(struct task_struct *tsk, struct fs_struct *fs)
 {
-	struct namespace *namespace = tsk->namespace;
+	struct namespace *namespace = tsk->nsproxy->namespace;
 	struct namespace *new_ns;
 	struct vfsmount *rootmnt = NULL, *pwdmnt = NULL, *altrootmnt = NULL;
 	struct vfsmount *p, *q;
@@ -1439,7 +1439,7 @@ struct namespace *dup_namespace(struct t
 
 int copy_namespace(int flags, struct task_struct *tsk)
 {
-	struct namespace *namespace = tsk->namespace;
+	struct namespace *namespace = tsk->nsproxy->namespace;
 	struct namespace *new_ns;
 	int err = 0;
 
@@ -1462,7 +1462,7 @@ int copy_namespace(int flags, struct tas
 		goto out;
 	}
 
-	tsk->namespace = new_ns;
+	tsk->nsproxy->namespace = new_ns;
 
 out:
 	put_namespace(namespace);
@@ -1685,7 +1685,7 @@ asmlinkage long sys_pivot_root(const cha
 	detach_mnt(user_nd.mnt, &root_parent);
 	attach_mnt(user_nd.mnt, &old_nd);     /* mount old root on put_old */
 	attach_mnt(new_nd.mnt, &root_parent); /* mount new_root on / */
-	touch_namespace(current->namespace);
+	touch_namespace(current->nsproxy->namespace);
 	spin_unlock(&vfsmount_lock);
 	chroot_fs_refs(&user_nd, &new_nd);
 	security_sb_post_pivotroot(&user_nd, &new_nd);
@@ -1727,11 +1727,16 @@ static void __init init_mount_tree(void)
 	namespace->root = mnt;
 	mnt->mnt_namespace = namespace;
 
-	init_task.namespace = namespace;
+	init_task.nsproxy->namespace = namespace;
 	read_lock(&tasklist_lock);
 	do_each_thread(g, p) {
+		/* do we want namespace count to be #nsproxies,
+		 * or # processes pointing to the namespace? */
 		get_namespace(namespace);
-		p->namespace = namespace;
+#if 0
+		/* should only be 1 nsproxy so far */
+		p->nsproxy->namespace = namespace;
+#endif
 	} while_each_thread(g, p);
 	read_unlock(&tasklist_lock);
 
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 6cc77dc..f74acae 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -72,6 +72,7 @@ #include <linux/seccomp.h>
 #include <linux/cpuset.h>
 #include <linux/audit.h>
 #include <linux/poll.h>
+#include <linux/nsproxy.h>
 #include "internal.h"
 
 /*
@@ -685,7 +686,7 @@ static int mounts_open(struct inode *ino
 	int ret = -EINVAL;
 
 	task_lock(task);
-	namespace = task->namespace;
+	namespace = task->nsproxy->namespace;
 	if (namespace)
 		get_namespace(namespace);
 	task_unlock(task);
@@ -752,7 +753,7 @@ static int mountstats_open(struct inode 
 		struct seq_file *m = file->private_data;
 		struct namespace *namespace;
 		task_lock(task);
-		namespace = task->namespace;
+		namespace = task->nsproxy->namespace;
 		if (namespace)
 			get_namespace(namespace);
 		task_unlock(task);
diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index 79ec4ea..672dc04 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -70,6 +70,7 @@ extern struct nsproxy init_nsproxy;
 #define INIT_NSPROXY(nsproxy) {						\
 	.count		= ATOMIC_INIT(1),				\
 	.nslock		= SPIN_LOCK_UNLOCKED,				\
+	.namespace	= NULL,						\
 }
 
 #define INIT_SIGHAND(sighand) {						\
diff --git a/include/linux/namespace.h b/include/linux/namespace.h
index 3abc8e3..d137009 100644
--- a/include/linux/namespace.h
+++ b/include/linux/namespace.h
@@ -4,6 +4,7 @@ #ifdef __KERNEL__
 
 #include <linux/mount.h>
 #include <linux/sched.h>
+#include <linux/nsproxy.h>
 
 struct namespace {
 	atomic_t		count;
@@ -26,11 +27,8 @@ static inline void put_namespace(struct 
 
 static inline void exit_namespace(struct task_struct *p)
 {
-	struct namespace *namespace = p->namespace;
+	struct namespace *namespace = p->nsproxy->namespace;
 	if (namespace) {
-		task_lock(p);
-		p->namespace = NULL;
-		task_unlock(p);
 		put_namespace(namespace);
 	}
 }
diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
index 065107d..64e9075 100644
--- a/include/linux/nsproxy.h
+++ b/include/linux/nsproxy.h
@@ -4,9 +4,12 @@ #define _LINUX_NSPROXY_H
 #include <linux/spinlock.h>
 #include <linux/sched.h>
 
+struct namespace;
+
 struct nsproxy {
 	atomic_t count;
 	spinlock_t nslock;
+	struct namespace *namespace;
 };
 extern struct nsproxy init_nsproxy;
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 4c0bbb3..f2c945b 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -235,7 +235,6 @@ extern signed long schedule_timeout_inte
 extern signed long schedule_timeout_uninterruptible(signed long timeout);
 asmlinkage void schedule(void);
 
-struct namespace;
 struct nsproxy;
 
 /* Maximum number of active map areas.. This is a random (large) number */
@@ -807,8 +806,7 @@ #endif
 	struct fs_struct *fs;
 /* open file information */
 	struct files_struct *files;
-/* namespace */
-	struct namespace *namespace;
+/* namespaces */
 	struct nsproxy *nsproxy;
 /* signal handlers */
 	struct signal_struct *signal;
diff --git a/kernel/exit.c b/kernel/exit.c
index 234f5ea..1862d36 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -14,6 +14,7 @@ #include <linux/capability.h>
 #include <linux/completion.h>
 #include <linux/personality.h>
 #include <linux/tty.h>
+#include <linux/nsproxy.h>
 #include <linux/namespace.h>
 #include <linux/key.h>
 #include <linux/security.h>
@@ -36,7 +37,6 @@ #include <linux/futex.h>
 #include <linux/compat.h>
 #include <linux/pipe_fs_i.h>
 #include <linux/audit.h> /* for audit_free() */
-#include <linux/nsproxy.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -416,11 +416,10 @@ void daemonize(const char *name, ...)
 	current->fs = fs;
 	atomic_inc(&fs->count);
 	exit_namespace(current);
-	current->namespace = init_task.namespace;
-	get_namespace(current->namespace);
 	exit_nsproxy(current);
 	current->nsproxy = init_task.nsproxy;
 	get_nsproxy(current->nsproxy);
+	get_namespace(current->nsproxy->namespace);
  	exit_files(current);
 	current->files = init_task.files;
 	atomic_inc(&current->files->count);
@@ -923,7 +922,7 @@ #endif
 	exit_sem(tsk);
 	__exit_files(tsk);
 	__exit_fs(tsk);
-	exit_namespace(tsk);
+	exit_namespace(current);
 	exit_nsproxy(current);
 	exit_thread();
 	cpuset_exit(tsk);
diff --git a/kernel/fork.c b/kernel/fork.c
index 9b6f1de..06cc87a 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1472,7 +1472,7 @@ static int unshare_fs(unsigned long unsh
  */
 static int unshare_namespace(unsigned long unshare_flags, struct namespace **new_nsp, struct fs_struct *new_fs)
 {
-	struct namespace *ns = current->namespace;
+	struct namespace *ns = current->nsproxy->namespace;
 
 	if ((unshare_flags & CLONE_NEWNS) &&
 	    (ns && atomic_read(&ns->count) > 1)) {
@@ -1609,8 +1609,8 @@ asmlinkage long sys_unshare(unsigned lon
 		}
 
 		if (new_ns) {
-			ns = current->namespace;
-			current->namespace = new_ns;
+			ns = current->nsproxy->namespace;
+			current->nsproxy->namespace = new_ns;
 			new_ns = ns;
 		}
 
-- 
1.3.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/9] nsproxy: incorporate fs namespace
  2006-05-10  2:11 ` [PATCH 2/9] nsproxy: incorporate fs namespace Serge E. Hallyn
@ 2006-05-10 12:46   ` Eric W. Biederman
  2006-05-10 13:26     ` Serge E. Hallyn
  2006-05-12 19:14     ` Serge E. Hallyn
  0 siblings, 2 replies; 11+ messages in thread
From: Eric W. Biederman @ 2006-05-10 12:46 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Andi Kleen, linux-kernel, herbert, dev, sam, xemul, haveblue, clg,
	frankeh

"Serge E. Hallyn" <serue@us.ibm.com> writes:


> @@ -1727,11 +1727,16 @@ static void __init init_mount_tree(void)
>  	namespace->root = mnt;
>  	mnt->mnt_namespace = namespace;
>  
> -	init_task.namespace = namespace;
> +	init_task.nsproxy->namespace = namespace;
>  	read_lock(&tasklist_lock);
>  	do_each_thread(g, p) {
> +		/* do we want namespace count to be #nsproxies,
> +		 * or # processes pointing to the namespace? */

I am fairly certain we want the count to be #nsproxies.

>  		get_namespace(namespace);
> -		p->namespace = namespace;
> +#if 0
> +		/* should only be 1 nsproxy so far */
> +		p->nsproxy->namespace = namespace;
> +#endif
>  	} while_each_thread(g, p);
>  	read_unlock(&tasklist_lock);

So I think this bit is wrong.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/9] nsproxy: incorporate fs namespace
  2006-05-10 12:46   ` Eric W. Biederman
@ 2006-05-10 13:26     ` Serge E. Hallyn
  2006-05-10 19:07       ` Eric W. Biederman
  2006-05-12 19:14     ` Serge E. Hallyn
  1 sibling, 1 reply; 11+ messages in thread
From: Serge E. Hallyn @ 2006-05-10 13:26 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Serge E. Hallyn, Andi Kleen, linux-kernel, herbert, dev, sam,
	xemul, haveblue, clg, frankeh

Quoting Eric W. Biederman (ebiederm@xmission.com):
> "Serge E. Hallyn" <serue@us.ibm.com> writes:
> 
> 
> > @@ -1727,11 +1727,16 @@ static void __init init_mount_tree(void)
> >  	namespace->root = mnt;
> >  	mnt->mnt_namespace = namespace;
> >  
> > -	init_task.namespace = namespace;
> > +	init_task.nsproxy->namespace = namespace;
> >  	read_lock(&tasklist_lock);
> >  	do_each_thread(g, p) {
> > +		/* do we want namespace count to be #nsproxies,
> > +		 * or # processes pointing to the namespace? */
> 
> I am fairly certain we want the count to be #nsproxies.
> 
> >  		get_namespace(namespace);
> > -		p->namespace = namespace;
> > +#if 0
> > +		/* should only be 1 nsproxy so far */
> > +		p->nsproxy->namespace = namespace;
> > +#endif
> >  	} while_each_thread(g, p);
> >  	read_unlock(&tasklist_lock);
> 
> So I think this bit is wrong.

Ok - in that case I need to audit the rest of namespace usage to make
certain nothing depends on the count being #tasks.

BTW - a first set of comparison results showed nsproxy to have better
dbench and tbench throughput, and worse kernbench performance.  Which
may make sense given that nsproxy results in lower memory usage but
likely increased cache misses due to extra pointer dereference.

-serge

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/9] nsproxy: incorporate fs namespace
  2006-05-10 13:26     ` Serge E. Hallyn
@ 2006-05-10 19:07       ` Eric W. Biederman
  2006-05-10 20:34         ` Serge E. Hallyn
  0 siblings, 1 reply; 11+ messages in thread
From: Eric W. Biederman @ 2006-05-10 19:07 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Andi Kleen, linux-kernel, herbert, dev, sam, xemul, haveblue, clg,
	frankeh

"Serge E. Hallyn" <serue@us.ibm.com> writes:

> Quoting Eric W. Biederman (ebiederm@xmission.com):
>> "Serge E. Hallyn" <serue@us.ibm.com> writes:
>> 
>> 
>> > @@ -1727,11 +1727,16 @@ static void __init init_mount_tree(void)
>> >  	namespace->root = mnt;
>> >  	mnt->mnt_namespace = namespace;
>> >  
>> > -	init_task.namespace = namespace;
>> > +	init_task.nsproxy->namespace = namespace;
>> >  	read_lock(&tasklist_lock);
>> >  	do_each_thread(g, p) {
>> > +		/* do we want namespace count to be #nsproxies,
>> > +		 * or # processes pointing to the namespace? */
>> 
>> I am fairly certain we want the count to be #nsproxies.
>> 
>> >  		get_namespace(namespace);
>> > -		p->namespace = namespace;
>> > +#if 0
>> > +		/* should only be 1 nsproxy so far */
>> > +		p->nsproxy->namespace = namespace;
>> > +#endif
>> >  	} while_each_thread(g, p);
>> >  	read_unlock(&tasklist_lock);
>> 
>> So I think this bit is wrong.
>
> Ok - in that case I need to audit the rest of namespace usage to make
> certain nothing depends on the count being #tasks.

Ok.  Thats makes sense.

> BTW - a first set of comparison results showed nsproxy to have better
> dbench and tbench throughput, and worse kernbench performance.  Which
> may make sense given that nsproxy results in lower memory usage but
> likely increased cache misses due to extra pointer dereference.

There are two additional things I can think of that are worth looking
at:
- moving copy_uts_namespace, and copy_namespace inside of copy_nsproxy
  so we only run those we create a new nsproxy instance.

- Attempting to optimize cache line utilization by placing the
  structures in line in struct ns_proxy:
	struct nsproxy {
		atomic_t count;
	        struct namespace *namespace;
	        struct uts_namespace *uts_ns;
	        struct namespace namespace_data;
	        struct new_utsname uts_data;
	};
  With the nsproxy count severing as a count for both the embedded
  data and for the nsproxy itself.  I think it is a long shot but it
  could be interesting.

Given the frequency of use of the uts namespace and the filesystem
namespace simply I think not accessing those namespaces on fork is
likely to reduce the additional cache line miss rate enough so
that it is lost in the noise.

Eric

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/9] nsproxy: incorporate fs namespace
  2006-05-10 19:07       ` Eric W. Biederman
@ 2006-05-10 20:34         ` Serge E. Hallyn
  2006-05-10 20:50           ` Eric W. Biederman
  0 siblings, 1 reply; 11+ messages in thread
From: Serge E. Hallyn @ 2006-05-10 20:34 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Serge E. Hallyn, Andi Kleen, linux-kernel, herbert, dev, sam,
	xemul, haveblue, clg, frankeh

Quoting Eric W. Biederman (ebiederm@xmission.com):
> There are two additional things I can think of that are worth looking
> at:
> - moving copy_uts_namespace, and copy_namespace inside of copy_nsproxy
>   so we only run those we create a new nsproxy instance.

Was about to do that when I stopped because I was thinking I'd need to
keep track of which namespace had been copied before a failaure, for
the sake of clone.

But of course I don't have to - copy_nsproxy could do the cleanup itself
on failure.

So this should be a nice little cleanup - especially as # namespaces
increases.

> - Attempting to optimize cache line utilization by placing the
>   structures in line in struct ns_proxy:
> 	struct nsproxy {
> 		atomic_t count;
> 	        struct namespace *namespace;
> 	        struct uts_namespace *uts_ns;
> 	        struct namespace namespace_data;
> 	        struct new_utsname uts_data;
> 	};
>   With the nsproxy count severing as a count for both the embedded
>   data and for the nsproxy itself.  I think it is a long shot but it
>   could be interesting.
> 
> Given the frequency of use of the uts namespace and the filesystem
> namespace simply I think not accessing those namespaces on fork is
> likely to reduce the additional cache line miss rate enough so
> that it is lost in the noise.

Not getting this.  Are you saying the uts_data would be a copy of
the contents of *uts_ns, or that uts_ns points to nsproxy->uts_data?
If the latter, then just unsharing uts_ns but not mounts namespace
is no longer possible, right?

thanks,
-serge

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/9] nsproxy: incorporate fs namespace
  2006-05-10 20:34         ` Serge E. Hallyn
@ 2006-05-10 20:50           ` Eric W. Biederman
  2006-05-12 15:24             ` Serge E. Hallyn
  0 siblings, 1 reply; 11+ messages in thread
From: Eric W. Biederman @ 2006-05-10 20:50 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Andi Kleen, linux-kernel, herbert, dev, sam, xemul, haveblue, clg,
	frankeh

"Serge E. Hallyn" <serue@us.ibm.com> writes:

> Quoting Eric W. Biederman (ebiederm@xmission.com):
>> There are two additional things I can think of that are worth looking
>> at:
>> - moving copy_uts_namespace, and copy_namespace inside of copy_nsproxy
>>   so we only run those we create a new nsproxy instance.
>
> Was about to do that when I stopped because I was thinking I'd need to
> keep track of which namespace had been copied before a failaure, for
> the sake of clone.
>
> But of course I don't have to - copy_nsproxy could do the cleanup itself
> on failure.
>
> So this should be a nice little cleanup - especially as # namespaces
> increases.

Yes.  At least if nsproxy doesn't show a performance degradation...

>> - Attempting to optimize cache line utilization by placing the
>>   structures in line in struct ns_proxy:
>> 	struct nsproxy {
>> 		atomic_t count;
>> 	        struct namespace *namespace;
>> 	        struct uts_namespace *uts_ns;
>> 	        struct namespace namespace_data;
>> 	        struct new_utsname uts_data;
>> 	};
>>   With the nsproxy count severing as a count for both the embedded
>>   data and for the nsproxy itself.  I think it is a long shot but it
>>   could be interesting.
>> 
>> Given the frequency of use of the uts namespace and the filesystem
>> namespace simply I think not accessing those namespaces on fork is
>> likely to reduce the additional cache line miss rate enough so
>> that it is lost in the noise.
>
> Not getting this.  Are you saying the uts_data would be a copy of
> the contents of *uts_ns, or that uts_ns points to nsproxy->uts_data?
> If the latter, then just unsharing uts_ns but not mounts namespace
> is no longer possible, right?

The latter, uts_ns normally points to nsproxy->uts_data.  But it still
remains possible to unshare just the mounts namespace by simply coping
the pointer when we clone nsproxy, and incrementing the previous
ns_proxies count.

Like I said I think it is a long shot but if the data for namespaces
really does remain small and they are usually all unshared in a group
it could be a win.

Eric


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/9] nsproxy: incorporate fs namespace
  2006-05-10 20:50           ` Eric W. Biederman
@ 2006-05-12 15:24             ` Serge E. Hallyn
  2006-05-12 15:44               ` Dave Hansen
  0 siblings, 1 reply; 11+ messages in thread
From: Serge E. Hallyn @ 2006-05-12 15:24 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Serge E. Hallyn, Andi Kleen, linux-kernel, herbert, dev, sam,
	xemul, haveblue, clg, frankeh

Quoting Eric W. Biederman (ebiederm@xmission.com):
> "Serge E. Hallyn" <serue@us.ibm.com> writes:
> 
> > Quoting Eric W. Biederman (ebiederm@xmission.com):
> >> There are two additional things I can think of that are worth looking
> >> at:
> >> - moving copy_uts_namespace, and copy_namespace inside of copy_nsproxy
> >>   so we only run those we create a new nsproxy instance.
> >
> > Was about to do that when I stopped because I was thinking I'd need to
> > keep track of which namespace had been copied before a failaure, for
> > the sake of clone.
> >
> > But of course I don't have to - copy_nsproxy could do the cleanup itself
> > on failure.
> >
> > So this should be a nice little cleanup - especially as # namespaces
> > increases.
> 
> Yes.  At least if nsproxy doesn't show a performance degradation...

Ok, at least here is a patch to do a bit of this.  It moves things
around enough that didn't want to combine this with setting
namespace->refcount to #nsproxies, or your inlining trick.

-serge

Subject: [PATCH] nsproxy: code cleanup

consolidate nsproxy and namespace copy/get/exit.

Signed-off-by:  <hallyn@elg11.watson.ibm.com>

---

 include/linux/namespace.h |    8 ----
 include/linux/nsproxy.h   |   26 ++++++-------
 kernel/exit.c             |   13 ++-----
 kernel/fork.c             |   39 +++++++++-----------
 kernel/nsproxy.c          |   87 ++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 113 insertions(+), 60 deletions(-)

5492e3bdc0245ab27e70fe8dc6c004cb437dc8f6
diff --git a/include/linux/namespace.h b/include/linux/namespace.h
index d137009..fce3714 100644
--- a/include/linux/namespace.h
+++ b/include/linux/namespace.h
@@ -25,14 +25,6 @@ static inline void put_namespace(struct 
 		__put_namespace(namespace);
 }
 
-static inline void exit_namespace(struct task_struct *p)
-{
-	struct namespace *namespace = p->nsproxy->namespace;
-	if (namespace) {
-		put_namespace(namespace);
-	}
-}
-
 static inline void get_namespace(struct namespace *namespace)
 {
 	atomic_inc(&namespace->count);
diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
index 18fcd8f..480f665 100644
--- a/include/linux/nsproxy.h
+++ b/include/linux/nsproxy.h
@@ -14,28 +14,24 @@ struct nsproxy {
 };
 extern struct nsproxy init_nsproxy;
 
-static inline void get_nsproxy(struct nsproxy *nsp)
+struct nsproxy *dup_namespaces(struct nsproxy *orig);
+int copy_namespaces(int flags, struct task_struct *tsk);
+void get_namespaces(struct task_struct *tsk);
+void exit_namespaces(struct nsproxy *ns);
+
+static inline void exit_task_namespaces(struct task_struct *p)
 {
-	atomic_inc(&nsp->count);
+	struct nsproxy *ns = p->nsproxy;
+	if (ns) {
+		exit_namespaces(p->nsproxy);
+		p->nsproxy = NULL;
+	}
 }
 
-struct nsproxy *clone_nsproxy(struct nsproxy *orig);
-int copy_nsproxy(int flags, struct task_struct *tsk);
-void free_nsproxy(struct nsproxy *nsp);
-
 static inline void put_nsproxy(struct nsproxy *nsp)
 {
 	if (atomic_dec_and_test(&nsp->count)) {
 		kfree(nsp);
 	}
 }
-
-static inline void exit_nsproxy(struct task_struct *p)
-{
-	struct nsproxy *nsp = p->nsproxy;
-	if (nsp) {
-		p->nsproxy = NULL;
-		put_nsproxy(nsp);
-	}
-}
 #endif
diff --git a/kernel/exit.c b/kernel/exit.c
index 921a4b7..4ded4e3 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -175,7 +175,6 @@ repeat:
 	write_unlock_irq(&tasklist_lock);
 	spin_unlock(&p->proc_lock);
 	proc_pid_flush(proc_dentry);
-	exit_nsproxy(p);
 	release_thread(p);
 	call_rcu(&p->rcu, delayed_put_task_struct);
 
@@ -416,13 +415,9 @@ void daemonize(const char *name, ...)
 	current->fs = fs;
 	atomic_inc(&fs->count);
 
-	exit_utsname(current);
-	exit_namespace(current);
-	exit_nsproxy(current);
+	exit_task_namespaces(current);
 	current->nsproxy = init_task.nsproxy;
-	get_nsproxy(current->nsproxy);
-	get_namespace(current->nsproxy->namespace);
-	get_uts_ns(current->nsproxy->uts_ns);
+	get_namespaces(current);
 
  	exit_files(current);
 	current->files = init_task.files;
@@ -926,9 +921,7 @@ fastcall NORET_TYPE void do_exit(long co
 	exit_sem(tsk);
 	__exit_files(tsk);
 	__exit_fs(tsk);
-	exit_utsname(current);
-	exit_namespace(current);
-	exit_nsproxy(current);
+	exit_task_namespaces(current);
 	exit_thread();
 	cpuset_exit(tsk);
 	exit_keys(tsk);
diff --git a/kernel/fork.c b/kernel/fork.c
index baeef86..f9b607c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -45,7 +45,6 @@
 #include <linux/acct.h>
 #include <linux/cn_proc.h>
 #include <linux/nsproxy.h>
-#include <linux/utsname.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
@@ -1062,15 +1061,11 @@ static task_t *copy_process(unsigned lon
 		goto bad_fork_cleanup_signal;
 	if ((retval = copy_keys(clone_flags, p)))
 		goto bad_fork_cleanup_mm;
-	if ((retval = copy_nsproxy(clone_flags, p)))
+	if ((retval = copy_namespaces(clone_flags, p)))
 		goto bad_fork_cleanup_keys;
-	if ((retval = copy_utsname(clone_flags, p)))
-		goto bad_fork_cleanup_nsproxy;
-	if ((retval = copy_namespace(clone_flags, p)))
-		goto bad_fork_cleanup_utsname;
 	retval = copy_thread(0, clone_flags, stack_start, stack_size, p, regs);
 	if (retval)
-		goto bad_fork_cleanup_namespace;
+		goto bad_fork_cleanup_namespaces;
 
 	p->set_child_tid = (clone_flags & CLONE_CHILD_SETTID) ? child_tidptr : NULL;
 	/*
@@ -1157,7 +1152,7 @@ static task_t *copy_process(unsigned lon
 		spin_unlock(&current->sighand->siglock);
 		write_unlock_irq(&tasklist_lock);
 		retval = -ERESTARTNOINTR;
-		goto bad_fork_cleanup_namespace;
+		goto bad_fork_cleanup_namespaces;
 	}
 
 	if (clone_flags & CLONE_THREAD) {
@@ -1170,7 +1165,7 @@ static task_t *copy_process(unsigned lon
 			spin_unlock(&current->sighand->siglock);
 			write_unlock_irq(&tasklist_lock);
 			retval = -EAGAIN;
-			goto bad_fork_cleanup_namespace;
+			goto bad_fork_cleanup_namespaces;
 		}
 
 		p->group_leader = current->group_leader;
@@ -1222,12 +1217,8 @@ static task_t *copy_process(unsigned lon
 	proc_fork_connector(p);
 	return p;
 
-bad_fork_cleanup_namespace:
-	exit_namespace(p);
-bad_fork_cleanup_utsname:
-	exit_utsname(p);
-bad_fork_cleanup_nsproxy:
-	exit_nsproxy(p);
+bad_fork_cleanup_namespaces:
+	exit_task_namespaces(p);
 bad_fork_cleanup_keys:
 	exit_keys(p);
 bad_fork_cleanup_mm:
@@ -1570,7 +1561,7 @@ asmlinkage long sys_unshare(unsigned lon
 	struct files_struct *fd, *new_fd = NULL;
 	struct sem_undo_list *new_ulist = NULL;
 	struct nsproxy *new_nsproxy, *old_nsproxy;
-	struct uts_namespace *new_uts = NULL;
+	struct uts_namespace *uts, *new_uts = NULL;
 
 	check_unshare_flags(&unshare_flags);
 
@@ -1595,21 +1586,20 @@ asmlinkage long sys_unshare(unsigned lon
 	if ((err = unshare_semundo(unshare_flags, &new_ulist)))
 		goto bad_unshare_cleanup_fd;
 	if ((err = unshare_utsname(unshare_flags, &new_uts)))
-		goto bad_unshare_cleanup_fd;
+		goto bad_unshare_cleanup_semundo;
 
 	if (new_fs || new_ns || new_sigh || new_mm || new_fd || new_ulist ||
 				new_uts) {
 
 		old_nsproxy = current->nsproxy;
-		new_nsproxy = clone_nsproxy(old_nsproxy);
+		new_nsproxy = dup_namespaces(old_nsproxy);
 		if (!new_nsproxy) {
 			err = -ENOMEM;
-			goto bad_unshare_cleanup_semundo;
+			goto bad_unshare_cleanup_uts;
 		}
 
 		task_lock(current);
 		current->nsproxy = new_nsproxy;
-		put_nsproxy(old_nsproxy);
 
 		if (new_fs) {
 			fs = current->fs;
@@ -1645,13 +1635,20 @@ asmlinkage long sys_unshare(unsigned lon
 		}
 
 		if (new_uts) {
-			put_uts_ns(current->nsproxy->uts_ns);
+			uts = current->nsproxy->uts_ns;
 			current->nsproxy->uts_ns = new_uts;
+			new_uts = uts;
 		}
 
 		task_unlock(current);
+
+		put_nsproxy(old_nsproxy);
 	}
 
+bad_unshare_cleanup_uts:
+	if (new_uts)
+		put_uts_ns(new_uts);
+
 bad_unshare_cleanup_semundo:
 bad_unshare_cleanup_fd:
 	if (new_fd)
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index 2390afb..a93a0c5 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -13,8 +13,31 @@
 #include <linux/module.h>
 #include <linux/version.h>
 #include <linux/nsproxy.h>
+#include <linux/namespace.h>
+#include <linux/utsname.h>
 
-struct nsproxy *clone_nsproxy(struct nsproxy *orig)
+static inline void get_nsproxy(struct nsproxy *ns)
+{
+	atomic_inc(&ns->count);
+}
+
+void get_namespaces(struct task_struct *tsk)
+{
+	struct nsproxy *ns = tsk->nsproxy;
+	if (ns) {
+		get_nsproxy(ns);
+		if (ns->namespace)
+			get_namespace(ns->namespace);
+		if (ns->uts_ns)
+			get_uts_ns(ns->uts_ns);
+	}
+}
+
+/*
+ * creates a copy of "orig" with refcount 1.
+ * This does not grab references to the contained namespaces.
+ */
+static inline struct nsproxy *clone_namespaces(struct nsproxy *orig)
 {
 	struct nsproxy *ns;
 
@@ -26,7 +49,42 @@ struct nsproxy *clone_nsproxy(struct nsp
 	return ns;
 }
 
-int copy_nsproxy(int flags, struct task_struct *tsk)
+/*
+ * copies the nsproxy, setting refcount to 1, and grabbing a
+ * reference to all contained namespaces.  Called from
+ * sys_unshare()
+ */
+struct nsproxy *dup_namespaces(struct nsproxy *orig)
+{
+	struct nsproxy *ns = clone_namespaces(orig);
+
+	if (ns) {
+		if (ns->namespace)
+			get_namespace(ns->namespace);
+		if (ns->uts_ns)
+			get_uts_ns(ns->uts_ns);
+	}
+
+	return ns;
+}
+
+/*
+ * Put refcount on nsproxy and each namespace therein
+ */
+void exit_namespaces(struct nsproxy *ns)
+{
+	if (ns->namespace)
+		put_namespace(ns->namespace);
+	if (ns->uts_ns)
+		put_uts_ns(ns->uts_ns);
+	put_nsproxy(ns);
+}
+
+/*
+ * called from clone.  This now handles copy for nsproxy and all
+ * namespaces therein.
+ */
+int copy_namespaces(int flags, struct task_struct *tsk)
 {
 	struct nsproxy *old_ns = tsk->nsproxy;
 	struct nsproxy *new_ns;
@@ -35,19 +93,36 @@ int copy_nsproxy(int flags, struct task_
 	if (!old_ns)
 		return 0;
 
-	get_nsproxy(old_ns);
+	get_namespaces(tsk);
 
-	if (!(flags & CLONE_NEWNS))
+	if (!(flags & (CLONE_NEWNS | CLONE_NEWUTS)))
 		return 0;
 
-	new_ns = clone_nsproxy(old_ns);
+	new_ns = clone_namespaces(old_ns);
 	if (!new_ns) {
 		err = -ENOMEM;
 		goto out;
 	}
+
 	tsk->nsproxy = new_ns;
 
+	err = copy_namespace(flags, tsk);
+	if (err) {
+		tsk->nsproxy = old_ns;
+		put_nsproxy(new_ns);
+		goto out;
+	}
+
+	err = copy_utsname(flags, tsk);
+	if (err) {
+		if (new_ns->namespace)
+			put_namespace(new_ns->namespace);
+		tsk->nsproxy = old_ns;
+		put_nsproxy(new_ns);
+		goto out;
+	}
+
 out:
-	put_nsproxy(old_ns);
+	exit_namespaces(old_ns);
 	return err;
 }
-- 
1.1.6

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/9] nsproxy: incorporate fs namespace
  2006-05-12 15:24             ` Serge E. Hallyn
@ 2006-05-12 15:44               ` Dave Hansen
  2006-05-12 16:54                 ` Serge E. Hallyn
  2006-05-12 19:12                 ` Serge E. Hallyn
  0 siblings, 2 replies; 11+ messages in thread
From: Dave Hansen @ 2006-05-12 15:44 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Eric W. Biederman, Andi Kleen, linux-kernel, herbert, dev, sam,
	xemul, clg, frankeh

On Fri, 2006-05-12 at 10:24 -0500, Serge E. Hallyn wrote:
> 
> -       exit_utsname(current);
> -       exit_namespace(current);
> -       exit_nsproxy(current);
> +       exit_task_namespaces(current);
>         current->nsproxy = init_task.nsproxy;
> -       get_nsproxy(current->nsproxy);
> -       get_namespace(current->nsproxy->namespace);
> -       get_uts_ns(current->nsproxy->uts_ns);
> +       get_namespaces(current); 

That really cleans up the main path quite a bit.  Very nice.

For parity with exit_task_namespaces(), should that be called
get_task_namespaces()?

-- Dave


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/9] nsproxy: incorporate fs namespace
  2006-05-12 15:44               ` Dave Hansen
@ 2006-05-12 16:54                 ` Serge E. Hallyn
  2006-05-12 19:12                 ` Serge E. Hallyn
  1 sibling, 0 replies; 11+ messages in thread
From: Serge E. Hallyn @ 2006-05-12 16:54 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Serge E. Hallyn, Eric W. Biederman, Andi Kleen, linux-kernel,
	herbert, dev, sam, xemul, clg, frankeh

Quoting Dave Hansen (haveblue@us.ibm.com):
> On Fri, 2006-05-12 at 10:24 -0500, Serge E. Hallyn wrote:
> > 
> > -       exit_utsname(current);
> > -       exit_namespace(current);
> > -       exit_nsproxy(current);
> > +       exit_task_namespaces(current);
> >         current->nsproxy = init_task.nsproxy;
> > -       get_nsproxy(current->nsproxy);
> > -       get_namespace(current->nsproxy->namespace);
> > -       get_uts_ns(current->nsproxy->uts_ns);
> > +       get_namespaces(current); 
> 
> That really cleans up the main path quite a bit.  Very nice.
> 
> For parity with exit_task_namespaces(), should that be called
> get_task_namespaces()?

Yup, that would make sense.  Will update the patch in my git tree.

Though those functions be renamed when the namespaces->count becomes
#nsproxies.  Getting a weird regression with that patch, though, so
won't be sending that out today.

thanks,
-serge

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/9] nsproxy: incorporate fs namespace
  2006-05-12 15:44               ` Dave Hansen
  2006-05-12 16:54                 ` Serge E. Hallyn
@ 2006-05-12 19:12                 ` Serge E. Hallyn
  1 sibling, 0 replies; 11+ messages in thread
From: Serge E. Hallyn @ 2006-05-12 19:12 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Serge E. Hallyn, Eric W. Biederman, Andi Kleen, linux-kernel,
	herbert, dev, sam, xemul, clg, frankeh

Quoting Dave Hansen (haveblue@us.ibm.com):
> On Fri, 2006-05-12 at 10:24 -0500, Serge E. Hallyn wrote:
> > 
> > -       exit_utsname(current);
> > -       exit_namespace(current);
> > -       exit_nsproxy(current);
> > +       exit_task_namespaces(current);
> >         current->nsproxy = init_task.nsproxy;
> > -       get_nsproxy(current->nsproxy);
> > -       get_namespace(current->nsproxy->namespace);
> > -       get_uts_ns(current->nsproxy->uts_ns);
> > +       get_namespaces(current); 
> 
> That really cleans up the main path quite a bit.  Very nice.
> 
> For parity with exit_task_namespaces(), should that be called
> get_task_namespaces()?

Here's a new patch:

Subject: [PATCH 10/11] nsproxy: code cleanup

consolidate nsproxy and namespace copy/get/exit.

Changelog:
	renamed get_namespaces(tsk) to get_task_namespaces(tsk).

Signed-off-by: Serge Hallyn <serue@us.ibm.com>

---

 include/linux/namespace.h |    8 ----
 include/linux/nsproxy.h   |   26 +++++--------
 kernel/exit.c             |   13 +-----
 kernel/fork.c             |   39 +++++++++----------
 kernel/nsproxy.c          |   92 ++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 118 insertions(+), 60 deletions(-)

3f3ce98f9c6fc8c3ce31cb170bf281c8eb0d4c6a
diff --git a/include/linux/namespace.h b/include/linux/namespace.h
index d137009..fce3714 100644
--- a/include/linux/namespace.h
+++ b/include/linux/namespace.h
@@ -25,14 +25,6 @@ static inline void put_namespace(struct 
 		__put_namespace(namespace);
 }
 
-static inline void exit_namespace(struct task_struct *p)
-{
-	struct namespace *namespace = p->nsproxy->namespace;
-	if (namespace) {
-		put_namespace(namespace);
-	}
-}
-
 static inline void get_namespace(struct namespace *namespace)
 {
 	atomic_inc(&namespace->count);
diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
index 18fcd8f..6046fc3 100644
--- a/include/linux/nsproxy.h
+++ b/include/linux/nsproxy.h
@@ -14,28 +14,24 @@ struct nsproxy {
 };
 extern struct nsproxy init_nsproxy;
 
-static inline void get_nsproxy(struct nsproxy *nsp)
+struct nsproxy *dup_namespaces(struct nsproxy *orig);
+int copy_namespaces(int flags, struct task_struct *tsk);
+void get_task_namespaces(struct task_struct *tsk);
+void exit_namespaces(struct nsproxy *ns);
+
+static inline void exit_task_namespaces(struct task_struct *p)
 {
-	atomic_inc(&nsp->count);
+	struct nsproxy *ns = p->nsproxy;
+	if (ns) {
+		exit_namespaces(p->nsproxy);
+		p->nsproxy = NULL;
+	}
 }
 
-struct nsproxy *clone_nsproxy(struct nsproxy *orig);
-int copy_nsproxy(int flags, struct task_struct *tsk);
-void free_nsproxy(struct nsproxy *nsp);
-
 static inline void put_nsproxy(struct nsproxy *nsp)
 {
 	if (atomic_dec_and_test(&nsp->count)) {
 		kfree(nsp);
 	}
 }
-
-static inline void exit_nsproxy(struct task_struct *p)
-{
-	struct nsproxy *nsp = p->nsproxy;
-	if (nsp) {
-		p->nsproxy = NULL;
-		put_nsproxy(nsp);
-	}
-}
 #endif
diff --git a/kernel/exit.c b/kernel/exit.c
index 921a4b7..bdc273a 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -175,7 +175,6 @@ repeat:
 	write_unlock_irq(&tasklist_lock);
 	spin_unlock(&p->proc_lock);
 	proc_pid_flush(proc_dentry);
-	exit_nsproxy(p);
 	release_thread(p);
 	call_rcu(&p->rcu, delayed_put_task_struct);
 
@@ -416,13 +415,9 @@ void daemonize(const char *name, ...)
 	current->fs = fs;
 	atomic_inc(&fs->count);
 
-	exit_utsname(current);
-	exit_namespace(current);
-	exit_nsproxy(current);
+	exit_task_namespaces(current);
 	current->nsproxy = init_task.nsproxy;
-	get_nsproxy(current->nsproxy);
-	get_namespace(current->nsproxy->namespace);
-	get_uts_ns(current->nsproxy->uts_ns);
+	get_task_namespaces(current);
 
  	exit_files(current);
 	current->files = init_task.files;
@@ -926,9 +921,7 @@ fastcall NORET_TYPE void do_exit(long co
 	exit_sem(tsk);
 	__exit_files(tsk);
 	__exit_fs(tsk);
-	exit_utsname(current);
-	exit_namespace(current);
-	exit_nsproxy(current);
+	exit_task_namespaces(current);
 	exit_thread();
 	cpuset_exit(tsk);
 	exit_keys(tsk);
diff --git a/kernel/fork.c b/kernel/fork.c
index baeef86..f9b607c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -45,7 +45,6 @@
 #include <linux/acct.h>
 #include <linux/cn_proc.h>
 #include <linux/nsproxy.h>
-#include <linux/utsname.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
@@ -1062,15 +1061,11 @@ static task_t *copy_process(unsigned lon
 		goto bad_fork_cleanup_signal;
 	if ((retval = copy_keys(clone_flags, p)))
 		goto bad_fork_cleanup_mm;
-	if ((retval = copy_nsproxy(clone_flags, p)))
+	if ((retval = copy_namespaces(clone_flags, p)))
 		goto bad_fork_cleanup_keys;
-	if ((retval = copy_utsname(clone_flags, p)))
-		goto bad_fork_cleanup_nsproxy;
-	if ((retval = copy_namespace(clone_flags, p)))
-		goto bad_fork_cleanup_utsname;
 	retval = copy_thread(0, clone_flags, stack_start, stack_size, p, regs);
 	if (retval)
-		goto bad_fork_cleanup_namespace;
+		goto bad_fork_cleanup_namespaces;
 
 	p->set_child_tid = (clone_flags & CLONE_CHILD_SETTID) ? child_tidptr : NULL;
 	/*
@@ -1157,7 +1152,7 @@ static task_t *copy_process(unsigned lon
 		spin_unlock(&current->sighand->siglock);
 		write_unlock_irq(&tasklist_lock);
 		retval = -ERESTARTNOINTR;
-		goto bad_fork_cleanup_namespace;
+		goto bad_fork_cleanup_namespaces;
 	}
 
 	if (clone_flags & CLONE_THREAD) {
@@ -1170,7 +1165,7 @@ static task_t *copy_process(unsigned lon
 			spin_unlock(&current->sighand->siglock);
 			write_unlock_irq(&tasklist_lock);
 			retval = -EAGAIN;
-			goto bad_fork_cleanup_namespace;
+			goto bad_fork_cleanup_namespaces;
 		}
 
 		p->group_leader = current->group_leader;
@@ -1222,12 +1217,8 @@ static task_t *copy_process(unsigned lon
 	proc_fork_connector(p);
 	return p;
 
-bad_fork_cleanup_namespace:
-	exit_namespace(p);
-bad_fork_cleanup_utsname:
-	exit_utsname(p);
-bad_fork_cleanup_nsproxy:
-	exit_nsproxy(p);
+bad_fork_cleanup_namespaces:
+	exit_task_namespaces(p);
 bad_fork_cleanup_keys:
 	exit_keys(p);
 bad_fork_cleanup_mm:
@@ -1570,7 +1561,7 @@ asmlinkage long sys_unshare(unsigned lon
 	struct files_struct *fd, *new_fd = NULL;
 	struct sem_undo_list *new_ulist = NULL;
 	struct nsproxy *new_nsproxy, *old_nsproxy;
-	struct uts_namespace *new_uts = NULL;
+	struct uts_namespace *uts, *new_uts = NULL;
 
 	check_unshare_flags(&unshare_flags);
 
@@ -1595,21 +1586,20 @@ asmlinkage long sys_unshare(unsigned lon
 	if ((err = unshare_semundo(unshare_flags, &new_ulist)))
 		goto bad_unshare_cleanup_fd;
 	if ((err = unshare_utsname(unshare_flags, &new_uts)))
-		goto bad_unshare_cleanup_fd;
+		goto bad_unshare_cleanup_semundo;
 
 	if (new_fs || new_ns || new_sigh || new_mm || new_fd || new_ulist ||
 				new_uts) {
 
 		old_nsproxy = current->nsproxy;
-		new_nsproxy = clone_nsproxy(old_nsproxy);
+		new_nsproxy = dup_namespaces(old_nsproxy);
 		if (!new_nsproxy) {
 			err = -ENOMEM;
-			goto bad_unshare_cleanup_semundo;
+			goto bad_unshare_cleanup_uts;
 		}
 
 		task_lock(current);
 		current->nsproxy = new_nsproxy;
-		put_nsproxy(old_nsproxy);
 
 		if (new_fs) {
 			fs = current->fs;
@@ -1645,13 +1635,20 @@ asmlinkage long sys_unshare(unsigned lon
 		}
 
 		if (new_uts) {
-			put_uts_ns(current->nsproxy->uts_ns);
+			uts = current->nsproxy->uts_ns;
 			current->nsproxy->uts_ns = new_uts;
+			new_uts = uts;
 		}
 
 		task_unlock(current);
+
+		put_nsproxy(old_nsproxy);
 	}
 
+bad_unshare_cleanup_uts:
+	if (new_uts)
+		put_uts_ns(new_uts);
+
 bad_unshare_cleanup_semundo:
 bad_unshare_cleanup_fd:
 	if (new_fd)
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index 2390afb..d963af9 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -13,8 +13,36 @@
 #include <linux/module.h>
 #include <linux/version.h>
 #include <linux/nsproxy.h>
+#include <linux/namespace.h>
+#include <linux/utsname.h>
 
-struct nsproxy *clone_nsproxy(struct nsproxy *orig)
+static inline void get_nsproxy(struct nsproxy *ns)
+{
+	atomic_inc(&ns->count);
+}
+
+static inline void get_namespaces(struct nsproxy *ns)
+{
+	get_nsproxy(ns);
+	if (ns->namespace)
+		get_namespace(ns->namespace);
+	if (ns->uts_ns)
+		get_uts_ns(ns->uts_ns);
+}
+
+void get_task_namespaces(struct task_struct *tsk)
+{
+	struct nsproxy *ns = tsk->nsproxy;
+	if (ns) {
+		get_namespaces(ns);
+	}
+}
+
+/*
+ * creates a copy of "orig" with refcount 1.
+ * This does not grab references to the contained namespaces.
+ */
+static inline struct nsproxy *clone_namespaces(struct nsproxy *orig)
 {
 	struct nsproxy *ns;
 
@@ -26,7 +54,42 @@ struct nsproxy *clone_nsproxy(struct nsp
 	return ns;
 }
 
-int copy_nsproxy(int flags, struct task_struct *tsk)
+/*
+ * copies the nsproxy, setting refcount to 1, and grabbing a
+ * reference to all contained namespaces.  Called from
+ * sys_unshare()
+ */
+struct nsproxy *dup_namespaces(struct nsproxy *orig)
+{
+	struct nsproxy *ns = clone_namespaces(orig);
+
+	if (ns) {
+		if (ns->namespace)
+			get_namespace(ns->namespace);
+		if (ns->uts_ns)
+			get_uts_ns(ns->uts_ns);
+	}
+
+	return ns;
+}
+
+/*
+ * Put refcount on nsproxy and each namespace therein
+ */
+void exit_namespaces(struct nsproxy *ns)
+{
+	if (ns->namespace)
+		put_namespace(ns->namespace);
+	if (ns->uts_ns)
+		put_uts_ns(ns->uts_ns);
+	put_nsproxy(ns);
+}
+
+/*
+ * called from clone.  This now handles copy for nsproxy and all
+ * namespaces therein.
+ */
+int copy_namespaces(int flags, struct task_struct *tsk)
 {
 	struct nsproxy *old_ns = tsk->nsproxy;
 	struct nsproxy *new_ns;
@@ -35,19 +98,36 @@ int copy_nsproxy(int flags, struct task_
 	if (!old_ns)
 		return 0;
 
-	get_nsproxy(old_ns);
+	get_namespaces(old_ns);
 
-	if (!(flags & CLONE_NEWNS))
+	if (!(flags & (CLONE_NEWNS | CLONE_NEWUTS)))
 		return 0;
 
-	new_ns = clone_nsproxy(old_ns);
+	new_ns = clone_namespaces(old_ns);
 	if (!new_ns) {
 		err = -ENOMEM;
 		goto out;
 	}
+
 	tsk->nsproxy = new_ns;
 
+	err = copy_namespace(flags, tsk);
+	if (err) {
+		tsk->nsproxy = old_ns;
+		put_nsproxy(new_ns);
+		goto out;
+	}
+
+	err = copy_utsname(flags, tsk);
+	if (err) {
+		if (new_ns->namespace)
+			put_namespace(new_ns->namespace);
+		tsk->nsproxy = old_ns;
+		put_nsproxy(new_ns);
+		goto out;
+	}
+
 out:
-	put_nsproxy(old_ns);
+	exit_namespaces(old_ns);
 	return err;
 }
-- 
1.1.6

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/9] nsproxy: incorporate fs namespace
  2006-05-10 12:46   ` Eric W. Biederman
  2006-05-10 13:26     ` Serge E. Hallyn
@ 2006-05-12 19:14     ` Serge E. Hallyn
  1 sibling, 0 replies; 11+ messages in thread
From: Serge E. Hallyn @ 2006-05-12 19:14 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Serge E. Hallyn, Andi Kleen, linux-kernel, herbert, dev, sam,
	xemul, haveblue, clg, frankeh

Quoting Eric W. Biederman (ebiederm@xmission.com):
> "Serge E. Hallyn" <serue@us.ibm.com> writes:
> 
> 
> > @@ -1727,11 +1727,16 @@ static void __init init_mount_tree(void)
> >  	namespace->root = mnt;
> >  	mnt->mnt_namespace = namespace;
> >  
> > -	init_task.namespace = namespace;
> > +	init_task.nsproxy->namespace = namespace;
> >  	read_lock(&tasklist_lock);
> >  	do_each_thread(g, p) {
> > +		/* do we want namespace count to be #nsproxies,
> > +		 * or # processes pointing to the namespace? */
> 
> I am fairly certain we want the count to be #nsproxies.
> 
> >  		get_namespace(namespace);
> > -		p->namespace = namespace;
> > +#if 0
> > +		/* should only be 1 nsproxy so far */
> > +		p->nsproxy->namespace = namespace;
> > +#endif
> >  	} while_each_thread(g, p);
> >  	read_unlock(&tasklist_lock);
> 
> So I think this bit is wrong.

Here is a patch (on top of the patchset + the patch I sent in response
to Dave) to change the fs namespace and utsname ->counts to being the
number of nsproxies holding a reference.

thanks,
-serge

Subject: [PATCH 11/11] nsproxy: change meaning of namespace refcount

switch namespace+utsname refcount to count nsproxies

Signed-off-by:  <hallyn@elg11.watson.ibm.com>

---

 fs/namespace.c          |   13 +------------
 include/linux/nsproxy.h |   18 +++++++++---------
 kernel/fork.c           |    3 +--
 kernel/nsproxy.c        |   39 ++++++++++++++-------------------------
 4 files changed, 25 insertions(+), 48 deletions(-)

41b3b9a9df03156627adc34b88c041dd3ade1236
diff --git a/fs/namespace.c b/fs/namespace.c
index 851a02d..33330fe 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1711,7 +1711,6 @@ static void __init init_mount_tree(void)
 {
 	struct vfsmount *mnt;
 	struct namespace *namespace;
-	struct task_struct *g, *p;
 
 	mnt = do_kern_mount("rootfs", 0, "rootfs", NULL);
 	if (IS_ERR(mnt))
@@ -1728,17 +1727,7 @@ static void __init init_mount_tree(void)
 	mnt->mnt_namespace = namespace;
 
 	init_task.nsproxy->namespace = namespace;
-	read_lock(&tasklist_lock);
-	do_each_thread(g, p) {
-		/* do we want namespace count to be #nsproxies,
-		 * or # processes pointing to the namespace? */
-		get_namespace(namespace);
-#if 0
-		/* should only be 1 nsproxy so far */
-		p->nsproxy->namespace = namespace;
-#endif
-	} while_each_thread(g, p);
-	read_unlock(&tasklist_lock);
+	get_namespace(namespace);
 
 	set_fs_pwd(current->fs, namespace->root, namespace->root->mnt_root);
 	set_fs_root(current->fs, namespace->root, namespace->root->mnt_root);
diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
index 6046fc3..3793017 100644
--- a/include/linux/nsproxy.h
+++ b/include/linux/nsproxy.h
@@ -17,21 +17,21 @@ extern struct nsproxy init_nsproxy;
 struct nsproxy *dup_namespaces(struct nsproxy *orig);
 int copy_namespaces(int flags, struct task_struct *tsk);
 void get_task_namespaces(struct task_struct *tsk);
-void exit_namespaces(struct nsproxy *ns);
+void free_nsproxy(struct nsproxy *ns);
 
-static inline void exit_task_namespaces(struct task_struct *p)
+static inline void put_nsproxy(struct nsproxy *ns)
 {
-	struct nsproxy *ns = p->nsproxy;
-	if (ns) {
-		exit_namespaces(p->nsproxy);
-		p->nsproxy = NULL;
+	if (atomic_dec_and_test(&ns->count)) {
+		free_nsproxy(ns);
 	}
 }
 
-static inline void put_nsproxy(struct nsproxy *nsp)
+static inline void exit_task_namespaces(struct task_struct *p)
 {
-	if (atomic_dec_and_test(&nsp->count)) {
-		kfree(nsp);
+	struct nsproxy *ns = p->nsproxy;
+	if (ns) {
+		put_nsproxy(ns);
+		p->nsproxy = NULL;
 	}
 }
 #endif
diff --git a/kernel/fork.c b/kernel/fork.c
index f9b607c..6214427 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1470,8 +1470,7 @@ static int unshare_namespace(unsigned lo
 {
 	struct namespace *ns = current->nsproxy->namespace;
 
-	if ((unshare_flags & CLONE_NEWNS) &&
-	    (ns && atomic_read(&ns->count) > 1)) {
+	if ((unshare_flags & CLONE_NEWNS) && ns) {
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
 
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index d963af9..19abf95 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -21,26 +21,18 @@ static inline void get_nsproxy(struct ns
 	atomic_inc(&ns->count);
 }
 
-static inline void get_namespaces(struct nsproxy *ns)
-{
-	get_nsproxy(ns);
-	if (ns->namespace)
-		get_namespace(ns->namespace);
-	if (ns->uts_ns)
-		get_uts_ns(ns->uts_ns);
-}
-
 void get_task_namespaces(struct task_struct *tsk)
 {
 	struct nsproxy *ns = tsk->nsproxy;
 	if (ns) {
-		get_namespaces(ns);
+		get_nsproxy(ns);
 	}
 }
 
 /*
  * creates a copy of "orig" with refcount 1.
- * This does not grab references to the contained namespaces.
+ * This does not grab references to the contained namespaces,
+ * so that needs to be done by dup_namespaces.
  */
 static inline struct nsproxy *clone_namespaces(struct nsproxy *orig)
 {
@@ -74,18 +66,6 @@ struct nsproxy *dup_namespaces(struct ns
 }
 
 /*
- * Put refcount on nsproxy and each namespace therein
- */
-void exit_namespaces(struct nsproxy *ns)
-{
-	if (ns->namespace)
-		put_namespace(ns->namespace);
-	if (ns->uts_ns)
-		put_uts_ns(ns->uts_ns);
-	put_nsproxy(ns);
-}
-
-/*
  * called from clone.  This now handles copy for nsproxy and all
  * namespaces therein.
  */
@@ -98,7 +78,7 @@ int copy_namespaces(int flags, struct ta
 	if (!old_ns)
 		return 0;
 
-	get_namespaces(old_ns);
+	get_nsproxy(old_ns);
 
 	if (!(flags & (CLONE_NEWNS | CLONE_NEWUTS)))
 		return 0;
@@ -128,6 +108,15 @@ int copy_namespaces(int flags, struct ta
 	}
 
 out:
-	exit_namespaces(old_ns);
+	put_nsproxy(old_ns);
 	return err;
 }
+
+void free_nsproxy(struct nsproxy *ns)
+{
+		if (ns->namespace)
+			put_namespace(ns->namespace);
+		if (ns->uts_ns)
+			put_uts_ns(ns->uts_ns);
+		kfree(ns);
+}
-- 
1.1.6

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2006-05-12 19:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <29vfyljM-1.2006059-s@us.ibm.com>
2006-05-10  2:11 ` [PATCH 2/9] nsproxy: incorporate fs namespace Serge E. Hallyn
2006-05-10 12:46   ` Eric W. Biederman
2006-05-10 13:26     ` Serge E. Hallyn
2006-05-10 19:07       ` Eric W. Biederman
2006-05-10 20:34         ` Serge E. Hallyn
2006-05-10 20:50           ` Eric W. Biederman
2006-05-12 15:24             ` Serge E. Hallyn
2006-05-12 15:44               ` Dave Hansen
2006-05-12 16:54                 ` Serge E. Hallyn
2006-05-12 19:12                 ` Serge E. Hallyn
2006-05-12 19:14     ` Serge E. Hallyn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox