On 08/19, Roland McGrath wrote:
>
> > 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;
Yes, I thought about this too, see the changelog.
But probably we need utrace_detached_ops->report_reap() anyway.
->report_death can return UTRACE_DETACH, and after that utrace_report_death()
can skip utrace_reset() and do ->report_reap().
> > 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.
Hmm. This is certainly new to me. Could you confirm?
If yes, shouldn't we change utrace_control()
- if (action >= UTRACE_REPORT && action < UTRACE_RESUME &&
+ if (action >= UTRACE_INTERRUPT && action < UTRACE_RESUME &&
unlikely(!(engine->flags & UTRACE_EVENT(QUIESCE)))) {
then?
I must admit, I do not understand why INTERRUPT needs QUIESCE.
utrace_get_signal() should respect QUIESCE, this is clear. But why
it is needed?
Oleg.
