Re: + cgroups-more-safe-tasklist-locking-in-cgroup_attach_proc.patch added to -mm tree

[Oleg Nesterov <oleg@redhat.com>]

On 09/08, Ben Blum wrote:
>
> On Thu, Sep 08, 2011 at 11:31:30PM +0200, Oleg Nesterov wrote:
> > On 09/08, Ben Blum wrote:
> > >
> > > As for the patch below (which is the same as it was last time?):
> >
> > It is the same, yes, I simply copied it from my old email. BTW, it
> > wasn't tested at all, even compiled.
>
> Testing is recommended ;)

Heh, that is why I didn't send the patch "officially". Not to mention
I never used cgroups.

> If you polished this patch off, I'd be happier, since I have a lot else
> on my plate.

Same here ;)

> > Off-topic question... Looking at this code, it seems that
> > attach_task_by_pid(zombie_pid, threadgroup => true) returns 0.
> > single-task-only case fails with -ESRCH in this case. I am not
> > saying this is wrong, just this looks a bit strange (unless I
> > misread the code).
>
> yeah, this is a side-effect of cgroup_attach_proc continuing to iterate
> in case any one of the sub-threads be still alive. you could keep track
> of whether all threads were zombies and return -ESRCH then,

Yes I see. Although PF_EXITING && thread_group_empty() could help.

> but it
> shouldn't matter, since the user-facing behaviour is indistinguishable
> from if the user had sent the command just before the task turned zombie
> but while it was still about to exit.

Not sure. A task can spend a lot of time being zombie. In this case
we return success or -ESRCH depending on threadgroup.

But I agree, this is minor.

Oleg.