On 08/17, Oleg Nesterov wrote:
>
> 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
>
> After that start_callback() skips "if (want & UTRACE_EVENT(QUIESCE))" block
> and returns utrace_detached_ops, utrace_detached_ops->report_exec == NULL
> will be called.

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) {


> But this is minor. Roland, I can't explain this, but I have the strong
> gut feeling there is something else we should worry about. I am still
> reading this code...

Or not... I'll recheck once again tomorrow and try to make the patch.

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().

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.


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?

Oleg.

Reply via email to