[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: Q: utrace_stop() && notification



> The usage of ->ptrace in utrace-ptrace is racy [...]

Sure.  Replace it.  I'm not sure what purpose ->ptrace (or ctx->flags)
serves for attach/detach.  You need somewhere to store the
PTRACE_SETOPTIONS state and so forth, sure.  But you can probably just
handle the attachedness at the utrace level.  That's what
UTRACE_ATTACH_EXCLUSIVE is for.

> I think we need another variable which should be used instead, and
> the only "good" place where it can live is engine->data, it should
> point to "struct ptrace_xxx".

Of course!  That was the main point of the whole ptrace-tracee data
structure cleanup.

> Yes, I see. That is why I named it "notify_stopped", not report_xxx,
> to emphasize it is really special and should be used with care and only
> when really needed.

Right, I got that.

> So, if we do not want to add the special (and not-a-report) hooks
> into engine->ops, then perhaps it makes sense to do
[...]

That is exactly the kludge the bad old utrace-ptrace.patch does.

> So, perhaps we can just add engine->ops->notify_stopped() (or even [...]

My main point was that we should look for an entirely different way to
attack the problem.  If I thought this hook would only be used for
ptrace, it might be fine.  (OTOH then it's not all that much better than
the terrible ptrace-only kludge.)  But AFAICT we'd likely wind up with
every other new utrace module that cares about doing synchronization
with debugger threads needing to use this hook too.  That's why I want
to talk about the general case in the utrace API, and not just ptrace.

Let's ignore SIGCHLD for a moment, and just think about ptrace's
report_* waking up wait.  That is like what we expect will be the
general case of this: a debugger thread wants to sleep somehow and have
its report_* callbacks say "wake the debugger so it can examine me".

So, how does the utrace module writer build that?  You'd probably use
wait_event() et al in your debugger thread, and wake_up() in report_*
callbacks.  (That's approximately what you have in ptrace+wait.)

So how much is satisfied with a "delayed wake-up" (rather than an
open-ended hook).  Let's say:

	utrace_wake_up(task, engine, wqh);	// replaces wake_up(wqh);
	return UTRACE_STOP; // | UTRACE_SIGNAL_IGN|UTRACE_SIGNAL_HOLD

That would mean, approximately, as if wake_up(wqh) and then the wakee
had called utrace_barrier(task,engine), but without the sched flutter.

Formally, I think the guarantee should be "when utrace_barrier would
return for you".  Technically this means that if your callback did not
use UTRACE_STOP, it's just sometime after your callback returns.  But if
you did use UTRACE_STOP, then it means after the task will definitely
stop.  That's all it means today, but I think we want to strengthen that
guarantee so that when you have used UTRACE_STOP (either in a callback
already in progress when you called utrace_barrier, or in a utrace_control
call before the utrace_barrier call), utrace_barrier doesn't return until
"utrace_prepare_examine will work", i.e. actually in TASK_TRACED/STOPPED.

In ptrace, I think this alone might suffice to cover things "cleanly" in
the utrace API.  (That is, clean for utrace. ;-) The ptrace layer could
use the trick of a custom wake function to implement SIGCHLD without
needing anything but utrace_wake_up from the utrace layer.  That is at
base the same thing as your notify_stopped hook--ptrace will supply a hook
that runs in very special circumstances.  But for the more common case in
other modules, using utrace_wake_up/wait_event this way can be very
straightforward and foolproof.

Thoughts?


Thanks,
Roland


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]