Re: [PATCH 10/11] ptrace: move JOBCTL_TRAPPING wait to wait(2) andptrace_check_attach()

[Oleg Nesterov]

On 05/11, Tejun Heo wrote:
>
> On Wed, May 11, 2011 at 06:49:47PM +0200, Oleg Nesterov wrote:
> > On 05/08, Tejun Heo wrote:
> > > 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 ;)
>
> I don't know.  Why is retrying hairy?  The whole waiting logic is
> built for clean retries.  The suggested change just does it without
> intervening sleeping and waking up.  I don't see anything particularly
> hairy there.

As always, this is subjective. But I didn't mean -EAGAIN itself. In fact
I was going to add this (simple) logic some time ago and kill the EXIT_DEAD
state. Hmm, and I'd still like to do this...

I meant the whole ptrace_wait_trapping() + lock dance + retry thing.
But of course I do not pretend my feeling is right.

Also. _Perhaps_ we can rethink the SIGCONT trapping, and perhaps in
this case do_wait() won't need any changes. May be.

> > > 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.
>
> Well, the above memory barrier dance wouldn't really change whether
> retry logic is required or not and I'd _really_ like to avoid complex
> barrier dances.

Agreed, the barriers always complicate the understanding.

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/