Re: [PATCH 2/8] kill tracehook_notify_death()

[Tejun Heo <tj@kernel.org>]

Hello, Oleg.

On Sun, Jun 26, 2011 at 10:51:57PM +0200, Oleg Nesterov wrote:
> > Yeap, we've discussed this before and this indeed is odd.  However, is
> > there something ptracer can't do with PTRACE_EVENT_EXIT instead?
> 
> Firstly, I think PTRACE_EVENT_EXIT should not stop the tracee if it
> was SIGKILL'ed. Even if the tracee stops, it can be killed later.
> The tracer can't detach after that, it can't even wait() to detecte
> a zombie leader.

For SIGKILL, yes, it is different, but if PTRACE_EVENT_EXIT is enough
for all other cases, I think we're mostly set.  BTW, it seems like we
would actually stop at PTRACE_EVENT_EXIT even after SIGKILL.  This is
wrong & racy.  may_ptrace_stop() should be checking for
sigkill_pending(), right?

> > Heh, I think this needs to be prettier even at the cost of an inline
> > function.
> 
> May be, but the code is sooooo simple. In fact I thought about the helper,
> but can't find a good name.
> 
> Anyway, this is so minor, unless you strongly object I am going to push
> this patch as is. We can add a helper later although I don't think it is
> needed.
> 
> The same logic could be written as
> 
> 	if (thread_group_empty(tsk)) {
> 		int sig = ptrace_reparented(tsk) ?
> 			SIGCHLD : tsk->exit_signal;
> 		autoreap = do_notify_parent(tsk, sig);
> 	} else if (task->ptrace) {
> 		autoreap = do_notify_parent(tsk, SIGCHLD);
> 	} else {
> 		autoreap = !thread_group_leader();
> 	}
> 
> note that it certainly looks "prettier". However, personaly I strongly
> prefer the non-pretty code above, imho it is more straighforward and
> understandable. It is hardly possible to misread/misunderstand it.

Well, it's cosmetic after all and you're the maintainer.  I don't have
any major problem with the original.  Please go ahead.

Thanks.

-- 
tejun