Re: [PATCHv2 5/7] cgroup: introduce cgroup namespaces

[Andy Lutomirski <luto@amacapital.net>]

On Fri, Oct 31, 2014 at 12:18 PM, Aditya Kali <adityakali@google.com> wrote:
> Introduce the ability to create new cgroup namespace. The newly created
> cgroup namespace remembers the cgroup of the process at the point
> of creation of the cgroup namespace (referred as cgroupns-root).
> The main purpose of cgroup namespace is to virtualize the contents
> of /proc/self/cgroup file. Processes inside a cgroup namespace
> are only able to see paths relative to their namespace root
> (unless they are moved outside of their cgroupns-root, at which point
>  they will see a relative path from their cgroupns-root).
> For a correctly setup container this enables container-tools
> (like libcontainer, lxc, lmctfy, etc.) to create completely virtualized
> containers without leaking system level cgroup hierarchy to the task.
> This patch only implements the 'unshare' part of the cgroupns.
>

> +       /* Prevent cgroup changes for this task. */
> +       threadgroup_lock(current);

This could just be me being dense, but what is the lock for?

> +
> +       /* CGROUPNS only virtualizes the cgroup path on the unified hierarchy.
> +        */
> +       cgrp = get_task_cgroup(current);
> +
> +       err = -ENOMEM;
> +       new_ns = alloc_cgroup_ns();
> +       if (!new_ns)
> +               goto err_out_unlock;
> +
> +       err = proc_alloc_inum(&new_ns->proc_inum);
> +       if (err)
> +               goto err_out_unlock;
> +
> +       new_ns->user_ns = get_user_ns(user_ns);
> +       new_ns->root_cgrp = cgrp;
> +
> +       threadgroup_unlock(current);
> +
> +       return new_ns;
> +
> +err_out_unlock:
> +       threadgroup_unlock(current);
> +err_out:
> +       if (cgrp)
> +               cgroup_put(cgrp);
> +       kfree(new_ns);
> +       return ERR_PTR(err);
> +}
> +
> +static int cgroupns_install(struct nsproxy *nsproxy, void *ns)
> +{
> +       pr_info("setns not supported for cgroup namespace");
> +       return -EINVAL;
> +}
> +
> +static void *cgroupns_get(struct task_struct *task)
> +{
> +       struct cgroup_namespace *ns = NULL;
> +       struct nsproxy *nsproxy;
> +
> +       rcu_read_lock();
> +       nsproxy = task->nsproxy;
> +       if (nsproxy) {
> +               ns = nsproxy->cgroup_ns;
> +               get_cgroup_ns(ns);
> +       }
> +       rcu_read_unlock();

How is this correct?  Other namespaces do it too, so it Must Be
Correct (tm), but I don't understand.  What is RCU protecting?

--Andy