Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1398366 > unrolled thread

[PATCH 0/3] sched: Fix wakeup preemption regression

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] sched: Fix wakeup preemption regression Peter Zijlstra <peterz@infradead.org> - 2016-05-10 19:50 +0200
    [PATCH 3/3] sched: Kill sched_class::task_waking Peter Zijlstra <peterz@infradead.org> - 2016-05-10 20:00 +0200
    Re: [PATCH 0/3] sched: Fix wakeup preemption regression Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-05-11 08:00 +0200
    Re: [PATCH 0/3] sched: Fix wakeup preemption regression Pavan Kondeti <pkondeti@codeaurora.org> - 2016-05-12 12:00 +0200
    Re: [PATCH 0/3] sched: Fix wakeup preemption regression Matt Fleming <matt@codeblueprint.co.uk> - 2016-05-12 13:00 +0200

#1398366 — [PATCH 0/3] sched: Fix wakeup preemption regression

A recent commit caused an interactivity/starvation issue because we wrecked rq
local wakeup preemption.

These patches rectify this while also (hopefully) keeping the problem that led
to the fault patch fixed.

Mike, Pavan, could you guys please confirm?

[toc] | [next] | [standalone]

#1398368 — [PATCH 3/3] sched: Kill sched_class::task_waking

With sched_class::task_waking being called only when we do
set_task_cpu(), we can make sched_class::migrate_task_rq() do the work
and eliminate sched_class::task_waking entirely.

Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Ben Segall <bsegall@google.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Mike Galbraith <umgwanakikbuti@gmail.com>
Cc: Morten Rasmussen <morten.rasmussen@arm.com>
Cc: Paul Turner <pjt@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: byungchul.park@lge.com
Cc: Andrew Hunter <ahh@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c  |    9 -------
 kernel/sched/fair.c  |   58 ++++++++++++++++++++++++---------------------------
 kernel/sched/sched.h |    7 ++----
 3 files changed, 32 insertions(+), 42 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1717,11 +1717,8 @@ ttwu_do_activate(struct rq *rq, struct t
 	if (p->sched_contributes_to_load)
 		rq->nr_uninterruptible--;
 
-	/*
-	 * If we migrated; we must have called sched_class::task_waking().
-	 */
 	if (wake_flags & WF_MIGRATED)
-		en_flags |= ENQUEUE_WAKING;
+		en_flags |= ENQUEUE_MIGRATED;
 #endif
 
 	ttwu_activate(rq, p, en_flags);
@@ -2049,10 +2046,6 @@ try_to_wake_up(struct task_struct *p, un
 	cpu = select_task_rq(p, p->wake_cpu, SD_BALANCE_WAKE, wake_flags);
 	if (task_cpu(p) != cpu) {
 		wake_flags |= WF_MIGRATED;
-
-		if (p->sched_class->task_waking)
-			p->sched_class->task_waking(p);
-
 		set_task_cpu(p, cpu);
 	}
 #endif /* CONFIG_SMP */
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3282,7 +3282,7 @@ static inline void check_schedstat_requi
  *
  * WAKEUP (remote)
  *
- *	->task_waking_fair()
+ *	->migrate_task_rq_fair() (p->state == TASK_WAKING)
  *	  vruntime -= min_vruntime
  *
  *	enqueue
@@ -3297,7 +3297,7 @@ static inline void check_schedstat_requi
 static void
 enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
-	bool renorm = !(flags & ENQUEUE_WAKEUP) || (flags & ENQUEUE_WAKING);
+	bool renorm = !(flags & ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATED);
 	bool curr = cfs_rq->curr == se;
 
 	/*
@@ -4865,33 +4865,6 @@ static unsigned long cpu_avg_load_per_ta
 	return 0;
 }
 
-/*
- * Called to migrate a waking task; as blocked tasks retain absolute vruntime
- * the migration needs to deal with this by subtracting the old and adding the
- * new min_vruntime -- the latter is done by enqueue_entity() when placing
- * the task on the new runqueue.
- */
-static void task_waking_fair(struct task_struct *p)
-{
-	struct sched_entity *se = &p->se;
-	struct cfs_rq *cfs_rq = cfs_rq_of(se);
-	u64 min_vruntime;
-
-#ifndef CONFIG_64BIT
-	u64 min_vruntime_copy;
-
-	do {
-		min_vruntime_copy = cfs_rq->min_vruntime_copy;
-		smp_rmb();
-		min_vruntime = cfs_rq->min_vruntime;
-	} while (min_vruntime != min_vruntime_copy);
-#else
-	min_vruntime = cfs_rq->min_vruntime;
-#endif
-
-	se->vruntime -= min_vruntime;
-}
-
 #ifdef CONFIG_FAIR_GROUP_SCHED
 /*
  * effective_load() calculates the load change as seen from the root_task_group
@@ -5648,6 +5621,32 @@ select_task_rq_fair(struct task_struct *
 static void migrate_task_rq_fair(struct task_struct *p)
 {
 	/*
+	 * As blocked tasks retain absolute vruntime the migration needs to
+	 * deal with this by subtracting the old and adding the new
+	 * min_vruntime -- the latter is done by enqueue_entity() when placing
+	 * the task on the new runqueue.
+	 */
+	if (p->state == TASK_WAKING) {
+		struct sched_entity *se = &p->se;
+		struct cfs_rq *cfs_rq = cfs_rq_of(se);
+		u64 min_vruntime;
+
+#ifndef CONFIG_64BIT
+		u64 min_vruntime_copy;
+
+		do {
+			min_vruntime_copy = cfs_rq->min_vruntime_copy;
+			smp_rmb();
+			min_vruntime = cfs_rq->min_vruntime;
+		} while (min_vruntime != min_vruntime_copy);
+#else
+		min_vruntime = cfs_rq->min_vruntime;
+#endif
+
+		se->vruntime -= min_vruntime;
+	}
+
+	/*
 	 * We are supposed to update the task to "current" time, then its up to date
 	 * and ready to go to new CPU/cfs_rq. But we have difficulty in getting
 	 * what current time is, so simply throw away the out-of-date time. This
@@ -8916,7 +8915,6 @@ const struct sched_class fair_sched_clas
 	.rq_online		= rq_online_fair,
 	.rq_offline		= rq_offline_fair,
 
-	.task_waking		= task_waking_fair,
 	.task_dead		= task_dead_fair,
 	.set_cpus_allowed	= set_cpus_allowed_common,
 #endif
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1164,7 +1164,7 @@ extern const u32 sched_prio_to_wmult[40]
  *
  * ENQUEUE_HEAD      - place at front of runqueue (tail if not specified)
  * ENQUEUE_REPLENISH - CBS (replenish runtime and postpone deadline)
- * ENQUEUE_WAKING    - sched_class::task_waking was called
+ * ENQUEUE_MIGRATED  - the task was migrated during wakeup
  *
  */
 
@@ -1179,9 +1179,9 @@ extern const u32 sched_prio_to_wmult[40]
 #define ENQUEUE_HEAD		0x08
 #define ENQUEUE_REPLENISH	0x10
 #ifdef CONFIG_SMP
-#define ENQUEUE_WAKING		0x20
+#define ENQUEUE_MIGRATED	0x20
 #else
-#define ENQUEUE_WAKING		0x00
+#define ENQUEUE_MIGRATED	0x00
 #endif
 
 #define RETRY_TASK		((void *)-1UL)
@@ -1213,7 +1213,6 @@ struct sched_class {
 	int  (*select_task_rq)(struct task_struct *p, int task_cpu, int sd_flag, int flags);
 	void (*migrate_task_rq)(struct task_struct *p);
 
-	void (*task_waking) (struct task_struct *task);
 	void (*task_woken) (struct rq *this_rq, struct task_struct *task);
 
 	void (*set_cpus_allowed)(struct task_struct *p,

[toc] | [prev] | [next] | [standalone]

#1398699

On Tue, 2016-05-10 at 19:43 +0200, Peter Zijlstra wrote:

> Mike, Pavan, could you guys please confirm?

Plugging the series into master.today, all seems peachy on my end.

	-Mike

[toc] | [prev] | [next] | [standalone]

#1399849

Hi Peter,

On Tue, May 10, 2016 at 11:13 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> A recent commit caused an interactivity/starvation issue because we wrecked rq
> local wakeup preemption.
>
> These patches rectify this while also (hopefully) keeping the problem that led
> to the fault patch fixed.
>
> Mike, Pavan, could you guys please confirm?
>
>

I tested with your latest patches. The original problem (migrated
task's vruntime falling
beyond the min_vruntime) is not reproducible.

Thanks,
Pavan

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1399915

On Tue, 10 May, at 07:43:14PM, Peter Zijlstra wrote:
> A recent commit caused an interactivity/starvation issue because we wrecked rq
> local wakeup preemption.
> 
> These patches rectify this while also (hopefully) keeping the problem that led
> to the fault patch fixed.
> 
> Mike, Pavan, could you guys please confirm?
 
FWIW, I took a quick look over these patches and they made sense to
me. (I appreciate the comment block above enqueue_entity())

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web