В Вт, 16/06/2015 в 19:07 +0300, Cyrill Gorcunov пишет:
> On Tue, Jun 16, 2015 at 06:19:44PM +0300, Kirill Tkhai wrote:
> > 
> > This patch brings a couple of problems. The first one is if we're setting
> > a ve cgroup for a forking task, it's possible the parent and the child
> > fall into different cgroups. For example, parent is inside CT, while
> > child is in ve0.
> > 
> > The second problem is that it is racy with all places, where we're doing
> > get_exec_env() and hoping on a stable result. There are a lot of places,
> > we use it, so every place need too be checked.
> > 
> > The first problem can be solved using cgroup_postfork() method, while
> > the second is worse. The number of get_exec_env() is big, and it tends
> > to increase.  We could check all of them, but an universal solutions
> > will be better. But it seems there is no easy universal solution.
> > 
> > We can implement a task lock and use get_exec_env() under it. 
> > We can use something like rcu_read_{lock,unlock} braces around 
> > get_exec_env(),
> > and use synchronize_rcu() in cgroup_postfork().
> > 
> > The two solutions above are examples what we can do. What do you think?
> 
> May not we simply add into ve cgroup?
> 
> .can_attach
>       ...
>       spin_lock(&task->sighand->siglock);
> .cance_attach
>       ...
>       spin_unlock(&task->sighand->siglock);
> .attach
>       ...
>       spin_unlock(&task->sighand->siglock);

It seems sighand lock is not good for this, because cgroup_attach_task()
iterating over all subsys:

for_each_subsys(root, ss) {
                if (ss->can_attach) {

so we may bumped into wrong lock order in one of them. Also this solves
problem №1, but does not solve problem №2.

Hm. How about stop_machine? It solves all of the problems. Also, it shouldn't
worsen performance, because this action is rare.

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to