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:
>
> 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.

> did you
> mean for Andrew to replace the old tasklist_lock patch with this one, or
> should one of us rewrite this against it?

This is up to you. And just in case, please feel free to do nothing and
keep the current fix.

> either way, it should have
> something like the comment I proposed in the first thread.

Confused... Aha, I missed another email from you. You mean

	/* If the leader exits, its links on the thread_group list become
	 * invalid. One way this can happen is if a sub-thread does exec() when
	 * de_thread() calls release_task(leader) (and leader->sighand gets set
	 * to NULL, in which case lock_task_sighand will fail). Since in that
	 * case the threadgroup is still around, cgroup_procs_write should try
	 * again (finding the new leader), which EAGAIN indicates here. This is
	 * "double-double-toil-and-trouble-check locking". */

Agreed.





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).

Oleg.