Bug in scheduler when using rt_mutex

Hi

I believe that there are some problems in the scheduling when the following happens:
- Normal priority process locks rt_mutex and sleeps while keeping it locked.

- RT priority process blocks on the rt_mutex while normal priority process is sleeping

This sequence can occur with I2C access when both normal priority thread and irq-thread access the same I2C bus. I2C core contains rt_mutex and I2C drivers can sleep with wait_for_completion.

Based on my debugging following sequence occurs (single CPU system):

1) There is some user process running at the background (like cat /dev/zero..)
2) User process reads sysfs entry which causes I2C acccess
3) User process locks rt_mutex in the I2C-core
4) User process sleeps while it keeps rt_mutex locked (wait_for_completion in I2C transfer function)

-----Original Message-----
From: ext Peter Zijlstra [mailto:[email protected]]
Sent: 17 January, 2011 17:00
To: Onkalo Samu.P (Nokia-MS/Tampere)
Cc: [email protected]; [email protected]; tglx
Subject: Re: Bug in scheduler when using rt_mutex

On Mon, 2011-01-17 at 16:42 +0200, Onkalo Samu wrote:

Quoted 6 lines

Hi

I believe that there are some problems in the scheduling when the following happens:
- Normal priority process locks rt_mutex and sleeps while keeping it locked.

There's your fail, don't do that!

Quoted 7 lines

- RT priority process blocks on the rt_mutex while normal priority process is sleeping

This sequence can occur with I2C access when both normal priority thread and irq-thread access the same I2C bus. I2C core contains rt_mutex and I2C drivers can sleep with wait_for_completion.

Why does I2C core use rt_mutex, that's utterly broken.

Quoted 10 lines

Based on my debugging following sequence occurs (single CPU system):

1) There is some user process running at the background (like cat /dev/zero..)
2) User process reads sysfs entry which causes I2C acccess
3) User process locks rt_mutex in the I2C-core
4) User process sleeps while it keeps rt_mutex locked (wait_for_completion in I2C transfer function)

That's where things go wrong, there's absolutely nothing you can do to fix the system once you block while holding a mutex.

Quoted 16 lines

-----Original Message-----
From: ext Peter Zijlstra [mailto:[email protected]]
Sent: 17 January, 2011 17:00
To: Onkalo Samu.P (Nokia-MS/Tampere)
Cc: [email protected]; [email protected]; tglx
Subject: Re: Bug in scheduler when using rt_mutex

Quoted 6 lines · Onkalo Samu

Hi

I believe that there are some problems in the scheduling when the following happens:
- Normal priority process locks rt_mutex and sleeps while keeping it locked.

Quoted 1 line

There's your fail, don't do that!

So that is forbidden:

rt_mutex_lock();
wait_for_completion(); <--- shared HW finishes its job rt_mutex_unlock();

Quoted 1 line

Why does I2C core use rt_mutex, that's utterly broken.

To get low priority task finish ongoing I2C access in time under heavy load cases I think.

Quoted 10 lines

Quoted 9 lines

Based on my debugging following sequence occurs (single CPU system):

1) There is some user process running at the background (like cat /dev/zero..)
2) User process reads sysfs entry which causes I2C acccess
3) User process locks rt_mutex in the I2C-core
4) User process sleeps while it keeps rt_mutex locked (wait_for_completion in I2C transfer function)

Quoted 2 lines

That's where things go wrong, there's absolutely nothing you can do to fix the system once you block while holding a mutex.

Of course other processes are waiting until the (rt_)mutex is unlocked. Problem is that after the rt_mutex_unlock is done, the task which just released the lock, may be in some non-running state for minutes.

Failure case:
- user process locks rt_mutex
- and goes to sleep (wait_for_completion etc.)
- user process is dequeued to sleep state
-> vruntime is not updated in dequeue_entity

Quoted 7 lines · Onkalo Samu

Failure case:
- user process locks rt_mutex
- and goes to sleep (wait_for_completion etc.)
- user process is dequeued to sleep state
-> vruntime is not updated in dequeue_entity

Does the below (completely untested) patch help?

---
 kernel/sched.c      |    6 +++++-
 kernel/sched_fair.c |   11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletions(-)

kernel/sched.c +5 -1

index a0eb094..be09581 100644
@@ -8108,8 +8108,10 @@ EXPORT_SYMBOL(__might_sleep);
 #ifdef CONFIG_MAGIC_SYSRQ
 static void normalize_task(struct rq *rq, struct task_struct *p)
 {
+	struct sched_class *prev_class = p->sched_class;
+	int old_prio = p->prio;
 	int on_rq;
-
+       
 	on_rq = p->se.on_rq;
 	if (on_rq)
 		deactivate_task(rq, p, 0);

@@ -8118,6 +8120,8 @@ static void normalize_task(struct rq *rq, struct task_struct *p)
 		activate_task(rq, p, 0);
 		resched_task(rq->curr);
 	}
+
+	check_class_changed(rq, p, prev_class, old_prio, task_current(rq, p));
 }
 
 void normalize_rt_tasks(void)

kernel/sched_fair.c +11 -0

index c62ebae..0a27b00 100644
@@ -4072,6 +4072,17 @@ static void prio_changed_fair(struct rq *rq, struct task_struct *p,
 static void switched_to_fair(struct rq *rq, struct task_struct *p,
 			     int running)
 {
+	struct sched_entity *se = &p->se;
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+	if (se->on_rq && cfs_rq->curr != se)
+		__dequeue_entity(cfs_rq, se);
+
+	place_entity(cfs_rq, se, 0);
+
+	if (se->on_rq && cfs_rq->curr != se)
+		__enqueue_entity(cfs_rq, se);
+
 	/*
 	 * We were most likely switched from sched_rt, so
 	 * kick off the schedule if running, otherwise just see

Quoted 9 lines · ext Peter Zijlstra

Quoted 7 lines · Onkalo Samu

Failure case:
- user process locks rt_mutex
- and goes to sleep (wait_for_completion etc.)
- user process is dequeued to sleep state
-> vruntime is not updated in dequeue_entity

Quoted 1 line

Does the below (completely untested) patch help?

Unfortunately no.

Quoted 10 lines · Onkalo Samu

Quoted 9 lines · ext Peter Zijlstra

Quoted 7 lines · Onkalo Samu

Failure case:
- user process locks rt_mutex
- and goes to sleep (wait_for_completion etc.)
- user process is dequeued to sleep state
-> vruntime is not updated in dequeue_entity

Quoted 3 lines

Quoted 1 line

Does the below (completely untested) patch help?

Unfortunately no.

It couldn't work because place_entity() will not place it backwards.

Quoted 13 lines · Yong Zhang

Quoted 12 lines · Onkalo Samu

Quoted 10 lines · ext Peter Zijlstra

Quoted 7 lines · Onkalo Samu

Failure case:
- user process locks rt_mutex
- and goes to sleep (wait_for_completion etc.)
- user process is dequeued to sleep state
-> vruntime is not updated in dequeue_entity

Does the below (completely untested) patch help?

Quoted 4 lines

Quoted 1 line

Unfortunately no.

It couldn't work because place_entity() will not place it backwards.

Ah indeed, I was somehow assuming it was way left, but that is not at all true, something like the below then..

---
Subject: sched: Fix switch_to_fair()
From: Peter Zijlstra <[email protected]>
Date: Mon Jan 17 17:03:27 CET 2011

When a task is placed back into fair_sched_class, we must update its placement, since we don't know how long its been gone, hence its vruntime is stale and cannot be trusted.

There is also a case where it was moved from fair_sched_class when it was in a blocked state and moved back while it is running, this causes an imbalance between DEQUEUE_SLEEP/DEQUEUE_WAKEUP for the fair class and leaves vruntime way out there (due to the min_vruntime adjustment).

Also update sysrq-n to call the ->switch_{to,from} methods.

Reported-by Onkalo Samu <[email protected]>

Signed-off-by Peter Zijlstra <[email protected]>

---
 kernel/sched.c      |    4 ++++
 kernel/sched_fair.c |   16 ++++++++++++++++
 2 files changed, 20 insertions(+)

Index: linux-2.6/kernel/sched.c
===================================================================

kernel/sched.c +4 -0

@@ -8106,6 +8106,8 @@ EXPORT_SYMBOL(__might_sleep);
 #ifdef CONFIG_MAGIC_SYSRQ
 static void normalize_task(struct rq *rq, struct task_struct *p)
 {
+	struct sched_class *prev_class = p->sched_class;
+	int old_prio = p->prio;
 	int on_rq;
 
 	on_rq = p->se.on_rq;

@@ -8116,6 +8118,8 @@ static void normalize_task(struct rq *rq
 		activate_task(rq, p, 0);
 		resched_task(rq->curr);
 	}
+
+	check_class_changed(rq, p, prev_class, old_prio, task_current(rq, p));
 }
 
 void normalize_rt_tasks(void)

Index: linux-2.6/kernel/sched_fair.c
===================================================================

kernel/sched_fair.c +16 -0

@@ -4075,6 +4075,22 @@ static void prio_changed_fair(struct rq
 static void switched_to_fair(struct rq *rq, struct task_struct *p,
 			     int running)
 {
+	struct sched_entity *se = &p->se;
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+	if (se->on_rq && cfs_rq->curr != se)
+		__dequeue_entity(cfs_rq, se);
+
+	/*
+	 * se->vruntime can be completely out there, there is no telling
+	 * how long this task was !fair and on what CPU if any it became
+	 * !fair. Therefore, reset it to a known, reasonable value.
+	 */
+	se->vruntime = cfs_rq->min_vruntime;
+
+	if (se->on_rq && cfs_rq->curr != se)
+		__enqueue_entity(cfs_rq, se);
+
 	/*
 	 * We were most likely switched from sched_rt, so
 	 * kick off the schedule if running, otherwise just see

Subject: sched: Fix switch_to_fair()
From: Peter Zijlstra <[email protected]>
Date: Mon Jan 17 17:03:27 CET 2011

When a task is placed back into fair_sched_class, we must update its placement, since we don't know how long its been gone, hence its vruntime is stale and cannot be trusted.

There is also a case where it was moved from fair_sched_class when it was in a blocked state and moved back while it is running, this causes an imbalance between DEQUEUE_SLEEP/DEQUEUE_WAKEUP for the fair class and leaves vruntime way out there (due to the min_vruntime adjustment).

Also update sysrq-n to call the ->switch_{to,from} methods.

Reported-by Onkalo Samu <[email protected]>

Signed-off-by Peter Zijlstra <[email protected]>

---
 kernel/sched.c      |    4 ++++
 kernel/sched_fair.c |   16 ++++++++++++++++

 2 files changed, 20 insertions(+)

Index: linux-2.6/kernel/sched_fair.c
===================================================================

Quoted 6 lines

Index: linux-2.6/kernel/sched_fair.c
===================================================================

kernel/sched_fair.c +0 -0

@@ -4075,6 +4075,22 @@ static void prio_changed_fair(struct rq
 static void switched_to_fair(struct rq *rq, struct task_struct *p,

kernel/sched_fair.c static void prio_changed_fair(struct rq · line 4081

Quoted 6 lines

kernel/sched_fair.c +4 -0

                            int running)
 {
+       struct sched_entity *se = &p->se;
+       struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+       if (se->on_rq && cfs_rq->curr != se)

(cfs_rq->curr != se) equals to (!running), no?

kernel/sched_fair.c static void prio_changed_fair(struct rq · line 4089

Quoted 8 lines

kernel/sched_fair.c +8 -0

+               __dequeue_entity(cfs_rq, se);
+
+       /*
+        * se->vruntime can be completely out there, there is no telling
+        * how long this task was !fair and on what CPU if any it became
+        * !fair. Therefore, reset it to a known, reasonable value.
+        */
+       se->vruntime = cfs_rq->min_vruntime;

But this is not fair for !SLEEP task.
You know se->vruntime -= cfs_rq->min_vruntime for !SLEEP task, then after it go through sched_fair-->sched_rt-->sched_fair by some means, current cfs_rq->min_vruntime is added back.

But here se is putted before where it should be. Is this what we want?

Quoted 1 line

(cfs_rq->curr != se) equals to (!running), no?

No, running is task_of(se) == rq->curr.  Another class or fair group task may be rq_of(cfs_rq)->curr

cfs_rq->curr != se is always true.

Quoted 1 line · Yong Zhang

cfs_rq->curr != se is always true.

If that were always true, we'd illegally enqueue a running task.

Quoted 5 lines · Mike Galbraith

Quoted 1 line · Yong Zhang

cfs_rq->curr != se is always true.

If that were always true, we'd illegally enqueue a running task.

I'm sorry that I'm not express myself correctly.

The conclusion of (cfs_rq->curr != se is always true) is not self-contained. IOW, it's based on one condition which is (task_of(se) != rq->curr). So what I want to say is:
task_of(se) != rq->curr ==> cfs_rq_of(se)->curr != se
So,

         !running                   ==>   cfs_rq_of(se)->curr != se

Is this more clear?

Quoted 8 lines · Yong Zhang

Quoted 5 lines · Mike Galbraith

Quoted 1 line · Yong Zhang

cfs_rq->curr != se is always true.

If that were always true, we'd illegally enqueue a running task.

I'm sorry that I'm not express myself correctly.

Human communication methods are all buggy as hell :)

Quoted 10 lines · Mike Galbraith

Quoted 8 lines · Yong Zhang

Quoted 5 lines · Mike Galbraith

Quoted 1 line · Yong Zhang

cfs_rq->curr != se is always true.

If that were always true, we'd illegally enqueue a running task.

I'm sorry that I'm not express myself correctly.

Quoted 1 line

Human communication methods are all buggy as hell :)

Not to mention that they are slow, inefficient and ambiguous.

But wht did you expect? The original authors of the code are long gone and maintenance is done by newcomers who are patching the code bit by bit. What you get from such a development model is pretty predictable: ~1 billion years old spaghetti DNA that no-one truly understands.

Quoted 6 lines

Index: linux-2.6/kernel/sched_fair.c
===================================================================

kernel/sched_fair.c +0 -0

@@ -4075,6 +4075,22 @@ static void prio_changed_fair(struct rq
 static void switched_to_fair(struct rq *rq, struct task_struct *p,

kernel/sched_fair.c static void prio_changed_fair(struct rq · line 4081

Quoted 6 lines

kernel/sched_fair.c +4 -0

                            int running)
 {
+       struct sched_entity *se = &p->se;
+       struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+       if (se->on_rq && cfs_rq->curr != se)

(cfs_rq->curr != se) equals to (!running), no?

kernel/sched_fair.c static void prio_changed_fair(struct rq · line 4089

Quoted 8 lines

kernel/sched_fair.c +8 -0

+               __dequeue_entity(cfs_rq, se);
+
+       /*
+        * se->vruntime can be completely out there, there is no telling
+        * how long this task was !fair and on what CPU if any it became
+        * !fair. Therefore, reset it to a known, reasonable value.
+        */
+       se->vruntime = cfs_rq->min_vruntime;

But this is not fair for !SLEEP task.
You know se->vruntime -= cfs_rq->min_vruntime for !SLEEP task, then after it go through sched_fair-->sched_rt-->sched_fair by some means, current cfs_rq->min_vruntime is added back.

But here se is putted before where it should be. Is this what we want?

kernel/sched_fair.c +16 -2

+static void
+switched_from_fair(struct rq *rq, struct task_struct *p, int running)
+{
+       struct sched_entity *se = &p->se;
+       struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+       if (!se->on_rq && p->state != TASK_RUNNING)
+               se->vruntime -= cfs_rq->min_vruntime;
+}
+
 /*
  * We switched to the sched_fair class.
  */
-static void switched_to_fair(struct rq *rq, struct task_struct *p,
-                            int running)
+static void
+switched_to_fair(struct rq *rq, struct task_struct *p, int running)
 {
+       struct sched_entity *se = &p->se;
+       struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+       if (se->on_rq && cfs_rq->curr != se)

Hrm, I think the to bit is not needed with the from thing in place, the enqueue from _setprio will already have added min_vruntime

Quoted 2 lines · Peter Zijlstra

Hrm, I think the to bit is not needed with the from thing in place, the enqueue from _setprio will already have added min_vruntime

Would lead to something like this:

---
Index: linux-2.6/kernel/sched.c
===================================================================

kernel/sched.c +4 -0

@@ -8108,6 +8108,8 @@ EXPORT_SYMBOL(__might_sleep);
 #ifdef CONFIG_MAGIC_SYSRQ
 static void normalize_task(struct rq *rq, struct task_struct *p)
 {
+	struct sched_class *prev_class = p->sched_class;
+	int old_prio = p->prio;
 	int on_rq;
 
 	on_rq = p->se.on_rq;

@@ -8118,6 +8120,8 @@ static void normalize_task(struct rq *rq
 		activate_task(rq, p, 0);
 		resched_task(rq->curr);
 	}
+
+	check_class_changed(rq, p, prev_class, old_prio, task_current(rq, p));
 }
 
 void normalize_rt_tasks(void)

Index: linux-2.6/kernel/sched_fair.c
===================================================================

kernel/sched_fair.c +22 -2

@@ -4066,11 +4066,30 @@ static void prio_changed_fair(struct rq
 		check_preempt_curr(rq, p, 0);
 }
 
+static void
+switched_from_fair(struct rq *rq, struct task_struct *p, int running)
+{
+	struct sched_entity *se = &p->se;
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+	/*
+	 * Ensure the task's vruntime is normalized, so that when its
+	 * switched back to the fair class the enqueue_entity(.flags=0) will
+	 * do the right thing.
+	 *
+	 * If it was on_rq, then the dequeue_entity(.flags=0) will already
+	 * have normalized the vruntime, if it was !on_rq, then only when
+	 * the task is sleeping will it still have non-normalized vruntime.
+	 */
+	if (!se->on_rq && p->state != TASK_RUNNING)
+		se->vruntime -= cfs_rq->min_vruntime;
+}
+
 /*
  * We switched to the sched_fair class.
  */
-static void switched_to_fair(struct rq *rq, struct task_struct *p,
-			     int running)
+static void
+switched_to_fair(struct rq *rq, struct task_struct *p, int running)
 {
 	/*
 	 * We were most likely switched from sched_rt, so

@@ -4163,6 +4182,7 @@ static const struct sched_class fair_sch
 	.task_fork		= task_fork_fair,
 
 	.prio_changed		= prio_changed_fair,
+	.switched_from		= switched_from_fair,
 	.switched_to		= switched_to_fair,
 
 	.get_rr_interval	= get_rr_interval_fair,

Quoted 8 lines · ext Peter Zijlstra

Quoted 2 lines · Peter Zijlstra

Hrm, I think the to bit is not needed with the from thing in place, the enqueue from _setprio will already have added min_vruntime

Would lead to something like this:

Doesn't work in my case.

Quoted 70 lines

---
Index: linux-2.6/kernel/sched.c
===================================================================

kernel/sched.c +4 -0

@@ -8108,6 +8108,8 @@ EXPORT_SYMBOL(__might_sleep);
 #ifdef CONFIG_MAGIC_SYSRQ
 static void normalize_task(struct rq *rq, struct task_struct *p)
 {
+	struct sched_class *prev_class = p->sched_class;
+	int old_prio = p->prio;
 	int on_rq;
 
 	on_rq = p->se.on_rq;

@@ -8118,6 +8120,8 @@ static void normalize_task(struct rq *rq
 		activate_task(rq, p, 0);
 		resched_task(rq->curr);
 	}
+
+	check_class_changed(rq, p, prev_class, old_prio, task_current(rq, p));
 }
 
 void normalize_rt_tasks(void)

Index: linux-2.6/kernel/sched_fair.c
===================================================================

kernel/sched_fair.c +22 -2

@@ -4066,11 +4066,30 @@ static void prio_changed_fair(struct rq
 		check_preempt_curr(rq, p, 0);
 }
 
+static void
+switched_from_fair(struct rq *rq, struct task_struct *p, int running)
+{
+	struct sched_entity *se = &p->se;
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+	/*
+	 * Ensure the task's vruntime is normalized, so that when its
+	 * switched back to the fair class the enqueue_entity(.flags=0) will
+	 * do the right thing.
+	 *
+	 * If it was on_rq, then the dequeue_entity(.flags=0) will already
+	 * have normalized the vruntime, if it was !on_rq, then only when
+	 * the task is sleeping will it still have non-normalized vruntime.
+	 */
+	if (!se->on_rq && p->state != TASK_RUNNING)
+		se->vruntime -= cfs_rq->min_vruntime;
+}
+
 /*
  * We switched to the sched_fair class.
  */
-static void switched_to_fair(struct rq *rq, struct task_struct *p,
-			     int running)
+static void
+switched_to_fair(struct rq *rq, struct task_struct *p, int running)
 {
 	/*
 	 * We were most likely switched from sched_rt, so

@@ -4163,6 +4182,7 @@ static const struct sched_class fair_sch
 	.task_fork		= task_fork_fair,
 
 	.prio_changed		= prio_changed_fair,
+	.switched_from		= switched_from_fair,
 	.switched_to		= switched_to_fair,
 
 	.get_rr_interval	= get_rr_interval_fair,

When the task is sleeping rt_mutex_setprio doesn't call check_class_changed since the task is not in queue at that moment I think.

kernel/sched_fair.c +6 -0

+static void switched_from_fair(struct rq *rq, struct task_struct *p)
+{
+       struct sched_entity *se = &p->se;
+       struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+       /*

Quoted 10 lines · Peter Zijlstra

kernel/sched_fair.c +10 -0

+static void switched_from_fair(struct rq *rq, struct task_struct *p)
+{
+       struct sched_entity *se = &p->se;
+       struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+       /*
+        * Ensure the task's vruntime is normalized, so that when its
+        * switched back to the fair class the enqueue_entity(.flags=0) will
+        * do the right thing.
+        *

Quoted 6 lines

+        * If it was on_rq, then the dequeue_entity(.flags=0) will already
+        * have normalized the vruntime, if it was !on_rq, then only when
+        * the task is sleeping will it still have non-normalized vruntime.
+        */
+       if (!se->on_rq && p->state != TASK_RUNNING)
+               se->vruntime -= cfs_rq->min_vruntime;

Here it's possible that se->vruntime is little than cfs_rq->min_vruntime.

Quoted 11 lines · Yong Zhang

Quoted 10 lines · Peter Zijlstra

kernel/sched_fair.c +10 -0

+static void switched_from_fair(struct rq *rq, struct task_struct *p)
+{
+       struct sched_entity *se = &p->se;
+       struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+       /*
+        * Ensure the task's vruntime is normalized, so that when its
+        * switched back to the fair class the enqueue_entity(.flags=0) will
+        * do the right thing.
+        *

Quoted 8 lines

Quoted 6 lines

+        * If it was on_rq, then the dequeue_entity(.flags=0) will already
+        * have normalized the vruntime, if it was !on_rq, then only when
+        * the task is sleeping will it still have non-normalized vruntime.
+        */
+       if (!se->on_rq && p->state != TASK_RUNNING)
+               se->vruntime -= cfs_rq->min_vruntime;

Here it's possible that se->vruntime is little than cfs_rq->min_vruntime.

Maybe we can:
                      place_entity(cfs_rq, se, 0); se->vruntime -= cfs_rq->min_vruntime;

Quoted 10 lines · Peter Zijlstra

kernel/sched_fair.c +10 -0

+static void switched_from_fair(struct rq *rq, struct task_struct *p)
+{
+       struct sched_entity *se = &p->se;
+       struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+       /*
+        * Ensure the task's vruntime is normalized, so that when its
+        * switched back to the fair class the enqueue_entity(.flags=0) will
+        * do the right thing.
+        *

kernel/sched_fair.c static void task_fork_fair(struct task_s · line 4090

Quoted 6 lines

kernel/sched_fair.c +6 -0

+        * If it was on_rq, then the dequeue_entity(.flags=0) will already
+        * have normalized the vruntime, if it was !on_rq, then only when
+        * the task is sleeping will it still have non-normalized vruntime.
+        */
+       if (!se->on_rq && p->state != TASK_RUNNING)
+               se->vruntime -= cfs_rq->min_vruntime;

Here it's possible that se->vruntime is little than cfs_rq->min_vruntime.

or the task is enqueued
at it's last offset as usual for runnable tasks.

Quoted 2 lines · Mike Galbraith

or the task is enqueued
at it's last offset as usual for runnable tasks.

But shouldn't we task the tasks as WAKEUP, through the task has been waked on other sched_class?

IOW, I wonder if we should play with place_entity() at some point.

If the task returns as a sleeper, place entity() will be called when it is awakened, so it's sleep credit will be clipped as usual.  So vruntime can be much less than min_vruntime at class exit time, and it doesn't matter, clipping on wakeup after re-entry takes care of it.. if that's what you were thinking about.

Quoted 5 lines · Mike Galbraith

If the task returns as a sleeper, place entity() will be called when it is awakened, so it's sleep credit will be clipped as usual. So vruntime can be much less than min_vruntime at class exit time, and it doesn't matter, clipping on wakeup after re-entry takes care of it.. if that's what you were thinking about.

For a sleep task which stay in sched_fair before it's waked:
try_to_wake_up()
ttwu_activate()

    activate_task()
      enqueue_task_fair()
        enqueue_entity()
          place_entity()        <== clip vruntime

For a sleep task which promote to sched_rt when it's sleep:
rt_mutex_setprio()
check_class_changed()

    switch_from_fair()       <== vruntime -= min_vruntime
      try_to_wake_up()
        ...run then stay on rq
        rt_mutex_setprio()
          enqueue_task_fair()     <==vruntime += min_vruntime

The difference is that in the second case, place_entity() is not called, but wrt sched_fair, the task is a WAKEUP task. Then we place this task in sched_fair before where it should be.

Quoted 24 lines · Yong Zhang

Quoted 5 lines · Mike Galbraith

If the task returns as a sleeper, place entity() will be called when it is awakened, so it's sleep credit will be clipped as usual.  So vruntime can be much less than min_vruntime at class exit time, and it doesn't matter, clipping on wakeup after re-entry takes care of it.. if that's what you were thinking about.

For a sleep task which stay in sched_fair before it's waked:
try_to_wake_up()
  ttwu_activate()
    activate_task()
      enqueue_task_fair()
        enqueue_entity()
          place_entity()        <== clip vruntime

For a sleep task which promote to sched_rt when it's sleep:
rt_mutex_setprio()
  check_class_changed()
    switch_from_fair()       <== vruntime -= min_vruntime try_to_wake_up()
        ...run then stay on rq
        rt_mutex_setprio()
          enqueue_task_fair()     <==vruntime += min_vruntime

Quoted 3 lines

The difference is that in the second case, place_entity() is not called, but wrt sched_fair, the task is a WAKEUP task. Then we place this task in sched_fair before where it should be.

D'oh.  You're right, he needs to be clipped before he leaves.

Quoted 25 lines · Mike Galbraith

Quoted 24 lines · Yong Zhang

Quoted 5 lines · Mike Galbraith

If the task returns as a sleeper, place entity() will be called when it is awakened, so it's sleep credit will be clipped as usual. So vruntime can be much less than min_vruntime at class exit time, and it doesn't matter, clipping on wakeup after re-entry takes care of it.. if that's what you were thinking about.

For a sleep task which stay in sched_fair before it's waked:
try_to_wake_up()
ttwu_activate()

    activate_task()
      enqueue_task_fair()
        enqueue_entity()
          place_entity()        <== clip vruntime

For a sleep task which promote to sched_rt when it's sleep:
rt_mutex_setprio()
check_class_changed()

    switch_from_fair()       <== vruntime -= min_vruntime
      try_to_wake_up()
        ...run then stay on rq
        rt_mutex_setprio()
          enqueue_task_fair()     <==vruntime += min_vruntime

Quoted 5 lines

Quoted 3 lines

The difference is that in the second case, place_entity() is not called, but wrt sched_fair, the task is a WAKEUP task. Then we place this task in sched_fair before where it should be.

D'oh. You're right, he needs to be clipped before he leaves.

Exactly we should clip it when it comes back, because it still could sleep for some time after it leaves ;)

Quoted 26 lines · Yong Zhang

Quoted 25 lines · Mike Galbraith

Quoted 24 lines · Yong Zhang

Quoted 5 lines · Mike Galbraith

If the task returns as a sleeper, place entity() will be called when it is awakened, so it's sleep credit will be clipped as usual. So vruntime can be much less than min_vruntime at class exit time, and it doesn't matter, clipping on wakeup after re-entry takes care of it.. if that's what you were thinking about.

For a sleep task which stay in sched_fair before it's waked:
try_to_wake_up()
ttwu_activate()

    activate_task()
      enqueue_task_fair()
        enqueue_entity()
          place_entity()        <== clip vruntime

For a sleep task which promote to sched_rt when it's sleep:
rt_mutex_setprio()
check_class_changed()

    switch_from_fair()       <== vruntime -= min_vruntime
      try_to_wake_up()
        ...run then stay on rq
        rt_mutex_setprio()
          enqueue_task_fair()     <==vruntime += min_vruntime

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

The difference is that in the second case, place_entity() is not called, but wrt sched_fair, the task is a WAKEUP task. Then we place this task in sched_fair before where it should be.

D'oh. You're right, he needs to be clipped before he leaves.

Exactly we should clip it when it comes back, because it still could sleep for some time after it leaves ;)

That's ok, we don't and aren't supposed to care what happens while he's gone. But we do have to make sure that vruntime is sane either when he leaves, or when he comes back. Seems to me the easiest is clip when he leaves to cover him having slept a long time before leaving, then coming back on us as a runner. If he comes back as a sleeper, he'll be clipped again anyway, so all is well.

sched_fork() should probably zero child's vruntime too, so non-fair children can't enter fair_class with some bogus lag they never had.

Quoted 9 lines

That's ok, we don't and aren't supposed to care what happens while he's gone. But we do have to make sure that vruntime is sane either when he leaves, or when he comes back. Seems to me the easiest is clip when he leaves to cover him having slept a long time before leaving, then coming back on us as a runner. If he comes back as a sleeper, he'll be clipped again anyway, so all is well.

sched_fork() should probably zero child's vruntime too, so non-fair children can't enter fair_class with some bogus lag they never had.

Something like so?

Index: linux-2.6/kernel/sched.c
===================================================================

kernel/sched.c +0 -0

 	if (p->sched_class->task_fork)
 		p->sched_class->task_fork(p);

Index: linux-2.6/kernel/sched_fair.c
===================================================================

kernel/sched_fair.c +0 -0

@@ -4086,8 +4086,14 @@ static void switched_from_fair(struct rq
 	 * have normalized the vruntime, if it was !on_rq, then only when

Quoted 6 lines

Index: linux-2.6/kernel/sched.c
===================================================================

kernel/sched.c +0 -0

@@ -2624,6 +2624,8 @@ void sched_fork(struct task_struct *p, i
 

kernel/sched.c void sched_fork(struct task_struct *p, i · line 2628

Quoted 4 lines

kernel/sched.c +2 -0

      if (!rt_prio(p->prio))
              p->sched_class = &fair_sched_class;
+     else
+             p->se.vruntime = 0;

This can be moved to __sched_fork()

Quoted 6 lines · Yong Zhang

Quoted 6 lines

Index: linux-2.6/kernel/sched.c
===================================================================

kernel/sched.c +0 -0

@@ -2624,6 +2624,8 @@ void sched_fork(struct task_struct *p, i
 

Quoted 6 lines

kernel/sched.c void sched_fork(struct task_struct *p, i · line 2628

Quoted 4 lines

kernel/sched.c +2 -0

      if (!rt_prio(p->prio))
              p->sched_class = &fair_sched_class;
+     else
+             p->se.vruntime = 0;

This can be moved to __sched_fork()

But we cannot do it unconditionally, we want to inherit the parent's ->vruntime when we're a fair task clone.

Quoted 14 lines · Peter Zijlstra

Quoted 12 lines · Yong Zhang

kernel/sched.c void sched_fork(struct task_struct *p, i · line 2628

Quoted 10 lines

Index: linux-2.6/kernel/sched.c
===================================================================

kernel/sched.c +2 -0

@@ -2624,6 +2624,8 @@ void sched_fork(struct task_struct *p, i
 
      if (!rt_prio(p->prio))
              p->sched_class = &fair_sched_class;
+     else
+             p->se.vruntime = 0;

This can be moved to __sched_fork()

Quoted 2 lines

But we cannot do it unconditionally, we want to inherit the parent's ->vruntime when we're a fair task clone.

Doesn't task_fork_fair()(which is called after __sched_fork()) take the inherit job?

Am I missing something?

Quoted 26 lines · Yong Zhang

Quoted 25 lines · Mike Galbraith

Quoted 24 lines · Yong Zhang

Quoted 5 lines · Mike Galbraith

If the task returns as a sleeper, place entity() will be called when it is awakened, so it's sleep credit will be clipped as usual. So vruntime can be much less than min_vruntime at class exit time, and it doesn't matter, clipping on wakeup after re-entry takes care of it.. if that's what you were thinking about.

For a sleep task which stay in sched_fair before it's waked:
try_to_wake_up()
ttwu_activate()

    activate_task()
      enqueue_task_fair()
        enqueue_entity()
          place_entity()        <== clip vruntime

For a sleep task which promote to sched_rt when it's sleep:
rt_mutex_setprio()
check_class_changed()

    switch_from_fair()       <== vruntime -= min_vruntime
      try_to_wake_up()
        ...run then stay on rq
        rt_mutex_setprio()
          enqueue_task_fair()     <==vruntime += min_vruntime

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

The difference is that in the second case, place_entity() is not called, but wrt sched_fair, the task is a WAKEUP task. Then we place this task in sched_fair before where it should be.

D'oh. You're right, he needs to be clipped before he leaves.

Exactly we should clip it when it comes back, because it still could sleep for some time after it leaves ;)

That's ok, we don't and aren't supposed to care what happens while he's gone. But we do have to make sure that vruntime is sane either when he leaves, or when he comes back. Seems to me the easiest is clip when he leaves to cover him having slept a long time before leaving, then coming back on us as a runner. If he comes back as a sleeper, he'll be clipped again anyway, so all is well.

sched_fork() should probably zero child's vruntime too, so non-fair children can't enter fair_class with some bogus lag they never had.

Quoted 4 lines · Onkalo Samu

When the task is sleeping rt_mutex_setprio doesn't call check_class_changed since the task is not in queue at that moment I think.

Ah, indeed, more changes then..

(I think we can also remove the prio_changed_idle thing, since I think we killed that hotplug usage)

---
Index: linux-2.6/kernel/sched.c
===================================================================

kernel/sched.c +13 -11

@@ -2025,14 +2025,14 @@ inline int task_curr(const struct task_s
 
 static inline void check_class_changed(struct rq *rq, struct task_struct *p,
 				       const struct sched_class *prev_class,
-				       int oldprio, int running)
+				       int oldprio)
 {
 	if (prev_class != p->sched_class) {
 		if (prev_class->switched_from)
-			prev_class->switched_from(rq, p, running);
-		p->sched_class->switched_to(rq, p, running);
-	} else
-		p->sched_class->prio_changed(rq, p, oldprio, running);
+			prev_class->switched_from(rq, p);
+		p->sched_class->switched_to(rq, p);
+	} else if (oldprio != p->prio)
+		p->sched_class->prio_changed(rq, p, oldprio);
 }
 
 static void check_preempt_curr(struct rq *rq, struct task_struct *p, int flags)

@@ -4570,11 +4570,10 @@ void rt_mutex_setprio(struct task_struct
 
 	if (running)
 		p->sched_class->set_curr_task(rq);
-	if (on_rq) {
+	if (on_rq)
 		enqueue_task(rq, p, oldprio < prio ? ENQUEUE_HEAD : 0);
 
-		check_class_changed(rq, p, prev_class, oldprio, running);
-	}
+	check_class_changed(rq, p, prev_class, oldprio);
 	task_rq_unlock(rq, &flags);
 }
 

@@ -4902,11 +4901,10 @@ static int __sched_setscheduler(struct t
 
 	if (running)
 		p->sched_class->set_curr_task(rq);
-	if (on_rq) {
+	if (on_rq)
 		activate_task(rq, p, 0);
 
-		check_class_changed(rq, p, prev_class, oldprio, running);
-	}
+	check_class_changed(rq, p, prev_class, oldprio);
 	__task_rq_unlock(rq);
 	raw_spin_unlock_irqrestore(&p->pi_lock, flags);
 

@@ -8109,6 +8107,8 @@ EXPORT_SYMBOL(__might_sleep);
 #ifdef CONFIG_MAGIC_SYSRQ
 static void normalize_task(struct rq *rq, struct task_struct *p)
 {
+	struct sched_class *prev_class = p->sched_class;
+	int old_prio = p->prio;
 	int on_rq;
 
 	on_rq = p->se.on_rq;

@@ -8119,6 +8119,8 @@ static void normalize_task(struct rq *rq
 		activate_task(rq, p, 0);
 		resched_task(rq->curr);
 	}
+
+	check_class_changed(rq, p, prev_class, old_prio);
 }
 
 void normalize_rt_tasks(void)

Index: linux-2.6/kernel/sched_fair.c
===================================================================

kernel/sched_fair.c +30 -6

@@ -4054,33 +4054,56 @@ static void task_fork_fair(struct task_s
  * Priority of the task has changed. Check to see if we preempt
  * the current task.
  */
-static void prio_changed_fair(struct rq *rq, struct task_struct *p,
-			      int oldprio, int running)
+static void
+prio_changed_fair(struct rq *rq, struct task_struct *p, int oldprio)
 {
+	if (!p->se.on_rq)
+		return;
+
 	/*
 	 * Reschedule if we are currently running on this runqueue and
 	 * our priority decreased, or if we are not currently running on
 	 * this runqueue and our priority is higher than the current's
 	 */
-	if (running) {
+	if (rq->curr == p) {
 		if (p->prio > oldprio)
 			resched_task(rq->curr);
 	} else
 		check_preempt_curr(rq, p, 0);
 }
 
+static void switched_from_fair(struct rq *rq, struct task_struct *p)
+{
+	struct sched_entity *se = &p->se;
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+	/*
+	 * Ensure the task's vruntime is normalized, so that when its
+	 * switched back to the fair class the enqueue_entity(.flags=0) will
+	 * do the right thing.
+	 *
+	 * If it was on_rq, then the dequeue_entity(.flags=0) will already
+	 * have normalized the vruntime, if it was !on_rq, then only when
+	 * the task is sleeping will it still have non-normalized vruntime.
+	 */
+	if (!se->on_rq && p->state != TASK_RUNNING)
+		se->vruntime -= cfs_rq->min_vruntime;
+}
+
 /*
  * We switched to the sched_fair class.
  */
-static void switched_to_fair(struct rq *rq, struct task_struct *p,
-			     int running)
+static void switched_to_fair(struct rq *rq, struct task_struct *p)
 {
+	if (!p->se.on_rq)
+		return;
+
 	/*
 	 * We were most likely switched from sched_rt, so
 	 * kick off the schedule if running, otherwise just see
 	 * if we can still preempt the current task.
 	 */
-	if (running)
+	if (rq->curr == p)
 		resched_task(rq->curr);
 	else
 		check_preempt_curr(rq, p, 0);

@@ -4166,6 +4189,7 @@ static const struct sched_class fair_sch
 	.task_fork		= task_fork_fair,
 
 	.prio_changed		= prio_changed_fair,
+	.switched_from		= switched_from_fair,
 	.switched_to		= switched_to_fair,
 
 	.get_rr_interval	= get_rr_interval_fair,

Index: linux-2.6/include/linux/sched.h
===================================================================

include/linux/sched.h +3 -5

@@ -1084,12 +1084,10 @@ struct sched_class {
 	void (*task_tick) (struct rq *rq, struct task_struct *p, int queued);
 	void (*task_fork) (struct task_struct *p);
 
-	void (*switched_from) (struct rq *this_rq, struct task_struct *task,
-			       int running);
-	void (*switched_to) (struct rq *this_rq, struct task_struct *task,
-			     int running);
+	void (*switched_from) (struct rq *this_rq, struct task_struct *task);
+	void (*switched_to) (struct rq *this_rq, struct task_struct *task);
 	void (*prio_changed) (struct rq *this_rq, struct task_struct *task,
-			     int oldprio, int running);
+			     int oldprio);
 
 	unsigned int (*get_rr_interval) (struct rq *rq,
 					 struct task_struct *task);

Index: linux-2.6/kernel/sched_idletask.c
===================================================================

kernel/sched_idletask.c +3 -4

@@ -52,8 +52,7 @@ static void set_curr_task_idle(struct rq
 {
 }
 
-static void switched_to_idle(struct rq *rq, struct task_struct *p,
-			     int running)
+static void switched_to_idle(struct rq *rq, struct task_struct *p)
 {
 	/* Can this actually happen?? */
 	if (running)

@@ -62,8 +61,8 @@ static void switched_to_idle(struct rq *
 		check_preempt_curr(rq, p, 0);
 }
 
-static void prio_changed_idle(struct rq *rq, struct task_struct *p,
-			      int oldprio, int running)
+static void
+prio_changed_idle(struct rq *rq, struct task_struct *p, int oldprio)
 {
 	/* This can happen for hot plug CPUS */
 

Index: linux-2.6/kernel/sched_rt.c
===================================================================

kernel/sched_rt.c +10 -9

@@ -1595,8 +1595,7 @@ static void rq_offline_rt(struct rq *rq)
  * When switch from the rt queue, we bring ourselves to a position
  * that we might want to pull RT tasks from other runqueues.
  */
-static void switched_from_rt(struct rq *rq, struct task_struct *p,
-			   int running)
+static void switched_from_rt(struct rq *rq, struct task_struct *p)
 {
 	/*
 	 * If there are other RT tasks then we will reschedule

@@ -1605,7 +1604,7 @@ static void switched_from_rt(struct rq *
 	 * we may need to handle the pulling of RT tasks
 	 * now.
 	 */
-	if (!rq->rt.rt_nr_running)
+	if (p->se.on_rq && !rq->rt.rt_nr_running)
 		pull_rt_task(rq);
 }
 

@@ -1624,8 +1623,7 @@ static inline void init_sched_rt_class(v
  * with RT tasks. In this case we try to push them off to
  * other runqueues.
  */
-static void switched_to_rt(struct rq *rq, struct task_struct *p,
-			   int running)
+static void switched_to_rt(struct rq *rq, struct task_struct *p)
 {
 	int check_resched = 1;
 

@@ -1636,7 +1634,7 @@ static void switched_to_rt(struct rq *rq
 	 * If that current running task is also an RT task
 	 * then see if we can move to another run queue.
 	 */
-	if (!running) {
+	if (p->se.on_rq && rq->curr != p) {
 #ifdef CONFIG_SMP
 		if (rq->rt.overloaded && push_rt_task(rq) &&
 		    /* Don't resched if we changed runqueues */

@@ -1652,10 +1650,13 @@ static void switched_to_rt(struct rq *rq
  * Priority of the task has changed. This may cause
  * us to initiate a push or pull.
  */
-static void prio_changed_rt(struct rq *rq, struct task_struct *p,
-			    int oldprio, int running)
+static void
+prio_changed_rt(struct rq *rq, struct task_struct *p, int oldprio)
 {
-	if (running) {
+	if (!p->se.on_rq)
+		return;
+
+	if (rq->curr == p) {
 #ifdef CONFIG_SMP
 		/*
 		 * If our priority decreases while running, we

Index: linux-2.6/kernel/sched_stoptask.c
===================================================================

kernel/sched_stoptask.c +3 -4

@@ -59,14 +59,13 @@ static void set_curr_task_stop(struct rq
 {
 }
 
-static void switched_to_stop(struct rq *rq, struct task_struct *p,
-			     int running)
+static void switched_to_stop(struct rq *rq, struct task_struct *p)
 {
 	BUG(); /* its impossible to change to this class */
 }
 
-static void prio_changed_stop(struct rq *rq, struct task_struct *p,
-			      int oldprio, int running)
+static void
+prio_changed_stop(struct rq *rq, struct task_struct *p, int oldprio)
 {
 	BUG(); /* how!?, what priority? */
 }

Quoted 6 lines · ext Peter Zijlstra

Quoted 4 lines · Onkalo Samu

When the task is sleeping rt_mutex_setprio doesn't call check_class_changed since the task is not in queue at that moment I think.

Quoted 4 lines

Ah, indeed, more changes then..

(I think we can also remove the prio_changed_idle thing, since I think we killed that hotplug usage)

Doesn't compile because kernel/sched_idletask.c functions still use "running" parameter which was removed.

Quoted 276 lines

---
Index: linux-2.6/kernel/sched.c
===================================================================

kernel/sched.c +13 -11

@@ -2025,14 +2025,14 @@ inline int task_curr(const struct task_s
 
 static inline void check_class_changed(struct rq *rq, struct task_struct *p,
 				       const struct sched_class *prev_class,
-				       int oldprio, int running)
+				       int oldprio)
 {
 	if (prev_class != p->sched_class) {
 		if (prev_class->switched_from)
-			prev_class->switched_from(rq, p, running);
-		p->sched_class->switched_to(rq, p, running);
-	} else
-		p->sched_class->prio_changed(rq, p, oldprio, running);
+			prev_class->switched_from(rq, p);
+		p->sched_class->switched_to(rq, p);
+	} else if (oldprio != p->prio)
+		p->sched_class->prio_changed(rq, p, oldprio);
 }
 
 static void check_preempt_curr(struct rq *rq, struct task_struct *p, int flags)

@@ -4570,11 +4570,10 @@ void rt_mutex_setprio(struct task_struct
 
 	if (running)
 		p->sched_class->set_curr_task(rq);
-	if (on_rq) {
+	if (on_rq)
 		enqueue_task(rq, p, oldprio < prio ? ENQUEUE_HEAD : 0);
 
-		check_class_changed(rq, p, prev_class, oldprio, running);
-	}
+	check_class_changed(rq, p, prev_class, oldprio);
 	task_rq_unlock(rq, &flags);
 }
 

@@ -4902,11 +4901,10 @@ static int __sched_setscheduler(struct t
 
 	if (running)
 		p->sched_class->set_curr_task(rq);
-	if (on_rq) {
+	if (on_rq)
 		activate_task(rq, p, 0);
 
-		check_class_changed(rq, p, prev_class, oldprio, running);
-	}
+	check_class_changed(rq, p, prev_class, oldprio);
 	__task_rq_unlock(rq);
 	raw_spin_unlock_irqrestore(&p->pi_lock, flags);
 

@@ -8109,6 +8107,8 @@ EXPORT_SYMBOL(__might_sleep);
 #ifdef CONFIG_MAGIC_SYSRQ
 static void normalize_task(struct rq *rq, struct task_struct *p)
 {
+	struct sched_class *prev_class = p->sched_class;
+	int old_prio = p->prio;
 	int on_rq;
 
 	on_rq = p->se.on_rq;

@@ -8119,6 +8119,8 @@ static void normalize_task(struct rq *rq
 		activate_task(rq, p, 0);
 		resched_task(rq->curr);
 	}
+
+	check_class_changed(rq, p, prev_class, old_prio);
 }
 
 void normalize_rt_tasks(void)

Index: linux-2.6/kernel/sched_fair.c
===================================================================

kernel/sched_fair.c +30 -6

@@ -4054,33 +4054,56 @@ static void task_fork_fair(struct task_s
  * Priority of the task has changed. Check to see if we preempt
  * the current task.
  */
-static void prio_changed_fair(struct rq *rq, struct task_struct *p,
-			      int oldprio, int running)
+static void
+prio_changed_fair(struct rq *rq, struct task_struct *p, int oldprio)
 {
+	if (!p->se.on_rq)
+		return;
+
 	/*
 	 * Reschedule if we are currently running on this runqueue and
 	 * our priority decreased, or if we are not currently running on
 	 * this runqueue and our priority is higher than the current's
 	 */
-	if (running) {
+	if (rq->curr == p) {
 		if (p->prio > oldprio)
 			resched_task(rq->curr);
 	} else
 		check_preempt_curr(rq, p, 0);
 }
 
+static void switched_from_fair(struct rq *rq, struct task_struct *p)
+{
+	struct sched_entity *se = &p->se;
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+	/*
+	 * Ensure the task's vruntime is normalized, so that when its
+	 * switched back to the fair class the enqueue_entity(.flags=0) will
+	 * do the right thing.
+	 *
+	 * If it was on_rq, then the dequeue_entity(.flags=0) will already
+	 * have normalized the vruntime, if it was !on_rq, then only when
+	 * the task is sleeping will it still have non-normalized vruntime.
+	 */
+	if (!se->on_rq && p->state != TASK_RUNNING)
+		se->vruntime -= cfs_rq->min_vruntime;
+}
+
 /*
  * We switched to the sched_fair class.
  */
-static void switched_to_fair(struct rq *rq, struct task_struct *p,
-			     int running)
+static void switched_to_fair(struct rq *rq, struct task_struct *p)
 {
+	if (!p->se.on_rq)
+		return;
+
 	/*
 	 * We were most likely switched from sched_rt, so
 	 * kick off the schedule if running, otherwise just see
 	 * if we can still preempt the current task.
 	 */
-	if (running)
+	if (rq->curr == p)
 		resched_task(rq->curr);
 	else
 		check_preempt_curr(rq, p, 0);

@@ -4166,6 +4189,7 @@ static const struct sched_class fair_sch
 	.task_fork		= task_fork_fair,
 
 	.prio_changed		= prio_changed_fair,
+	.switched_from		= switched_from_fair,
 	.switched_to		= switched_to_fair,
 
 	.get_rr_interval	= get_rr_interval_fair,

Index: linux-2.6/include/linux/sched.h
===================================================================

include/linux/sched.h +3 -5

@@ -1084,12 +1084,10 @@ struct sched_class {
 	void (*task_tick) (struct rq *rq, struct task_struct *p, int queued);
 	void (*task_fork) (struct task_struct *p);
 
-	void (*switched_from) (struct rq *this_rq, struct task_struct *task,
-			       int running);
-	void (*switched_to) (struct rq *this_rq, struct task_struct *task,
-			     int running);
+	void (*switched_from) (struct rq *this_rq, struct task_struct *task);
+	void (*switched_to) (struct rq *this_rq, struct task_struct *task);
 	void (*prio_changed) (struct rq *this_rq, struct task_struct *task,
-			     int oldprio, int running);
+			     int oldprio);
 
 	unsigned int (*get_rr_interval) (struct rq *rq,
 					 struct task_struct *task);

Index: linux-2.6/kernel/sched_idletask.c
===================================================================

kernel/sched_idletask.c +3 -4

@@ -52,8 +52,7 @@ static void set_curr_task_idle(struct rq
 {
 }
 
-static void switched_to_idle(struct rq *rq, struct task_struct *p,
-			     int running)
+static void switched_to_idle(struct rq *rq, struct task_struct *p)
 {
 	/* Can this actually happen?? */
 	if (running)

@@ -62,8 +61,8 @@ static void switched_to_idle(struct rq *
 		check_preempt_curr(rq, p, 0);
 }
 
-static void prio_changed_idle(struct rq *rq, struct task_struct *p,
-			      int oldprio, int running)
+static void
+prio_changed_idle(struct rq *rq, struct task_struct *p, int oldprio)
 {
 	/* This can happen for hot plug CPUS */
 

Index: linux-2.6/kernel/sched_rt.c
===================================================================

kernel/sched_rt.c +10 -9

@@ -1595,8 +1595,7 @@ static void rq_offline_rt(struct rq *rq)
  * When switch from the rt queue, we bring ourselves to a position
  * that we might want to pull RT tasks from other runqueues.
  */
-static void switched_from_rt(struct rq *rq, struct task_struct *p,
-			   int running)
+static void switched_from_rt(struct rq *rq, struct task_struct *p)
 {
 	/*
 	 * If there are other RT tasks then we will reschedule

@@ -1605,7 +1604,7 @@ static void switched_from_rt(struct rq *
 	 * we may need to handle the pulling of RT tasks
 	 * now.
 	 */
-	if (!rq->rt.rt_nr_running)
+	if (p->se.on_rq && !rq->rt.rt_nr_running)
 		pull_rt_task(rq);
 }
 

@@ -1624,8 +1623,7 @@ static inline void init_sched_rt_class(v
  * with RT tasks. In this case we try to push them off to
  * other runqueues.
  */
-static void switched_to_rt(struct rq *rq, struct task_struct *p,
-			   int running)
+static void switched_to_rt(struct rq *rq, struct task_struct *p)
 {
 	int check_resched = 1;
 

@@ -1636,7 +1634,7 @@ static void switched_to_rt(struct rq *rq
 	 * If that current running task is also an RT task
 	 * then see if we can move to another run queue.
 	 */
-	if (!running) {
+	if (p->se.on_rq && rq->curr != p) {
 #ifdef CONFIG_SMP
 		if (rq->rt.overloaded && push_rt_task(rq) &&
 		    /* Don't resched if we changed runqueues */

@@ -1652,10 +1650,13 @@ static void switched_to_rt(struct rq *rq
  * Priority of the task has changed. This may cause
  * us to initiate a push or pull.
  */
-static void prio_changed_rt(struct rq *rq, struct task_struct *p,
-			    int oldprio, int running)
+static void
+prio_changed_rt(struct rq *rq, struct task_struct *p, int oldprio)
 {
-	if (running) {
+	if (!p->se.on_rq)
+		return;
+
+	if (rq->curr == p) {
 #ifdef CONFIG_SMP
 		/*
 		 * If our priority decreases while running, we

Index: linux-2.6/kernel/sched_stoptask.c
===================================================================

kernel/sched_stoptask.c +3 -4

@@ -59,14 +59,13 @@ static void set_curr_task_stop(struct rq
 {
 }
 
-static void switched_to_stop(struct rq *rq, struct task_struct *p,
-			     int running)
+static void switched_to_stop(struct rq *rq, struct task_struct *p)
 {
 	BUG(); /* its impossible to change to this class */
 }
 
-static void prio_changed_stop(struct rq *rq, struct task_struct *p,
-			      int oldprio, int running)
+static void
+prio_changed_stop(struct rq *rq, struct task_struct *p, int oldprio)
 {
 	BUG(); /* how!?, what priority? */
 }

more or less, the idea is that we only call __{dequeue,enqueue}_entity() when the task is actually in the tree and current is not.