Re: [PATCH] prctl: add PR_{SET,GET}_CHILD_SUBREAPER to allow simple process supervision

[Andrew Morton <akpm@linux-foundation.org>]

On Mon, 9 Jan 2012 16:07:01 +0100
Oleg Nesterov <oleg@redhat.com> wrote:

> > > +		/* find the first ancestor marked as child_subreaper */
> > > +		for (reaper = father->real_parent;
> > > +		     reaper != &init_task;
> > > +		     reaper = reaper->real_parent) {
> >
> > I admit being insufficiently caffienated - does this DTRT in a PID namespace? That
> > &init_task looks fishy to me...
> 
> Probably this needs a comment. Initially I was confused too.
> 
> Note that the code below checks same_thread_group(reaper, pid_ns->child_reaper),
> this is what we need to DTRT in a PID namespace. However we still need the
> check above, see http://marc.info/?l=linux-kernel&m=131385460420380

In light of Kay's haughty silence, I did this:

--- a/kernel/exit.c~prctl-add-pr_setget_child_subreaper-to-allow-simple-process-supervision-fix-fix
+++ a/kernel/exit.c
@@ -724,7 +724,13 @@ static struct task_struct *find_new_reap
 	} else if (father->signal->has_child_subreaper) {
 		struct task_struct *reaper;
 
-		/* find the first ancestor marked as child_subreaper */
+		/*
+		 * Find the first ancestor marked as child_subreaper.
+		 * Note that the code below checks same_thread_group(reaper,
+		 * pid_ns->child_reaper).  This is what we need to DTRT in a
+		 * PID namespace. However we still need the check above, see
+		 * http://marc.info/?l=linux-kernel&m=131385460420380
+		 */
 		for (reaper = father->real_parent;
 		     reaper != &init_task;
 		     reaper = reaper->real_parent) {
_

I'm not a fan of URLs-in-comments, but that email was too gnarly to be
condensed into a sane comment.