[PATCH] task_struct.exit_state usage

I just did a quick audit of the use of exit_state and the EXIT_* bit
macros.  I guess I didn't really review these changes very closely when you
did them originally.  :-(

I found several places that seem like lossy cases of query-replace without
enough thought about the code.  Linus has previously said the >= tests
ought to be & tests instead.  But for exit_state, it can only ever be 0,
EXIT_DEAD, or EXIT_ZOMBIE--so a nonzero test is actually the same as
testing & (EXIT_DEAD|EXIT_ZOMBIE), and maybe its code is a tiny bit better.
 The case like in choose_new_parent is just confusing, to have the
always-false test for EXIT_* bits in ->state there too.

The two cases in wants_signal and do_process_times are actual regressions
that will give us back old bugs in race conditions.  These places had
s/TASK/EXIT/ but not s/state/exit_state/, and now there tests for exiting
tasks are now wrong and never catching them.  I take it back: there is no
regression in wants_signal in practice I think, because of the PF_EXITING
test that makes the EXIT_* state checks superfluous anyway.  So that is
just another cosmetic case of confusing code.  But in do_process_times,
there is that SIGXCPU-while-exiting race condition back again.

Signed-off-by: Roland McGrath <roland@redhat.com>
Acked-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

4 files changed, 9 insertions, 9 deletions

diff --git a/kernel/exit.c b/kernel/exit.c
index d7b09c996f6d58..f6b64f9e3dcc4c 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -159,7 +159,7 @@ static int will_become_orphaned_pgrp(int pgrp, task_t *ignored_task)
 
 	do_each_task_pid(pgrp, PIDTYPE_PGID, p) {
 		if (p == ignored_task
-				|| p->exit_state >= EXIT_ZOMBIE
+				|| p->exit_state
 				|| p->real_parent->pid == 1)
 			continue;
 		if (process_group(p->real_parent) != pgrp
@@ -517,7 +517,7 @@ static inline void choose_new_parent(task_t *p, task_t *reaper, task_t *child_re
 	 * Make sure we're not reparenting to ourselves and that
 	 * the parent is not a zombie.
 	 */
-	BUG_ON(p == reaper || reaper->state >= EXIT_ZOMBIE || reaper->exit_state >= EXIT_ZOMBIE);
+	BUG_ON(p == reaper || reaper->exit_state >= EXIT_ZOMBIE);
 	p->real_parent = reaper;
 	if (p->parent == p->real_parent)
 		BUG();
@@ -599,7 +599,7 @@ static inline void forget_original_parent(struct task_struct * father,
 			reaper = child_reaper;
 			break;
 		}
-	} while (reaper->exit_state >= EXIT_ZOMBIE);
+	} while (reaper->exit_state);
 
 	/*
 	 * There are only two places where our children can be:
@@ -1182,7 +1182,7 @@ static int wait_task_stopped(task_t *p, int delayed_group_leader, int noreap,
 	 * race with the EXIT_ZOMBIE case.
 	 */
 	exit_code = xchg(&p->exit_code, 0);
-	if (unlikely(p->exit_state >= EXIT_ZOMBIE)) {
+	if (unlikely(p->exit_state)) {
 		/*
 		 * The task resumed and then died.  Let the next iteration
 		 * catch it in EXIT_ZOMBIE.  Note that exit_code might
diff --git a/kernel/sched.c b/kernel/sched.c
index 3386e7b876143f..9f1e93a2bf0efb 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -2541,7 +2541,7 @@ asmlinkage void __sched schedule(void)
 	 * schedule() atomically, we ignore that path for now.
 	 * Otherwise, whine if we are scheduling when we should not be.
 	 */
-	if (likely(!(current->exit_state & (EXIT_DEAD | EXIT_ZOMBIE)))) {
+	if (likely(!current->exit_state)) {
 		if (unlikely(in_atomic())) {
 			printk(KERN_ERR "scheduling while atomic: "
 				"%s/0x%08x/%d\n",
diff --git a/kernel/signal.c b/kernel/signal.c
index 7191de92659bf5..85ad9a0ae3f155 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -934,10 +934,10 @@ __group_complete_signal(int sig, struct task_struct *p)
 	struct task_struct *t;
 
 	/*
-	 * Don't bother zombies and stopped tasks (but
+	 * Don't bother traced and stopped tasks (but
 	 * SIGKILL will punch through stopped state)
 	 */
-	mask = EXIT_DEAD | EXIT_ZOMBIE | TASK_TRACED;
+	mask = TASK_TRACED;
 	if (sig != SIGKILL)
 		mask |= TASK_STOPPED;
 
@@ -1094,7 +1094,7 @@ void zap_other_threads(struct task_struct *p)
 		/*
 		 * Don't bother with already dead threads
 		 */
-		if (t->exit_state & (EXIT_ZOMBIE|EXIT_DEAD))
+		if (t->exit_state)
 			continue;
 
 		/*
diff --git a/kernel/timer.c b/kernel/timer.c
index d0eed9b563c48b..d0177a7f573a18 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -806,7 +806,7 @@ static inline void do_process_times(struct task_struct *p,
 
 	psecs = (p->utime += user);
 	psecs += (p->stime += system);
-	if (p->signal && !unlikely(p->state & (EXIT_DEAD|EXIT_ZOMBIE)) &&
+	if (p->signal && !unlikely(p->exit_state) &&
 	    psecs / HZ >= p->signal->rlim[RLIMIT_CPU].rlim_cur) {
 		/* Send SIGXCPU every second.. */
 		if (!(psecs % HZ))