Re: [PATCH 2/9] nsproxy: incorporate fs namespace

From: Serge E. Hallyn
Date: Fri May 12 2006 - 11:23:48 EST

Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
> "Serge E. Hallyn" <serue@xxxxxxxxxx> writes:
>
> > Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
> >> 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@xxxxxxxxxxxxxxxxxxxx>

---

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/