Re: [PATCH 01/17] ptrace: remove silly wait_trap variable fromptrace_attach()

[Oleg Nesterov]

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.

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