Message-ID: <20110511131314.GF1661@htj.dyndns.org>
Date:	Wed, 11 May 2011 15:13:14 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Denys Vlasenko <vda.linux@...glemail.com>
Cc:	oleg@...hat.com, jan.kratochvil@...hat.com,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
	akpm@...ux-foundation.org, indan@....nu
Subject: Re: [PATCH 11/11] ptrace: implement group stop notification for
 ptracer

Hey,

On Wed, May 11, 2011 at 02:01:16PM +0200, Denys Vlasenko wrote:
> On Wed, May 11, 2011 at 11:05 AM, Tejun Heo <tj@...nel.org> wrote:
> > Debugger may continue tracee and put it in a different trap.  In such
> > cases, group stop may be initiated and lifted multiple times while a
> > tracee is trapped.  It's much nicer to have symmetric notifications in
> > those cases.
> 
> I don't understand what exactly you mean by this.
> You mean that it's better to have one INTERRUPT trap per each
> stop/cont transition? Such as:
> 
> tracer                  tracee                  3rd process
> waitpid <---------------some ptrace stop
> <tracer is doing something,
>  not yet ptrace(CONT)ing>
>                         <-----------------------kill -STOP
>                         <-----------------------kill -CONT
>                         <-----------------------kill -STOP
>                         <-----------------------kill -CONT
> ptrace(CONT)----------->
> waitpid <---------------INTERRUPT
> ptrace(GETSIGINFO)<-----"it's a group stop"
> waitpid <---------------INTERRUPT
> ptrace(GETSIGINFO)<-----"it's a cont"
> ptrace(CONT)----------->
> waitpid <---------------INTERRUPT
> ptrace(GETSIGINFO)<-----"it's a group stop"
> waitpid <---------------INTERRUPT
> ptrace(GETSIGINFO)<-----"it's a cont"
> ptrace(CONT)----------->
> ............................................................
> 
> I agree that it's not a bad thing to deliver ALL notifications.

No, not at all.

> But from what I understood, with your patch it won't do this either:
> you don't remember every stop and cont in a queue, you have just one bit?

What I want to guarantee is that after one or more events, at least
one notification is always delivered in finite userland time.

> And second, I don't think we really need this. We only need this:
> if tracer did see group stop, it MUST see cont (or kill) - cont
> should never be missed. But it's ok to miss whole pairs of stop+cont
> which were done in quick succession (and similarly for cont+stop
> pairs on a stopped tracee, as in example above).

Yeah, sure.  There's no event queueing and I'm not gonna add one.

> > Also, we might end up adding more status flags to si_pt_flags and it's
> > much better to be able to say "INTERRUPT becomes and stays pending
> > until the next GETSIGINFO if any of these flags may have chagned" than
> > "if XXX change from x to y, or YYY changes to x to y or y to x,
> > ... blah blah".
> 
> Why do you want this data to be sticky at all?
> Do you think tracer will "forget" to retrieve it?
> That'd be a bug in the tracer if it doesn't retrieve the data
> and therefore mishandles stop and/or cont.

Please read on.

> >> Can we just add a "group cont" notification which looks like
> >> a waitpid result with WIFCONTINUED(waitpid_status) == 1 to the tracer?
> >
> > Consider the following scenario.
> >
> > 1. A syscall traced tracee enters group stop while returning from a
> >   syscall.  Tracer is notified.
> >
> > 2. SIGCONT generated.  Tracee is woken up and wait state is generated.
> >
> > 3. Tracee traps for syscall completion before tracer has chance to
> >   wait(2).
> 
> The order in (3) seems wrong:
> Tracee doesn't enter group stop while it is in syscall.
> It exits syscall first, and then it enters group stop.
> Therefore in step (3) above it can't "trap for syscall completion".
> It can trap for syscall entry at worst; however:

Okay, whatever other trap then.

> > 4. Oops, continued wait state lost.
> 
> Why is it lost? Why kernel signals syscall entry instead of CONT
> in your scenario?

SIGCONT doesn't have to be delivered by the specific task being traced
and the userland can continue executing without any specific time
limit breaking the 'in finite userland time' part of the guarantee.

> The rest isn't interesting (typical buggy handling of stop
> notification in current strace/kernel):

The whole thing is uninteresting.  The trap type doesn't matter at
all.  The only thing matter is that PTRACE_CONT may race with end of
group stop and control may return to userland and event can be lost.

> Can you describe your scenario in more details. I don't understand it.

Stop thinking about strace(1).  It's the most trivial case.  A tracee
can be in any trap regardless of group stop state and tracer can issue
PTRACE_CONT anytime which can race with SIGCONT generation.  You want
to deliver the notification to the tracer in finite userland time.
Think about SIGCONT and PTRACE_CONT racing each other and SIGCONT
being delivered to another task.

> Why tracer needs to schedule anything? As you see above,
> at this point tracer is in waitpid anyway. It is ALREADY prepared
> to get cont notification. It doesn't need to generate any extra
> ptrace stops for that.

The debugger doesn't want the debuggee to be running.  It wants tracee
to be in basically the same state as in group stop after the temporary
inspection.  It doesn't want to leave it running while group stop is
in effect but still wants to be notified of end of group stop.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists