Re: + cgroups-fix-ordering-of-calls-in-cgroup_attach_proc.patchadded to -mm tree

From: Tejun Heo
Date: Fri Aug 26 2011 - 11:21:47 EST


Hello, Oleg.

On Fri, Aug 26, 2011 at 05:12:45PM +0200, Oleg Nesterov wrote:
> Yes, I think this is what we need, the patch itself looks fine.
>
> But this doesn't answer my another question. After that the code does
>
> * step 4: do expensive, non-thread-specific subsystem callbacks.
>
> ss->attach(ss, cgrp, oldcgrp, leader);
>
> OK, non-thread-specific is nice, but how can this "leader" represent
> the process?
>
> It can be zombie (but still group_leader) even without any races.
> Say, cpuset_attach() and mem_cgroup_move_task() need get_task_mm(p).
> How this can work if the leader is dead?
>
> Also. Even if we add the locking around while_each_thread() (btw,
> we need this in any case), we can race with exec which can change
> the leader. In this case this task_struct has nothing to do with
> the process we are going to attach, at all.
>
> And, ss->can_attach(leader) has the same problems, it seems.

Please take a look at the following series.

http://thread.gmane.org/gmane.linux.kernel/1184375

attach racing against exit/exec is problematic and maybe we should
extend task->threadgroup_fork_lock protection to cover both exit and
exec. I can't like it tho. I hope this can be somehow done in
lighter way. cgroup attaches are quite cold paths and we're putting
an extra rwsem in each task for that.

Thanks.

--
tejun
--
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/