From: Roland McGrath <roland@redhat.com>
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>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
25-akpm/kernel/exit.c | 8 ++++----
25-akpm/kernel/sched.c | 2 +-
25-akpm/kernel/signal.c | 6 +++---
25-akpm/kernel/timer.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
diff -puN kernel/exit.c~task_structexit_state-usage kernel/exit.c
--- 25/kernel/exit.c~task_structexit_state-usage 2004-12-27 00:42:47.633032600 -0800
+++ 25-akpm/kernel/exit.c 2004-12-27 00:42:47.647030472 -0800
@@ -159,7 +159,7 @@ static int will_become_orphaned_pgrp(int
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(tas
* 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_paren
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,
* 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 -puN kernel/sched.c~task_structexit_state-usage kernel/sched.c
--- 25/kernel/sched.c~task_structexit_state-usage 2004-12-27 00:42:47.636032144 -0800
+++ 25-akpm/kernel/sched.c 2004-12-27 00:42:47.652029712 -0800
@@ -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 -puN kernel/signal.c~task_structexit_state-usage kernel/signal.c
--- 25/kernel/signal.c~task_structexit_state-usage 2004-12-27 00:42:47.639031688 -0800
+++ 25-akpm/kernel/signal.c 2004-12-27 00:42:47.659028648 -0800
@@ -934,10 +934,10 @@ __group_complete_signal(int sig, struct
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_struc
/*
* Don't bother with already dead threads
*/
- if (t->exit_state & (EXIT_ZOMBIE|EXIT_DEAD))
+ if (t->exit_state)
continue;
/*
diff -puN kernel/timer.c~task_structexit_state-usage kernel/timer.c
--- 25/kernel/timer.c~task_structexit_state-usage 2004-12-27 00:42:47.642031232 -0800
+++ 25-akpm/kernel/timer.c 2004-12-27 00:42:47.661028344 -0800
@@ -806,7 +806,7 @@ static inline void do_process_times(stru
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))
_