Re: +prctl-add-pr_setget_child_reaper-to-allow-simple-process-supervision.patch added to -mm tree

[Oleg Nesterov]

Hello Kay,

I need to go away, I'll read this patch (and the whole email) tomorrow.

Just a quick note right now,

On 08/18, Kay Sievers wrote:
>
>  static struct task_struct *find_new_reaper(struct task_struct *father)
>  	__releases(&tasklist_lock)
> @@ -724,6 +725,19 @@ static struct task_struct *find_new_reap
>  		 * forget_original_parent() must move them somewhere.
>  		 */
>  		pid_ns->child_reaper = init_pid_ns.child_reaper;
> +	} else {
> +		/* find the first ancestor which is marked as child_reaper */
> +		for (thread = father->real_parent;
> +		     thread != thread->real_parent;
> +		     thread = thread->real_parent) {
> +			if (thread == pid_ns->child_reaper)
> +				break;
> +			if (!thread->signal->child_reaper)
> +				continue;
> +			if (thread->flags & PF_EXITING)
> +				continue;
> +			return thread;

No, this doesn't look right.

This code should do something like

		for (reaper = father->real_parent;
		     !same_thread_group(reaper, pid_ns->child_reaper);
		     reaper = reaper->real_parent) {
			if (!signal->child_reaper)
				continue;

			if (there is a !PF_EXITING thread)
				return thread;
		}

And I forgot to mention, could you please-please rename child_reaper?
Say, is_child_reaper or is_sub_reaper. Or whatever. I do not really
care about the naming. But I use grep very often, and personally I
dislike the task->child_reaper/signal->child_reaper confusion.

Oleg.

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