Messages in this thread | | | Date | Mon, 27 Jun 2011 10:24:43 +0200 | From | Tejun Heo <> | Subject | Re: [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
| |