With the new locking TASK_WAKING has become obsolete, remove it.
Suggested-by: Oleg Nesterov <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
LKML-Reference: <new-submission>
---
fs/proc/array.c | 1 -
include/linux/sched.h | 5 ++---
kernel/sched.c | 4 ++--
3 files changed, 4 insertions(+), 6 deletions(-)
Index: linux-2.6/fs/proc/array.c
===================================================================
--- linux-2.6.orig/fs/proc/array.c
+++ linux-2.6/fs/proc/array.c
@@ -141,7 +141,6 @@ static const char *task_state_array[] =
"X (dead)", /* 32 */
"x (dead)", /* 64 */
"K (wakekill)", /* 128 */
- "W (waking)", /* 256 */
};
static inline const char *get_task_state(struct task_struct *tsk)
Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h
+++ linux-2.6/include/linux/sched.h
@@ -189,10 +189,9 @@ print_cfs_rq(struct seq_file *m, int cpu
/* in tsk->state again */
#define TASK_DEAD 64
#define TASK_WAKEKILL 128
-#define TASK_WAKING 256
-#define TASK_STATE_MAX 512
+#define TASK_STATE_MAX 256
-#define TASK_STATE_TO_CHAR_STR "RSDTtZXxKW"
+#define TASK_STATE_TO_CHAR_STR "RSDTtZXxK"
extern char ___assert_task_state[1 - 2*!!(
sizeof(TASK_STATE_TO_CHAR_STR)-1 != ilog2(TASK_STATE_MAX)+1)];
Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -2175,7 +2175,7 @@ void set_task_cpu(struct task_struct *p,
* We should never call set_task_cpu() on a blocked task,
* ttwu() will sort out the placement.
*/
- WARN_ON_ONCE(p->state != TASK_RUNNING && p->state != TASK_WAKING &&
+ WARN_ON_ONCE(p->state != TASK_RUNNING &&
!(task_thread_info(p)->preempt_count & PREEMPT_ACTIVE));
#ifdef CONFIG_LOCKDEP
@@ -2613,7 +2613,7 @@ try_to_wake_up(struct task_struct *p, un
smp_rmb();
p->sched_contributes_to_load = !!task_contributes_to_load(p);
- p->state = TASK_WAKING;
+ p->state = TASK_RUNNING;
if (p->sched_class->task_waking)
p->sched_class->task_waking(p);
On 03/02/11 09:38, Peter Zijlstra wrote:
> With the new locking TASK_WAKING has become obsolete, remove it.
>
> Suggested-by: Oleg Nesterov <[email protected]>
> Signed-off-by: Peter Zijlstra <[email protected]>
> LKML-Reference: <new-submission>
> ---
< snip >
> Index: linux-2.6/kernel/sched.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched.c
> +++ linux-2.6/kernel/sched.c
> @@ -2175,7 +2175,7 @@ void set_task_cpu(struct task_struct *p,
> * We should never call set_task_cpu() on a blocked task,
> * ttwu() will sort out the placement.
> */
> - WARN_ON_ONCE(p->state != TASK_RUNNING && p->state != TASK_WAKING &&
> + WARN_ON_ONCE(p->state != TASK_RUNNING &&
> !(task_thread_info(p)->preempt_count & PREEMPT_ACTIVE));
>
> #ifdef CONFIG_LOCKDEP
> @@ -2613,7 +2613,7 @@ try_to_wake_up(struct task_struct *p, un
> smp_rmb();
>
> p->sched_contributes_to_load = !!task_contributes_to_load(p);
> - p->state = TASK_WAKING;
> + p->state = TASK_RUNNING;
>
> if (p->sched_class->task_waking)
> p->sched_class->task_waking(p);
No harm if the coded as in the patch, but an alternate suggestion
if you like it:
The only reason left for "p->state = TASK_RUNNING;" here is when
cpu is remote. If cpu is not remote then p->state will be set by:
ttwu_queue()
ttwu_do_activate()
ttwu_do_wakeup()
p->state = TASK_RUNNING;
It would be more clear that setting state to TASK_RUNNING is protecting
the process until it has been removed from the wake_list by
sched_ttwu_pending() by setting p->state = TASK_RUNNING in ttwu_queue_remote().
On Thu, 2011-03-10 at 17:49 -0800, Frank Rowand wrote:
> On 03/02/11 09:38, Peter Zijlstra wrote:
> > With the new locking TASK_WAKING has become obsolete, remove it.
> >
> > Suggested-by: Oleg Nesterov <[email protected]>
> > Signed-off-by: Peter Zijlstra <[email protected]>
> > LKML-Reference: <new-submission>
> > ---
>
> < snip >
>
> > Index: linux-2.6/kernel/sched.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/sched.c
> > +++ linux-2.6/kernel/sched.c
> > @@ -2175,7 +2175,7 @@ void set_task_cpu(struct task_struct *p,
> > * We should never call set_task_cpu() on a blocked task,
> > * ttwu() will sort out the placement.
> > */
> > - WARN_ON_ONCE(p->state != TASK_RUNNING && p->state != TASK_WAKING &&
> > + WARN_ON_ONCE(p->state != TASK_RUNNING &&
> > !(task_thread_info(p)->preempt_count & PREEMPT_ACTIVE));
> >
> > #ifdef CONFIG_LOCKDEP
> > @@ -2613,7 +2613,7 @@ try_to_wake_up(struct task_struct *p, un
> > smp_rmb();
> >
> > p->sched_contributes_to_load = !!task_contributes_to_load(p);
> > - p->state = TASK_WAKING;
> > + p->state = TASK_RUNNING;
> >
> > if (p->sched_class->task_waking)
> > p->sched_class->task_waking(p);
>
> No harm if the coded as in the patch, but an alternate suggestion
> if you like it:
>
> The only reason left for "p->state = TASK_RUNNING;" here is when
> cpu is remote. If cpu is not remote then p->state will be set by:
>
> ttwu_queue()
> ttwu_do_activate()
> ttwu_do_wakeup()
> p->state = TASK_RUNNING;
>
> It would be more clear that setting state to TASK_RUNNING is protecting
> the process until it has been removed from the wake_list by
> sched_ttwu_pending() by setting p->state = TASK_RUNNING in ttwu_queue_remote().
>
Yeah, its a bit of a maze.. maybe we should just drop this and keep the
slightly redundant but more clear TASK_WAKING around.