lkml.org 
[lkml]   [2011]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/8] kill tracehook_notify_death()
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


\
 
 \ /
  Last update: 2011-06-27 10:39    [W:0.149 / U:0.756 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site