Skip to contentSkip to conversation

sched/fair: fix vruntime_normalized for remote non-migration wakeup

PATCH In review Steve Muckle 3 participants 3 messages Aug 31, 2018 – Sep 10, 2018 Scheduler

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sched/fair: fix vruntime_normalized for remote non-migration wakeup

When a task which previously ran on a given CPU is remotely queued to wake up on that same CPU, there is a period where the task's state is TASK_WAKING and its vruntime is not normalized. This is not accounted for in vruntime_normalized() which will cause an
error in the task's vruntime if it is switched from the fair class during this time, for example if it is boosted to RT priority via rt_mutex_setprio. The rq's min_vruntime will not be subtracted from the task's vruntime but it will be added again when the task returns to the fair class. The task's vruntime will have been erroneously doubled and the effective priority of the task will be reduced.

Note this will also lead to inflation of all vruntimes since the doubled vruntime value will become the rq's min_vruntime when other tasks leave the rq. This leads to repeated doubling of the vruntime and priority penalty.

Fix this by recognizing a WAKING task's vruntime as normalized only if sched_remote_wakeup is true. This indicates a migration, in which case the vruntime would have been normalized in migrate_task_rq_fair().

Based on a similar patch from [email protected].

---
 kernel/sched/fair.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/sched/fair.c +2 -1
index b39fb596f6c1..b3b62cf37fb6 100644
@@ -9638,7 +9638,8 @@ static inline bool vruntime_normalized(struct task_struct *p)
 	 * - A task which has been woken up by try_to_wake_up() and
 	 *   waiting for actually being woken up by sched_ttwu_pending().
 	 */
-	if (!se->sum_exec_runtime || p->state == TASK_WAKING)
+	if (!se->sum_exec_runtime ||
+	    (p->state == TASK_WAKING && p->sched_remote_wakeup))
 		return true;
 
 	return false;
Signature · 1 line
2.19.0.rc1.350.ge57e33dbd1-goog
reviewer Tested-by on v1 1/1
#1 lore raw
Quoted 34 lines · Steve Muckle

When a task which previously ran on a given CPU is remotely queued to wake up on that same CPU, there is a period where the task's state is TASK_WAKING and its vruntime is not normalized. This is not accounted for in vruntime_normalized() which will cause an
error in the task's vruntime if it is switched from the fair class during this time, for example if it is boosted to RT priority via rt_mutex_setprio. The rq's min_vruntime will not be subtracted from the task's vruntime but it will be added again when the task returns to the fair class. The task's vruntime will have been erroneously doubled and the effective priority of the task will be reduced.

Note this will also lead to inflation of all vruntimes since the doubled vruntime value will become the rq's min_vruntime when other tasks leave the rq. This leads to repeated doubling of the vruntime and priority penalty.

Fix this by recognizing a WAKING task's vruntime as normalized only if sched_remote_wakeup is true. This indicates a migration, in which case the vruntime would have been normalized in migrate_task_rq_fair().

Based on a similar patch from [email protected].

---
  kernel/sched/fair.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
kernel/sched/fair.c +0 -0
index b39fb596f6c1..b3b62cf37fb6 100644

kernel/sched/fair.c static inline bool vruntime_normalized(struct task_struct *p) · line 9645

Quoted 9 lines
kernel/sched/fair.c +2 -1
@@ -9638,7 +9638,8 @@ static inline bool vruntime_normalized(struct task_struct *p)
  	 * - A task which has been woken up by try_to_wake_up() and
  	 *   waiting for actually being woken up by sched_ttwu_pending().
  	 */
-	if (!se->sum_exec_runtime || p->state == TASK_WAKING)
+	if (!se->sum_exec_runtime ||
+	    (p->state == TASK_WAKING && p->sched_remote_wakeup))
  		return true;
 
  	return false;
#2 lore raw

[tip:sched/core] sched/fair: Fix vruntime_normalized() for remote non-migration wakeup

In reply to Steve Muckle
sched/fair: Fix vruntime_normalized() for remote non-migration wakeup
Commit-ID:  d0cdb3ce8834332d918fc9c8ff74f8a169ec9abe
Gitweb:     https://git.kernel.org/tip/d0cdb3ce8834332d918fc9c8ff74f8a169ec9abe
Author:     Steve Muckle <[email protected]>
AuthorDate: Fri, 31 Aug 2018 15:42:17 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 10 Sep 2018 10:13:47 +0200

sched/fair: Fix vruntime_normalized() for remote non-migration wakeup

When a task which previously ran on a given CPU is remotely queued to wake up on that same CPU, there is a period where the task's state is TASK_WAKING and its vruntime is not normalized. This is not accounted for in vruntime_normalized() which will cause an error in the task's vruntime if it is switched from the fair class during this time.

For example if it is boosted to RT priority via rt_mutex_setprio(), rq->min_vruntime will not be subtracted from the task's vruntime but it will be added again when the task returns to the fair class. The task's vruntime will have been erroneously doubled and the effective priority of the task will be reduced.

Note this will also lead to inflation of all vruntimes since the doubled vruntime value will become the rq's min_vruntime when other tasks leave the rq. This leads to repeated doubling of the vruntime and priority penalty.

Fix this by recognizing a WAKING task's vruntime as normalized only if sched_remote_wakeup is true. This indicates a migration, in which case the vruntime would have been normalized in migrate_task_rq_fair().

Based on a similar patch from John Dias <[email protected]>.

Cc Chris Redpath <[email protected]>
Cc John Dias <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Miguel de Dios <[email protected]>
Cc Morten Rasmussen <[email protected]>
Cc Patrick Bellasi <[email protected]>
Cc Paul Turner <[email protected]>
Cc Quentin Perret <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Todd Kjos <[email protected]>
Fixes b5179ac70de8 ("sched/fair: Prepare to fix fairness problems on migration")
---
 kernel/sched/fair.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/sched/fair.c +2 -1
index 8cff8d55ee95..c6b7d6daab20 100644
@@ -9644,7 +9644,8 @@ static inline bool vruntime_normalized(struct task_struct *p)
 	 * - A task which has been woken up by try_to_wake_up() and
 	 *   waiting for actually being woken up by sched_ttwu_pending().
 	 */
-	if (!se->sum_exec_runtime || p->state == TASK_WAKING)
+	if (!se->sum_exec_runtime ||
+	    (p->state == TASK_WAKING && p->sched_remote_wakeup))
 		return true;
 
 	return false;