* [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master())
@ 2013-11-13 14:35 Oleg Nesterov
2013-11-13 14:36 ` [PATCH 1/3] proc: cleanup/simplify get_task_state/task_state_array Oleg Nesterov
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Oleg Nesterov @ 2013-11-13 14:35 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: David Laight, Geert Uytterhoeven, Ingo Molnar, Tejun Heo, linux-kernel
On 11/12, Oleg Nesterov wrote:
>
> On 11/12, Peter Zijlstra wrote:
> >
> > We have to put in something...
> >
> > BUILD_BUG_ON(1 + ilog2(TASK_STATE_MAX) != ARRAY_SIZE(task_state_array));
> >
> > However, since we always set it together with TASK_UNINTERUPTIBLE
> > userspace shouldn't actually ever see the I thing.
>
> OOPS. I didn't know that get_task_state() does &= TASK_REPORT. So it
> can never report anything > EXIT_DEAD.
>
> Perhaps we should change BUILD_BUG_ON() and shrink task_state_array?
Seriously, imho this looks confusing enough and deserves a cleanup.
As for "nobody should use exit_state". I'll try to recheck, but iirc
we already discussed this... do you remember any reason why
schedule_debug() can't check prev->state == TASK_DEAD instead of
->exit_state?
Note that ->exit_state is not exactly right, it is set by exit_notify()
but in_atomic_preempt_off() should be only ignored when the task does
the last schedule() in TASK_DEAD.
Oleg.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] proc: cleanup/simplify get_task_state/task_state_array
2013-11-13 14:35 [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master()) Oleg Nesterov
@ 2013-11-13 14:36 ` Oleg Nesterov
2013-11-13 14:36 ` [PATCH 2/3] fork: no need to initialize child->exit_state Oleg Nesterov
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2013-11-13 14:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: David Laight, Geert Uytterhoeven, Ingo Molnar, Tejun Heo, linux-kernel
get_task_state() and task_state_array[] look confusing and
suboptimal, it is not clear what it can actually report to
user-space and task_state_array[] blows .data for no reason.
1. state = (tsk->state & TASK_REPORT) | tsk->exit_state is not
clear. TASK_REPORT is self-documenting but it is not clear
what ->exit_state can add.
Move the potential exit_state's (EXIT_ZOMBIE and EXIT_DEAD)
into TASK_REPORT and use it to calculate the final result.
2. With the change above it is obvious that task_state_array[]
has the unused entries just to make BUILD_BUG_ON() happy.
Change this BUILD_BUG_ON() to use TASK_REPORT rather than
TASK_STATE_MAX and shrink task_state_array[].
3. Turn the "while (state)" loop into fls(state).
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
fs/proc/array.c | 15 +++------------
include/linux/sched.h | 2 +-
2 files changed, 4 insertions(+), 13 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c
index cbd0f1b..c995807 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -140,24 +140,15 @@ static const char * const task_state_array[] = {
"t (tracing stop)", /* 8 */
"Z (zombie)", /* 16 */
"X (dead)", /* 32 */
- "x (dead)", /* 64 */
- "K (wakekill)", /* 128 */
- "W (waking)", /* 256 */
- "P (parked)", /* 512 */
};
static inline const char *get_task_state(struct task_struct *tsk)
{
- unsigned int state = (tsk->state & TASK_REPORT) | tsk->exit_state;
- const char * const *p = &task_state_array[0];
+ unsigned int state = (tsk->state | tsk->exit_state) & TASK_REPORT;
- BUILD_BUG_ON(1 + ilog2(TASK_STATE_MAX) != ARRAY_SIZE(task_state_array));
+ BUILD_BUG_ON(1 + ilog2(TASK_REPORT) != ARRAY_SIZE(task_state_array)-1);
- while (state) {
- p++;
- state >>= 1;
- }
- return *p;
+ return task_state_array[fls(state)];
}
static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
diff --git a/include/linux/sched.h b/include/linux/sched.h
index e27baee..ee2c2e6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -163,7 +163,7 @@ extern char ___assert_task_state[1 - 2*!!(
/* get_task_state() */
#define TASK_REPORT (TASK_RUNNING | TASK_INTERRUPTIBLE | \
TASK_UNINTERRUPTIBLE | __TASK_STOPPED | \
- __TASK_TRACED)
+ __TASK_TRACED | EXIT_ZOMBIE | EXIT_DEAD)
#define task_is_traced(task) ((task->state & __TASK_TRACED) != 0)
#define task_is_stopped(task) ((task->state & __TASK_STOPPED) != 0)
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] fork: no need to initialize child->exit_state
2013-11-13 14:35 [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master()) Oleg Nesterov
2013-11-13 14:36 ` [PATCH 1/3] proc: cleanup/simplify get_task_state/task_state_array Oleg Nesterov
@ 2013-11-13 14:36 ` Oleg Nesterov
2013-11-27 14:10 ` [tip:sched/core] tasks/fork: Remove unnecessary child->exit_state tip-bot for Oleg Nesterov
2013-11-13 14:36 ` [PATCH 3/3] exit_state: kill task_is_dead() Oleg Nesterov
2013-11-13 14:48 ` [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master()) Peter Zijlstra
3 siblings, 1 reply; 12+ messages in thread
From: Oleg Nesterov @ 2013-11-13 14:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: David Laight, Geert Uytterhoeven, Ingo Molnar, Tejun Heo, linux-kernel
A zombie task obviously can't fork(), remove the unnecessary
initialization of child->exit_state. It is zero anyway after
dup_task_struct().
Note: copy_process() is huge and it has a lot of chaotic
initializations, probably it makes sense to move them into the
new helper called by dup_task_struct().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/fork.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index 8531609..2cb6024 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1405,13 +1405,11 @@ static struct task_struct *copy_process(unsigned long clone_flags,
p->tgid = p->pid;
}
- p->pdeath_signal = 0;
- p->exit_state = 0;
-
p->nr_dirtied = 0;
p->nr_dirtied_pause = 128 >> (PAGE_SHIFT - 10);
p->dirty_paused_when = 0;
+ p->pdeath_signal = 0;
INIT_LIST_HEAD(&p->thread_group);
p->task_works = NULL;
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] exit_state: kill task_is_dead()
2013-11-13 14:35 [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master()) Oleg Nesterov
2013-11-13 14:36 ` [PATCH 1/3] proc: cleanup/simplify get_task_state/task_state_array Oleg Nesterov
2013-11-13 14:36 ` [PATCH 2/3] fork: no need to initialize child->exit_state Oleg Nesterov
@ 2013-11-13 14:36 ` Oleg Nesterov
2013-11-27 14:10 ` [tip:sched/core] tasks/exit: Remove unused task_is_dead() method tip-bot for Oleg Nesterov
2013-11-13 14:48 ` [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master()) Peter Zijlstra
3 siblings, 1 reply; 12+ messages in thread
From: Oleg Nesterov @ 2013-11-13 14:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: David Laight, Geert Uytterhoeven, Ingo Molnar, Tejun Heo, linux-kernel
task_is_dead() has no users since commit 43e13cc107cf
"cred: remove task_is_dead() from __task_cred() validation", and
nobody except exit.c should rely on ->exit_state (we still have
the users which should be changed).
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
include/linux/sched.h | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index ee2c2e6..33cf854 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -167,7 +167,6 @@ extern char ___assert_task_state[1 - 2*!!(
#define task_is_traced(task) ((task->state & __TASK_TRACED) != 0)
#define task_is_stopped(task) ((task->state & __TASK_STOPPED) != 0)
-#define task_is_dead(task) ((task)->exit_state != 0)
#define task_is_stopped_or_traced(task) \
((task->state & (__TASK_STOPPED | __TASK_TRACED)) != 0)
#define task_contributes_to_load(task) \
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master())
2013-11-13 14:35 [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master()) Oleg Nesterov
` (2 preceding siblings ...)
2013-11-13 14:36 ` [PATCH 3/3] exit_state: kill task_is_dead() Oleg Nesterov
@ 2013-11-13 14:48 ` Peter Zijlstra
2013-11-13 14:50 ` Peter Zijlstra
2013-11-13 15:45 ` [PATCH 0/1] sched: Check TASK_DEAD rather than EXIT_DEAD in schedule_debug() Oleg Nesterov
3 siblings, 2 replies; 12+ messages in thread
From: Peter Zijlstra @ 2013-11-13 14:48 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, David Laight, Geert Uytterhoeven, Ingo Molnar,
Tejun Heo, linux-kernel
On Wed, Nov 13, 2013 at 03:35:52PM +0100, Oleg Nesterov wrote:
> On 11/12, Oleg Nesterov wrote:
> >
> > On 11/12, Peter Zijlstra wrote:
> > >
> > > We have to put in something...
> > >
> > > BUILD_BUG_ON(1 + ilog2(TASK_STATE_MAX) != ARRAY_SIZE(task_state_array));
> > >
> > > However, since we always set it together with TASK_UNINTERUPTIBLE
> > > userspace shouldn't actually ever see the I thing.
> >
> > OOPS. I didn't know that get_task_state() does &= TASK_REPORT. So it
> > can never report anything > EXIT_DEAD.
> >
> > Perhaps we should change BUILD_BUG_ON() and shrink task_state_array?
>
> Seriously, imho this looks confusing enough and deserves a cleanup.
>
>
> As for "nobody should use exit_state". I'll try to recheck, but iirc
> we already discussed this... do you remember any reason why
> schedule_debug() can't check prev->state == TASK_DEAD instead of
> ->exit_state?
I have no such memories :/ but a quick test shows that such a kernel
does boot without issue.
> Note that ->exit_state is not exactly right, it is set by exit_notify()
> but in_atomic_preempt_off() should be only ignored when the task does
> the last schedule() in TASK_DEAD.
Agreed.
For these patches:
Acked-by: Peter Zijlstra <peterz@infradead.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master())
2013-11-13 14:48 ` [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master()) Peter Zijlstra
@ 2013-11-13 14:50 ` Peter Zijlstra
2013-11-13 15:45 ` [PATCH 0/1] sched: Check TASK_DEAD rather than EXIT_DEAD in schedule_debug() Oleg Nesterov
1 sibling, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2013-11-13 14:50 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, David Laight, Geert Uytterhoeven, Ingo Molnar,
Tejun Heo, linux-kernel
On Wed, Nov 13, 2013 at 03:48:56PM +0100, Peter Zijlstra wrote:
> For these patches:
>
> Acked-by: Peter Zijlstra <peterz@infradead.org>
On that, I'll just pick them up and stuff them through the scheduler
tree, its mostly sched related anyway.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 0/1] sched: Check TASK_DEAD rather than EXIT_DEAD in schedule_debug()
2013-11-13 14:48 ` [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master()) Peter Zijlstra
2013-11-13 14:50 ` Peter Zijlstra
@ 2013-11-13 15:45 ` Oleg Nesterov
2013-11-13 15:45 ` [PATCH 1/1] " Oleg Nesterov
2013-11-13 15:58 ` [PATCH 0/1] " Peter Zijlstra
1 sibling, 2 replies; 12+ messages in thread
From: Oleg Nesterov @ 2013-11-13 15:45 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Andrew Morton, David Laight, Geert Uytterhoeven, Ingo Molnar,
Tejun Heo, linux-kernel
On 11/13, Peter Zijlstra wrote:
>
> On Wed, Nov 13, 2013 at 03:35:52PM +0100, Oleg Nesterov wrote:
> >
> > As for "nobody should use exit_state". I'll try to recheck, but iirc
> > we already discussed this... do you remember any reason why
> > schedule_debug() can't check prev->state == TASK_DEAD instead of
> > ->exit_state?
>
> I have no such memories :/ but a quick test shows that such a kernel
> does boot without issue.
probably my memory fools me. OK, it seems that you agree with this change,
hopefully we both can't be wrong ;)
> Acked-by: Peter Zijlstra <peterz@infradead.org>
Thanks! And since picked this series, perhaps you can also queue this
one?
Oleg.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/1] sched: Check TASK_DEAD rather than EXIT_DEAD in schedule_debug()
2013-11-13 15:45 ` [PATCH 0/1] sched: Check TASK_DEAD rather than EXIT_DEAD in schedule_debug() Oleg Nesterov
@ 2013-11-13 15:45 ` Oleg Nesterov
2013-11-27 14:10 ` [tip:sched/core] " tip-bot for Oleg Nesterov
2013-11-13 15:58 ` [PATCH 0/1] " Peter Zijlstra
1 sibling, 1 reply; 12+ messages in thread
From: Oleg Nesterov @ 2013-11-13 15:45 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Andrew Morton, David Laight, Geert Uytterhoeven, Ingo Molnar,
Tejun Heo, linux-kernel
schedule_debug() ignores in_atomic() if prev->exit_state != 0.
This is not what we want, ->exit_state is set by exit_notify()
but we should complain until the task does the last schedule()
in TASK_DEAD.
See also 7407251a0e2e "PF_DEAD cleanup", I think this ancient
commit explains why schedule() had to rely on ->exit_state,
until that commit exit_notify() disabled preemption and set
PF_DEAD which was used to detect the exiting task.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/sched/core.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 5ac63c9..7184357 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2287,10 +2287,10 @@ static inline void schedule_debug(struct task_struct *prev)
{
/*
* Test if we are atomic. Since do_exit() needs to call into
- * schedule() atomically, we ignore that path for now.
- * Otherwise, whine if we are scheduling when we should not be.
+ * schedule() atomically, we ignore that path. Otherwise whine
+ * if we are scheduling when we should not.
*/
- if (unlikely(in_atomic_preempt_off() && !prev->exit_state))
+ if (unlikely(in_atomic_preempt_off() && prev->state != TASK_DEAD))
__schedule_bug(prev);
rcu_sleep_check();
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/1] sched: Check TASK_DEAD rather than EXIT_DEAD in schedule_debug()
2013-11-13 15:45 ` [PATCH 0/1] sched: Check TASK_DEAD rather than EXIT_DEAD in schedule_debug() Oleg Nesterov
2013-11-13 15:45 ` [PATCH 1/1] " Oleg Nesterov
@ 2013-11-13 15:58 ` Peter Zijlstra
1 sibling, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2013-11-13 15:58 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, David Laight, Geert Uytterhoeven, Ingo Molnar,
Tejun Heo, linux-kernel
On Wed, Nov 13, 2013 at 04:45:16PM +0100, Oleg Nesterov wrote:
> Thanks! And since picked this series, perhaps you can also queue this
> one?
Done; if there's something fishy we're bound to hit it before .14 (I
hope).
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tip:sched/core] tasks/fork: Remove unnecessary child->exit_state
2013-11-13 14:36 ` [PATCH 2/3] fork: no need to initialize child->exit_state Oleg Nesterov
@ 2013-11-27 14:10 ` tip-bot for Oleg Nesterov
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Oleg Nesterov @ 2013-11-27 14:10 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, torvalds, peterz, David.Laight, geert,
akpm, tj, tglx, oleg
Commit-ID: bb8cbbfee68518796df4050868e5b0f5ad078f9f
Gitweb: http://git.kernel.org/tip/bb8cbbfee68518796df4050868e5b0f5ad078f9f
Author: Oleg Nesterov <oleg@redhat.com>
AuthorDate: Wed, 13 Nov 2013 15:36:12 +0100
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 27 Nov 2013 13:50:50 +0100
tasks/fork: Remove unnecessary child->exit_state
A zombie task obviously can't fork(), remove the unnecessary
initialization of child->exit_state. It is zero anyway after
dup_task_struct().
Note: copy_process() is huge and it has a lot of chaotic
initializations, probably it makes sense to move them into the
new helper called by dup_task_struct().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: David Laight <David.Laight@ACULAB.COM>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20131113143612.GA10540@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/fork.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index 728d5be..b308082 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1402,13 +1402,11 @@ static struct task_struct *copy_process(unsigned long clone_flags,
p->tgid = p->pid;
}
- p->pdeath_signal = 0;
- p->exit_state = 0;
-
p->nr_dirtied = 0;
p->nr_dirtied_pause = 128 >> (PAGE_SHIFT - 10);
p->dirty_paused_when = 0;
+ p->pdeath_signal = 0;
INIT_LIST_HEAD(&p->thread_group);
p->task_works = NULL;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [tip:sched/core] tasks/exit: Remove unused task_is_dead() method
2013-11-13 14:36 ` [PATCH 3/3] exit_state: kill task_is_dead() Oleg Nesterov
@ 2013-11-27 14:10 ` tip-bot for Oleg Nesterov
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Oleg Nesterov @ 2013-11-27 14:10 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, torvalds, peterz, David.Laight, geert,
akpm, tj, tglx, oleg
Commit-ID: 86506a99a62400e9f7b7d1344bcc9ea235faf98f
Gitweb: http://git.kernel.org/tip/86506a99a62400e9f7b7d1344bcc9ea235faf98f
Author: Oleg Nesterov <oleg@redhat.com>
AuthorDate: Wed, 13 Nov 2013 15:36:14 +0100
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 27 Nov 2013 13:50:52 +0100
tasks/exit: Remove unused task_is_dead() method
task_is_dead() has no users since commit 43e13cc107cf
("cred: remove task_is_dead() from __task_cred() validation"), and
nobody except exit.c should rely on ->exit_state (we still have
the users which should be changed).
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Cc: David Laight <David.Laight@ACULAB.COM>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20131113143614.GA10547@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/sched.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 7e35d4b..cda8d63 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -168,7 +168,6 @@ extern char ___assert_task_state[1 - 2*!!(
#define task_is_traced(task) ((task->state & __TASK_TRACED) != 0)
#define task_is_stopped(task) ((task->state & __TASK_STOPPED) != 0)
-#define task_is_dead(task) ((task)->exit_state != 0)
#define task_is_stopped_or_traced(task) \
((task->state & (__TASK_STOPPED | __TASK_TRACED)) != 0)
#define task_contributes_to_load(task) \
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [tip:sched/core] sched: Check TASK_DEAD rather than EXIT_DEAD in schedule_debug()
2013-11-13 15:45 ` [PATCH 1/1] " Oleg Nesterov
@ 2013-11-27 14:10 ` tip-bot for Oleg Nesterov
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Oleg Nesterov @ 2013-11-27 14:10 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, torvalds, peterz, David.Laight, geert,
akpm, tj, tglx, oleg
Commit-ID: 192301e70af3f6803c6354a464ebfa742da738ae
Gitweb: http://git.kernel.org/tip/192301e70af3f6803c6354a464ebfa742da738ae
Author: Oleg Nesterov <oleg@redhat.com>
AuthorDate: Wed, 13 Nov 2013 16:45:38 +0100
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 27 Nov 2013 13:50:53 +0100
sched: Check TASK_DEAD rather than EXIT_DEAD in schedule_debug()
schedule_debug() ignores in_atomic() if prev->exit_state != 0.
This is not what we want, ->exit_state is set by exit_notify()
but we should complain until the task does the last schedule()
in TASK_DEAD.
See also 7407251a0e2e "PF_DEAD cleanup", I think this ancient
commit explains why schedule() had to rely on ->exit_state,
until that commit exit_notify() disabled preemption and set
PF_DEAD which was used to detect the exiting task.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Cc: David Laight <David.Laight@ACULAB.COM>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20131113154538.GB15810@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 687985b..19db8f3 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2414,10 +2414,10 @@ static inline void schedule_debug(struct task_struct *prev)
{
/*
* Test if we are atomic. Since do_exit() needs to call into
- * schedule() atomically, we ignore that path for now.
- * Otherwise, whine if we are scheduling when we should not be.
+ * schedule() atomically, we ignore that path. Otherwise whine
+ * if we are scheduling when we should not.
*/
- if (unlikely(in_atomic_preempt_off() && !prev->exit_state))
+ if (unlikely(in_atomic_preempt_off() && prev->state != TASK_DEAD))
__schedule_bug(prev);
rcu_sleep_check();
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-11-27 14:11 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-13 14:35 [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master()) Oleg Nesterov
2013-11-13 14:36 ` [PATCH 1/3] proc: cleanup/simplify get_task_state/task_state_array Oleg Nesterov
2013-11-13 14:36 ` [PATCH 2/3] fork: no need to initialize child->exit_state Oleg Nesterov
2013-11-27 14:10 ` [tip:sched/core] tasks/fork: Remove unnecessary child->exit_state tip-bot for Oleg Nesterov
2013-11-13 14:36 ` [PATCH 3/3] exit_state: kill task_is_dead() Oleg Nesterov
2013-11-27 14:10 ` [tip:sched/core] tasks/exit: Remove unused task_is_dead() method tip-bot for Oleg Nesterov
2013-11-13 14:48 ` [PATCH 0/3] state/exit_state cleanups (Was: Remove unused variable ret from sync_thread_master()) Peter Zijlstra
2013-11-13 14:50 ` Peter Zijlstra
2013-11-13 15:45 ` [PATCH 0/1] sched: Check TASK_DEAD rather than EXIT_DEAD in schedule_debug() Oleg Nesterov
2013-11-13 15:45 ` [PATCH 1/1] " Oleg Nesterov
2013-11-27 14:10 ` [tip:sched/core] " tip-bot for Oleg Nesterov
2013-11-13 15:58 ` [PATCH 0/1] " Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).