Q: utrace && ptrace_check_attach()

Roland McGrath roland at redhat.com
Sat Jul 25 00:19:33 UTC 2009


> > Yes, I think you're right.  I think the "if (unlikely(utrace->stopped)) {"
> > case in utrace_get_signal() needs to do some variant of utrace_reset.  If
> > any engine_wants_stop() then it should not dequeue a signal even if one is
> > ready, and probably it should also not do any spontaneous report.
> 
> Hmm. If ->stopped == T after return from do_signal_stop()->schedule(),
> then we must have the engine which is engine_wants_stop(), no?

Hmm.  If he went away, he would have done utrace_wakeup, so yes.

> Damn. Just can't think today...

I think you're doing better than I am. ;-)

> Roland, can we just remove all code which plays with ->stopped in
> utrace_report_jctl() and utrace_get_signal(), and do something like
[...]
> 	@@ -1602,6 +1602,16 @@ static int do_signal_stop(int signr)
> 		do {
> 			schedule();
> 		} while (try_to_freeze());
> 	+
> 	+	// needs a tracehook helper
> 	+	if (utrace->stopped) {

Hmm.  I guess we could.  I think my original thinking was just minimizing
the number of places in the core kernel code where we inserted any
tracehook call, keeping them to about exactly where the ptrace hooks had
been.  But now I'd say it is certainly fine to insert another hook in the
signals code if it keeps the utrace code cleaner.

I recall you wanted to rework something about tracehook_notify_jctl earlier
too.  Now is the time to revisit all that and clean the code up however
seems best.  It shouldn't be a problem to rewrite the last half of
do_signal_stop() and one (or two) tracehook_*_jctl interfaces to be optimal
for utrace.

We should also think about having utrace_do_stop() change TASK_STOPPED into
TASK_TRACED like the old ptrace_check_attach() does.  That way SIGCONT just
won't wake it up in the first place.  I think the only reason I avoided
that before may have been the dual code paths with old ptrace, but if we
are now doing a cleanup that entirely replaces the old ptrace guts, that is
not an issue.  The existing utrace_wakeup logic for turning TASK_TRACED
back into TASK_STOPPED might be OK as it is.

The perhaps we don't actually need the ->stopped flag.  
It means exactly:
	->state == TASK_TRACED ||
	(->exit_state && !(->utrace_flags & _UTRACE_DEATH_EVENTS))
which can be checked under utrace->lock.

What do you think?


Thanks,
Roland




More information about the utrace-devel mailing list