> > report_reap is covered with the utrace_detached_reap() stub. Every other
> > callback uses start_callback(), i.e. calls report_quiesce first.
> > utrace_detached_quiesce() returns UTRACE_DETACH,
>
> Assuming ->report_quiesce() will be called.
>
> Suppose that, say, engine->flags == UTRACE_EVENT(EXEC) (no QUIESCE).
>
> 1. start_callback() reads want = engine->flags (== EXEC)
>
> 2. mark_engine_detached() sets engine->ops = &utrace_detached_ops
>
> 3. start_callback() gets ops = utrace_detached_ops
Right, good point.
> OK, instead of filling utrace_detached_ops{} we can change start_callback()
>
> - if (want & UTRACE_EVENT(QUIESCE)) {
> + if ((want & UTRACE_EVENT(QUIESCE)) || ops == detached_ops) {
If we're going to have a special-case check for &utrace_detached_ops, then
it can just short-circuit entirely and there is no need for any callbacks
in utrace_detached_ops. (Then we might even use NULL or (void *)-1l
instead of &utrace_detached_ops.) All it needs to do is:
report->detaches = true;
utrace->reporting = NULL;
return NULL;
> In particular, I'd like to think if we can simplify this somehow.
> Say, avoid the barriers somewhere, or avoid changing engine->flags in
> mark_engine_detached().
Yeah. The original idea of changing engine->flags was just to make sure it
had QUIESCE so that it would be noticed and detached on any next callback
run, not just one that happened to be for the events it was previously
tracing (which could be only rare ones, or none but implicit reap). But if
we have a special case check in start_callback() before even looking at the
flags, then it doesn't matter at all what the flags are.
> I am worried that mark_engine_detached() sets engine->flags, but it
> doesn't add QUIESCE to utrace_flags. I can't convince myself that
> utrace_do_stop() closes all possible races.
Ok. The worst case races should not be real problems, only delays in the
final engine cleanup. Any eventual utrace_reset() or reaping will of
course clean it up at some point.
> And, in particular, I don't understand this code in utrace_get_signal:
>
> } else if (!(task->utrace_flags & UTRACE_EVENT(QUIESCE))) {
> /*
> * We only got here to clear utrace->signal_handler.
> */
> return -1;
> }
>
> How so? What if we were called because of utrace_control(INTERRUPT)
> and QUIESCE is not set?
If an engine used UTRACE_INTERRUPT without having first set QUIESCE,
then it has no rightful expectation of getting any callback.
Thanks,
Roland
