Messages in this thread | | | Date | Wed, 1 Jun 2011 20:47:43 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 01/17] ptrace: remove silly wait_trap variable from ptrace_attach() |
| |
On 05/30, Tejun Heo wrote: > > Local variable wait_trap is used to determine whether to call > wait_event() on TRAPPING or not, which doesn't change or optimize > anything. Remove it. > > ... > > - if (wait_trap) > - wait_event(current->signal->wait_chldexit, > - !(task->group_stop & GROUP_STOP_TRAPPING)); > + wait_event(current->signal->wait_chldexit, > + !(task->group_stop & GROUP_STOP_TRAPPING)); > return retval;
Well, it doesn't change anything, but only if ptrace_attach() succeeds. The caller should not wait if STOP_TRAPPING was already set by another tracer and we are going to fail. Afaics, nothing really bad can happen but still this doesn't look very clean.
And. Please note that this patch is buggy until 8/17 "use bit_waitqueue for TRAPPING", wait_event(current->signal->wait_chldexit) can hang forever in this case since we are not ->parent.
I agree, wait_trap should go away. We can hit STOP_TRAPPING after attach if we change detach to set this bit. But perhaps it would be more clean to not call wait_event/wait_bit unconditionally anyway, we can check retval == 0.
Oleg.
| |