(I am trying to cleanup this code and make it more understandable, but
of course this is subjective).
utrace_do_stop() considers the dead task without _UTRACE_DEATH_EVENTS
as quiescent. This is correct, but
- this is only needed for UTRACE_DETACH case
- this _looks_ racy, ->exit_state is not protected
- it is not immediately obvious this check also relies on
utrace_control_dead() which was already called by our
caller.
- utrace_control_dead() if fact has already verified it is
safe to detach. Even if _UTRACE_DEATH_EVENTS are set, we
know that utrace_report_death/utrace_release_task can not
be called until we drop utrace->lock (or utrace_report_death
was already called). In any case it is safe to detach, we
can't race with our quiesce/death/reap callbacks.
Also, I tried to document "if (exit_state)" check in utrace_control.
Signed-off-by: Oleg Nesterov <o...@redhat.com>
--- __UTRACE/kernel/utrace.c~7_DO_STOP_NO_EXIT_STATE 2009-08-26
18:03:36.000000000 +0200
+++ __UTRACE/kernel/utrace.c 2009-08-26 19:22:02.000000000 +0200
@@ -660,15 +660,7 @@ static bool utrace_do_stop(struct task_s
{
bool stopped = false;
- if (unlikely(target->exit_state)) {
- /*
- * On the exit path, it's only truly quiescent
- * if it has already been through
- * utrace_report_death(), or never will.
- */
- if (!(target->utrace_flags & _UTRACE_DEATH_EVENTS))
- stopped = true;
- } else if (task_is_stopped(target)) {
+ if (task_is_stopped(target)) {
/*
* Stopped is considered quiescent; when it wakes up, it will
* go through utrace_finish_jctl() before doing anything else.
@@ -1073,17 +1065,24 @@ int utrace_control(struct task_struct *t
if (unlikely(IS_ERR(utrace)))
return PTR_ERR(utrace);
- if (target->exit_state) {
+ resume = utrace->stopped;
+ ret = 0;
+
+ /*
+ * ->exit_state can change under us, this doesn't matter.
+ * We do not care about ->exit_state in fact, but we do
+ * care about ->reap and ->death. If either flag is set,
+ * we must also see ->exit_state != 0.
+ */
+ if (unlikely(target->exit_state)) {
ret = utrace_control_dead(target, utrace, action);
if (ret) {
spin_unlock(&utrace->lock);
return ret;
}
+ resume = true;
}
- resume = utrace->stopped;
- ret = 0;
-
clear_engine_wants_stop(engine);
switch (action) {
case UTRACE_STOP:
