Messages in this thread | | | Date | Wed, 11 May 2011 18:49:47 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 10/11] ptrace: move JOBCTL_TRAPPING wait to wait(2) and ptrace_check_attach() |
| |
On 05/08, Tejun Heo wrote: > > TRAPPING will also be used to implement end of group stop retrapping, > which can be initiated by tasks other than tracer. To allow this,
I didn't read the next patch yet, so I can't undestand/comment the motivation.
But,
> this patch moves TRAPPING wait from attach completion path to > operations which are actually affected by the transition - wait(2) and > following ptrace(2) requests.
You know, I'd wish I could find the serious bugs in this patch. The code becomes really hairy. -EAGAIN in do_wait() doesn't make it more simple ;)
> Both wait and ptrace paths are updated to retry the operation after > TRAPPING wait. Note that wait_task_stopped() now always grabs siglock > for ptrace waits. This can be avoided with "task_stopped_code() -> > rmb() -> TRAPPING -> rmb() -> task_stopped_code()" sequence
And so far I think this would be better, because it seems we can avoid the retry logic.
First of all, this patch returns one of the user-visible and undesirable changes. The tracer know that the task is stopped, attaches, and then it can see the TASK_RUNNING tracee after ptrace(PTRACE_ATTACH) returns.
I agree, this looks minor. But if we can tolerate this, probably we can tolerate another oddity: wait_task_stopped() can succeed and eat the stop code before the tracee actually stopps, no?
IOW, ignoring mb's and read-ordering, suppose that we simply change task_stopped_code:
if (ptrace) { - if (task_is_stopped_or_traced(p)) + if (task_is_traced(p) || JOBCTL_TRAPPING) return &p->exit_code; } else {
As for ptrace_check_attach(), it can simply do wait_event(), we only need to verify the caller is the tracer. No need to play with lock/unlock/retry.
What do you think?
Oleg.
| |