Re: [PATCH 0/2] CGroups: cgroup member list enhancement/fix

[Matt Helsley <matthltc@us.ibm.com>]

On Tue, Jul 14, 2009 at 01:38:30PM -0700, Paul Menage wrote:
> On Tue, Jul 14, 2009 at 10:43 AM, Paul Menage<menage@google.com> wrote:
> >
> > I've been trying to think of a way to do that. AFAICS the only way to
> > do that reliably would be to move the call to cgroup_fork() that hooks
> > into the parent's cgroup inside the lock on the group leader's thread
> > list, and move the fork callbacks into cgroup_fork(). (Which would
> > mean that they'd not be able to sleep/fail, etc).
> 
> Currently the only user of the cgroup fork callbacks is the freezer cgroup.
> 
> Matt, if this callback was moved inside tasklist_lock, would that
> present any problems? Given that in other places you call
> freeze_task() from inside other low-level locks like css_set_lock
> (within a cgroup iteration) then hopefully it would be OK.

Yes, it should be OK.

> 
> The only question then would be whether anything between the point
> where cgroup_fork() is currently called, and the point where the new
> thread is added to its thread group list, cares about p->cgroups being
> valid. We can probably flush out any such assumptions by clearing
> tsk->cgroups in dup_task_struct, so that any attempts to reference it
> would immediately oops.

This is harder to verify the correctness of. You're probably right but
I'm not completely convinced yet.

Cheers,
	-Matt