namespaces: Use task_lock and not rcu to protect nsproxy

The synchronous syncrhonize_rcu in switch_task_namespaces makes setns
a sufficiently expensive system call that people have complained.

Upon inspect nsproxy no longer needs rcu protection for remote reads.
remote reads are rare.  So optimize for same process reads and write
by switching using rask_lock instead.

This yields a simpler to understand lock, and a faster setns system call.

In particular this fixes a performance regression observed
by Rafael David Tinoco <rafael.tinoco@canonical.com>.

This is effectively a revert of Pavel Emelyanov's commit
cf7b708c8d1d7a27736771bcf4c457b332b0f818 Make access to task's nsproxy lighter
from 2007.  The race this originialy fixed no longer exists as
do_notify_parent uses task_active_pid_ns(parent) instead of
parent->nsproxy.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

diff --git a/fs/namespace.c b/fs/namespace.c
index 182bc41cd88711d593c4d997171c6ad483a87577..7187d01329c3581856d9936f686c7ca8b4c9c743 100644 (file)
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2972,13 +2972,13 @@ static void *mntns_get(struct task_struct *task)
        struct mnt_namespace *ns = NULL;
        struct nsproxy *nsproxy;
 
-       rcu_read_lock();
-       nsproxy = task_nsproxy(task);
+       task_lock(task);
+       nsproxy = task->nsproxy;
        if (nsproxy) {
                ns = nsproxy->mnt_ns;
                get_mnt_ns(ns);
        }
-       rcu_read_unlock();
+       task_unlock(task);
 
        return ns;
 }

diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index 4677bb7dc7c29cda9d7eb5825b86044851dbca6c..a63af3e0a6126b4445bb9a3cef8687038e670b58 100644 (file)
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -113,9 +113,11 @@ static struct net *get_proc_task_net(struct inode *dir)
        rcu_read_lock();
        task = pid_task(proc_pid(dir), PIDTYPE_PID);
        if (task != NULL) {
-               ns = task_nsproxy(task);
+               task_lock(task);
+               ns = task->nsproxy;
                if (ns != NULL)
                        net = get_net(ns->net_ns);
+               task_unlock(task);
        }
        rcu_read_unlock();
 

diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index 1a81373947f33ec4e56b5e34e24814b4c267800a..73ca1740d839513468ced1b589bf5e469f391689 100644 (file)
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -232,17 +232,15 @@ static int mounts_open_common(struct inode *inode, struct file *file,
        if (!task)
                goto err;
 
-       rcu_read_lock();
-       nsp = task_nsproxy(task);
+       task_lock(task);
+       nsp = task->nsproxy;
        if (!nsp || !nsp->mnt_ns) {
-               rcu_read_unlock();
+               task_unlock(task);
                put_task_struct(task);
                goto err;
        }
        ns = nsp->mnt_ns;
        get_mnt_ns(ns);
-       rcu_read_unlock();
-       task_lock(task);
        if (!task->fs) {
                task_unlock(task);
                put_task_struct(task);

diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
index b4ec59d159ac5c8d04a8fc89351750a1a52a03fb..35fa08fd77399a3a95ad8d4183f16990b8e6292a 100644 (file)
--- a/include/linux/nsproxy.h
+++ b/include/linux/nsproxy.h
@@ -40,32 +40,28 @@ extern struct nsproxy init_nsproxy;
  * the namespaces access rules are:
  *
  *  1. only current task is allowed to change tsk->nsproxy pointer or
- *     any pointer on the nsproxy itself
+ *     any pointer on the nsproxy itself.  Current must hold the task_lock
+ *     when changing tsk->nsproxy.
  *
  *  2. when accessing (i.e. reading) current task's namespaces - no
  *     precautions should be taken - just dereference the pointers
  *
  *  3. the access to other task namespaces is performed like this
- *     rcu_read_lock();
- *     nsproxy = task_nsproxy(tsk);
+ *     task_lock(task);
+ *     nsproxy = task->nsproxy;
  *     if (nsproxy != NULL) {
  *             / *
  *               * work with the namespaces here
  *               * e.g. get the reference on one of them
  *               * /
  *     } / *
- *         * NULL task_nsproxy() means that this task is
+ *         * NULL task->nsproxy means that this task is
  *         * almost dead (zombie)
  *         * /
- *     rcu_read_unlock();
+ *     task_unlock(task);
  *
  */
 
-static inline struct nsproxy *task_nsproxy(struct task_struct *tsk)
-{
-       return rcu_dereference(tsk->nsproxy);
-}
-
 int copy_namespaces(unsigned long flags, struct task_struct *tsk);
 void exit_task_namespaces(struct task_struct *tsk);
 void switch_task_namespaces(struct task_struct *tsk, struct nsproxy *new);

diff --git a/ipc/namespace.c b/ipc/namespace.c
index 59451c1e214d71f1b771b764d309b20759e19eb6..b54468e48e3214899b992b38b130072d1a580d4b 100644 (file)
--- a/ipc/namespace.c
+++ b/ipc/namespace.c
@@ -154,11 +154,11 @@ static void *ipcns_get(struct task_struct *task)
        struct ipc_namespace *ns = NULL;
        struct nsproxy *nsproxy;
 
-       rcu_read_lock();
-       nsproxy = task_nsproxy(task);
+       task_lock(task);
+       nsproxy = task->nsproxy;
        if (nsproxy)
                ns = get_ipc_ns(nsproxy->ipc_ns);
-       rcu_read_unlock();
+       task_unlock(task);
 
        return ns;
 }

diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index 8e7811086b826cbc42b6be594d2fc605fee0b84d..ef42d0ab3115709adbe4b4a27c71f3638a481954 100644 (file)
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -204,20 +204,13 @@ void switch_task_namespaces(struct task_struct *p, struct nsproxy *new)
 
        might_sleep();
 
+       task_lock(p);
        ns = p->nsproxy;
+       p->nsproxy = new;
+       task_unlock(p);
 
-       rcu_assign_pointer(p->nsproxy, new);
-
-       if (ns && atomic_dec_and_test(&ns->count)) {
-               /*
-                * wait for others to get what they want from this nsproxy.
-                *
-                * cannot release this nsproxy via the call_rcu() since
-                * put_mnt_ns() will want to sleep
-                */
-               synchronize_rcu();
+       if (ns && atomic_dec_and_test(&ns->count))
                free_nsproxy(ns);
-       }
 }
 
 void exit_task_namespaces(struct task_struct *p)

diff --git a/kernel/utsname.c b/kernel/utsname.c
index fd393124e507f15f6d366a0f41e4808b81c64934..883aaaa7de8a90f28f13099b3fa77b222c20bd9c 100644 (file)
--- a/kernel/utsname.c
+++ b/kernel/utsname.c
@@ -93,13 +93,13 @@ static void *utsns_get(struct task_struct *task)
        struct uts_namespace *ns = NULL;
        struct nsproxy *nsproxy;
 
-       rcu_read_lock();
-       nsproxy = task_nsproxy(task);
+       task_lock(task);
+       nsproxy = task->nsproxy;
        if (nsproxy) {
                ns = nsproxy->uts_ns;
                get_uts_ns(ns);
        }
-       rcu_read_unlock();
+       task_unlock(task);
 
        return ns;
 }

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 85b62691f4f2d18b9b39bc9c610d1916f8f09273..7c6b51a58968613010576f3bd35a3cb6c527db08 100644 (file)
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -373,9 +373,11 @@ struct net *get_net_ns_by_pid(pid_t pid)
        tsk = find_task_by_vpid(pid);
        if (tsk) {
                struct nsproxy *nsproxy;
-               nsproxy = task_nsproxy(tsk);
+               task_lock(tsk);
+               nsproxy = tsk->nsproxy;
                if (nsproxy)
                        net = get_net(nsproxy->net_ns);
+               task_unlock(tsk);
        }
        rcu_read_unlock();
        return net;
@@ -632,11 +634,11 @@ static void *netns_get(struct task_struct *task)
        struct net *net = NULL;
        struct nsproxy *nsproxy;
 
-       rcu_read_lock();
-       nsproxy = task_nsproxy(task);
+       task_lock(task);
+       nsproxy = task->nsproxy;
        if (nsproxy)
                net = get_net(nsproxy->net_ns);
-       rcu_read_unlock();
+       task_unlock(task);
 
        return net;
 }