utrace_set_events/utrace_control && death/reap checks

[Oleg Nesterov]

utrace_set_events:

        (utrace->death && ((old_flags & ~events) & DEATH_EVENTS))

"(old_flags & ~events) & DEATH_EVENTS)" means the caller tries to
clear DEATH/QUIESCE. Why this is not allowed? And why this is not
allowed _only_ when the target runs utrace_report_death()->REPORT()?

I think this line can be just killed. I guess the intent was to
prevent utrace_release_task() from doing utrace_reap() in parallel
with utrace_report_death(), but note that utrace_set_events() can
never "shrinks" ->utrace_flags, it only sets new bits.

The next line looks strange too, don't we need

        (utrace->reap && ((events & ~old_flags) & UTRACE_EVENT(REAP)))

?


And I don't understand why do we need utrace->death at all. Apart from
utrace_set_events (which I think doesn't need it), it is only used by
utrace_control(UTRACE_DETACH). But I can't see how can we race with
utrace_report_death(). If it can be called, we have DEATH_EVENTS bits
set. But in that case utrace_do_stop() can't succeed, so UTRACE_DETACH
can only do mark_engine_wants_stop() but not utrace_reset().

IOW, could you explain why the patch below is wrong? (and why can't
we kill ->death then).

Oleg.

--- kernel/utrace.c
+++ kernel/utrace.c
@@ -1072,27 +1072,10 @@ int utrace_control(struct task_struct *t
                /*
                 * You can't do anything to a dead task but detach it.
                 * If release_task() has been called, you can't do that.
-                *
-                * On the exit path, DEATH and QUIESCE event bits are
-                * set only before utrace_report_death() has taken the
-                * lock.  At that point, the death report will come
-                * soon, so disallow detach until it's done.  This
-                * prevents us from racing with it detaching itself.
                 */
-               if (action != UTRACE_DETACH ||
-                   unlikely(utrace->reap)) {
+               if (action != UTRACE_DETACH || unlikely(utrace->reap)) {
                        spin_unlock(&utrace->lock);
                        return -ESRCH;
-               } else if (unlikely(target->utrace_flags & DEATH_EVENTS) ||
-                          unlikely(utrace->death)) {
-                       /*
-                        * We have already started the death report, or
-                        * are about to very soon.  We can't prevent
-                        * the report_death and report_reap callbacks,
-                        * so tell the caller they will happen.
-                        */
-                       spin_unlock(&utrace->lock);
-                       return -EALREADY;
                }
        }