[PATCH] sched: ensure a task has a non-normalized vruntime when returning back to cfs

[PATCH] sched: ensure a task has a non-normalized vruntime when returning back to cfs

[byungchul.park]

From: Byungchul Park <byungchul.park@lge.com>

current code ensures a task has a normalized vruntime when switching off
from fair class, but it does not ensure the task has a non-normalized
vruntime when switching back to the fair class.

this is an example breaking this consistency.

1. a task is in fair class and !queue
2. change its class to rt class (still !queue)
3. change its class to fair class again (stll !queue)

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 kernel/sched/fair.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ffa70dc..df265f5 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8150,8 +8150,22 @@ static void switched_to_fair(struct rq *rq, struct task_struct *p)
 	 */
 	se->depth = se->parent ? se->parent->depth + 1 : 0;
 #endif
-	if (!task_on_rq_queued(p))
+
+	if (!task_on_rq_queued(p)) {
+
+		/*
+		 * Ensure the task has a non-normalized vruntime when it is switched
+		 * back to the fair class with !queued, so that enqueue_entity() at
+		 * wake-up time will do the right thing.
+		 *
+		 * If it's queued, then the enqueue_entity(.flags=0) makes the task
+		 * has non-normalized vruntime, if it's !queued, then it still has
+		 * normalized vruntime.
+		 */
+		if (p->state != TASK_RUNNING)
+			se->vruntime += cfs_rq_of(se)->min_vruntime;
 		return;
+	}
 
 	/*
 	 * We were most likely switched from sched_rt, so
-- 
1.7.9.5

Re: [PATCH] sched: ensure a task has a non-normalized vruntime when returning back to cfs

[Peter Zijlstra]

On Mon, Aug 10, 2015 at 06:02:55PM +0900, byungchul.park@lge.com wrote:
> From: Byungchul Park <byungchul.park@lge.com>
> 
> current code ensures a task has a normalized vruntime when switching off
> from fair class, but it does not ensure the task has a non-normalized
> vruntime when switching back to the fair class.
> 
> this is an example breaking this consistency.
> 
> 1. a task is in fair class and !queue
> 2. change its class to rt class (still !queue)
> 3. change its class to fair class again (stll !queue)

Just curious, did you manage to trigger this in practise or did you find
it through code inspection only?

Re: [PATCH] sched: ensure a task has a non-normalized vruntime when returning back to cfs

[Byungchul Park]

On Mon, Aug 10, 2015 at 02:14:07PM +0200, Peter Zijlstra wrote:
> On Mon, Aug 10, 2015 at 06:02:55PM +0900, byungchul.park@lge.com wrote:
> > From: Byungchul Park <byungchul.park@lge.com>
> > 
> > current code ensures a task has a normalized vruntime when switching off
> > from fair class, but it does not ensure the task has a non-normalized
> > vruntime when switching back to the fair class.
> > 
> > this is an example breaking this consistency.
> > 
> > 1. a task is in fair class and !queue
> > 2. change its class to rt class (still !queue)
> > 3. change its class to fair class again (stll !queue)
> 
> Just curious, did you manage to trigger this in practise or did you find
> it through code inspection only?

hello,

not only through code inspection, but also checked it through gdb debugger.
when returning back to fair class (step 3), vruntime was still normalized.
more exactly, it has a very large value which means negative value.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] sched: ensure a task has a non-normalized vruntime when returning back to cfs

[Byungchul Park]

On Tue, Aug 11, 2015 at 08:35:29AM +0900, Byungchul Park wrote:
> On Mon, Aug 10, 2015 at 02:14:07PM +0200, Peter Zijlstra wrote:
> > On Mon, Aug 10, 2015 at 06:02:55PM +0900, byungchul.park@lge.com wrote:
> > > From: Byungchul Park <byungchul.park@lge.com>
> > > 
> > > current code ensures a task has a normalized vruntime when switching off
> > > from fair class, but it does not ensure the task has a non-normalized
> > > vruntime when switching back to the fair class.
> > > 
> > > this is an example breaking this consistency.
> > > 
> > > 1. a task is in fair class and !queue
> > > 2. change its class to rt class (still !queue)
> > > 3. change its class to fair class again (stll !queue)
> > 
> > Just curious, did you manage to trigger this in practise or did you find
> > it through code inspection only?

i found it through code inspection, and then i checked it with debugger.

> 
> hello,
> 
> not only through code inspection, but also checked it through gdb debugger.
> when returning back to fair class (step 3), vruntime was still normalized.
> more exactly, it has a very large value which means negative value.
> 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[tip:sched/core] sched: Ensure a task has a non-normalized vruntime when returning back to CFS

[tip-bot for Byungchul Park]

Commit-ID:  7855a35ac07a350e2cd26f09568a6d8e372be358
Gitweb:     http://git.kernel.org/tip/7855a35ac07a350e2cd26f09568a6d8e372be358
Author:     Byungchul Park <byungchul.park@lge.com>
AuthorDate: Mon, 10 Aug 2015 18:02:55 +0900
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 12 Aug 2015 12:06:09 +0200

sched: Ensure a task has a non-normalized vruntime when returning back to CFS

Current code ensures that a task has a normalized vruntime when switching away
from the fair class, but it does not ensure the task has a non-normalized
vruntime when switching back to the fair class.

This is an example breaking this consistency:

  1. a task is in fair class and !queued
  2. changes its class to RT class (still !queued)
  3. changes its class to fair class again (still !queued)

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1439197375-27927-1-git-send-email-byungchul.park@lge.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/fair.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 858b94a..f0950fd 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7930,16 +7930,31 @@ static void switched_from_fair(struct rq *rq, struct task_struct *p)
  */
 static void switched_to_fair(struct rq *rq, struct task_struct *p)
 {
-#ifdef CONFIG_FAIR_GROUP_SCHED
 	struct sched_entity *se = &p->se;
+
+#ifdef CONFIG_FAIR_GROUP_SCHED
 	/*
 	 * Since the real-depth could have been changed (only FAIR
 	 * class maintain depth value), reset depth properly.
 	 */
 	se->depth = se->parent ? se->parent->depth + 1 : 0;
 #endif
-	if (!task_on_rq_queued(p))
+
+	if (!task_on_rq_queued(p)) {
+
+		/*
+		 * Ensure the task has a non-normalized vruntime when it is switched
+		 * back to the fair class with !queued, so that enqueue_entity() at
+		 * wake-up time will do the right thing.
+		 *
+		 * If it's queued, then the enqueue_entity(.flags=0) makes the task
+		 * has non-normalized vruntime, if it's !queued, then it still has
+		 * normalized vruntime.
+		 */
+		if (p->state != TASK_RUNNING)
+			se->vruntime += cfs_rq_of(se)->min_vruntime;
 		return;
+	}
 
 	/*
 	 * We were most likely switched from sched_rt, so