After the previous change (->flags & PF_DEAD) <=> (->state == EXIT_DEAD),
so we can forget about PF_DEAD flag.
In my opinion PF_DEAD has nothing to do with process state, it is just
indication that task_struct should be freed after the last schedule.
Perhaps it makes sense to add new TASK_DEAD flag to use it instead of
EXIT_DEAD, while the latter should live only in ->exit_state.
Signed-off-by: Oleg Nesterov <[email protected]>
--- 2.6.15-rc2/include/linux/sched.h~5_KILL 2005-11-22 19:35:52.000000000 +0300
+++ 2.6.15-rc2/include/linux/sched.h 2005-11-24 20:06:19.000000000 +0300
@@ -890,7 +890,6 @@ do { if (atomic_dec_and_test(&(tsk)->usa
/* Not implemented yet, only for 486*/
#define PF_STARTING 0x00000002 /* being created */
#define PF_EXITING 0x00000004 /* getting shut down */
-#define PF_DEAD 0x00000008 /* Dead */
#define PF_FORKNOEXEC 0x00000040 /* forked but didn't exec */
#define PF_SUPERPRIV 0x00000100 /* used super-user privileges */
#define PF_DUMPCORE 0x00000200 /* dumped core */
--- 2.6.15-rc2/kernel/exit.c~5_KILL 2005-11-24 19:14:29.000000000 +0300
+++ 2.6.15-rc2/kernel/exit.c 2005-11-24 20:12:34.000000000 +0300
@@ -871,10 +871,8 @@ fastcall NORET_TYPE void do_exit(long co
tsk->mempolicy = NULL;
#endif
- /* PF_DEAD causes final put_task_struct after we schedule. */
+ /* ->state == EXIT_DEAD causes final put_task_struct after we schedule. */
preempt_disable();
- BUG_ON(tsk->flags & PF_DEAD);
- tsk->flags |= PF_DEAD;
tsk->state = EXIT_DEAD;
schedule();
--- 2.6.15-rc2/kernel/sched.c~5_KILL 2005-11-24 19:13:34.000000000 +0300
+++ 2.6.15-rc2/kernel/sched.c 2005-11-24 21:12:46.000000000 +0300
@@ -1615,7 +1615,7 @@ static inline void finish_task_switch(ru
__releases(rq->lock)
{
struct mm_struct *mm = rq->prev_mm;
- unsigned long prev_task_flags;
+ int task_dead;
rq->prev_mm = NULL;
@@ -1630,12 +1630,12 @@ static inline void finish_task_switch(ru
* be dropped twice.
* Manfred Spraul <[email protected]>
*/
- prev_task_flags = prev->flags;
+ task_dead = (prev->state == EXIT_DEAD);
finish_arch_switch(prev);
finish_lock_switch(rq, prev);
if (mm)
mmdrop(mm);
- if (unlikely(prev_task_flags & PF_DEAD))
+ if (unlikely(task_dead))
put_task_struct(prev);
}
@@ -4711,7 +4711,7 @@ static void migrate_dead(unsigned int de
BUG_ON(tsk->exit_state != EXIT_ZOMBIE && tsk->exit_state != EXIT_DEAD);
/* Cannot have done final schedule yet: would have vanished. */
- BUG_ON(tsk->flags & PF_DEAD);
+ BUG_ON(tsk->state == EXIT_DEAD);
get_task_struct(tsk);
--- 2.6.15-rc2/mm/oom_kill.c~5_KILL 2005-10-11 16:22:42.000000000 +0400
+++ 2.6.15-rc2/mm/oom_kill.c 2005-11-24 20:16:34.000000000 +0300
@@ -163,7 +163,7 @@ static struct task_struct * select_bad_p
*/
releasing = test_tsk_thread_flag(p, TIF_MEMDIE) ||
p->flags & PF_EXITING;
- if (releasing && !(p->flags & PF_DEAD))
+ if (releasing && (p->state != EXIT_DEAD))
return ERR_PTR(-1UL);
if (p->flags & PF_SWAPOFF)
return p;
* Oleg Nesterov <[email protected]> wrote:
> After the previous change (->flags & PF_DEAD) <=> (->state ==
> EXIT_DEAD), so we can forget about PF_DEAD flag.
>
> In my opinion PF_DEAD has nothing to do with process state, it is just
> indication that task_struct should be freed after the last schedule.
>
> Perhaps it makes sense to add new TASK_DEAD flag to use it instead of
> EXIT_DEAD, while the latter should live only in ->exit_state.
yes, i'd suggest a followup patch to keep the two flag spaces totally
disjunct - at least for testing in -mm. This area of code (when it was
introduced) was pretty fragile and blew up under PREEMPT+SMP a couple of
times. The separate handling of PF_DEAD was necessary until i fixed
do_exit() to only have PF_DEAD in an atomic path up to the final
schedule(). You are right that we can totally eliminate it now.
so your patch looks great to me! (I have also added this patch (and the
previous patch) to the -rt tree, to give it some more testing in more
extreme preemption scenarios.)
Acked-by: Ingo Molnar <[email protected]>
Ingo
Ingo Molnar wrote:
>
> > Perhaps it makes sense to add new TASK_DEAD flag to use it instead of
> > EXIT_DEAD, while the latter should live only in ->exit_state.
>
> yes, i'd suggest a followup patch to keep the two flag spaces totally
> disjunct - at least for testing in -mm. This area of code (when it was
Here it is.
I am also resending these 2 cleanups with EXIT_STATE changed to TASK_STATE.
Andrew, please take these new ones unless you or Ingo have any objections.
Oleg.
--- 2.6.15-rc2/include/linux/sched.h~3_RENAME 2005-11-25 21:54:07.000000000 +0300
+++ 2.6.15-rc2/include/linux/sched.h 2005-11-25 21:56:36.000000000 +0300
@@ -127,6 +127,7 @@ extern unsigned long nr_iowait(void);
#define EXIT_DEAD 32
/* in tsk->state again */
#define TASK_NONINTERACTIVE 64
+#define TASK_DEAD 128
#define __set_task_state(tsk, state_value) \
do { (tsk)->state = (state_value); } while (0)
--- 2.6.15-rc2/kernel/exit.c~3_RENAME 2005-11-25 21:54:07.000000000 +0300
+++ 2.6.15-rc2/kernel/exit.c 2005-11-25 21:56:36.000000000 +0300
@@ -871,9 +871,9 @@ fastcall NORET_TYPE void do_exit(long co
tsk->mempolicy = NULL;
#endif
- /* ->state == EXIT_DEAD causes final put_task_struct after we schedule. */
+ /* ->state == TASK_DEAD causes final put_task_struct after we schedule. */
preempt_disable();
- tsk->state = EXIT_DEAD;
+ tsk->state = TASK_DEAD;
schedule();
BUG();
--- 2.6.15-rc2/kernel/sched.c~3_RENAME 2005-11-25 21:54:07.000000000 +0300
+++ 2.6.15-rc2/kernel/sched.c 2005-11-25 21:56:36.000000000 +0300
@@ -1630,7 +1630,7 @@ static inline void finish_task_switch(ru
* be dropped twice.
* Manfred Spraul <[email protected]>
*/
- task_dead = (prev->state == EXIT_DEAD);
+ task_dead = (prev->state == TASK_DEAD);
finish_arch_switch(prev);
finish_lock_switch(rq, prev);
if (mm)
@@ -4711,7 +4711,7 @@ static void migrate_dead(unsigned int de
BUG_ON(tsk->exit_state != EXIT_ZOMBIE && tsk->exit_state != EXIT_DEAD);
/* Cannot have done final schedule yet: would have vanished. */
- BUG_ON(tsk->state == EXIT_DEAD);
+ BUG_ON(tsk->state == TASK_DEAD);
get_task_struct(tsk);
--- 2.6.15-rc2/mm/oom_kill.c~3_RENAME 2005-11-25 21:54:07.000000000 +0300
+++ 2.6.15-rc2/mm/oom_kill.c 2005-11-25 21:56:36.000000000 +0300
@@ -163,7 +163,7 @@ static struct task_struct * select_bad_p
*/
releasing = test_tsk_thread_flag(p, TIF_MEMDIE) ||
p->flags & PF_EXITING;
- if (releasing && (p->state != EXIT_DEAD))
+ if (releasing && (p->state != TASK_DEAD))
return ERR_PTR(-1UL);
if (p->flags & PF_SWAPOFF)
return p;