linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues
@ 2019-07-22 17:33 Rik van Riel
  2019-07-22 17:33 ` [PATCH 01/14] sched: introduce task_se_h_load helper Rik van Riel
                   ` (13 more replies)
  0 siblings, 14 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

The current implementation of the CPU controller uses hierarchical
runqueues, where on wakeup a task is enqueued on its group's runqueue,
the group is enqueued on the runqueue of the group above it, etc.

This increases a fairly large amount of overhead for workloads that
do a lot of wakeups a second, especially given that the default systemd
hierarchy is 2 or 3 levels deep.

This patch series is an attempt at reducing that overhead, by placing
all the tasks on the same runqueue, and scaling the task priority by
the priority of the group, which is calculated periodically.

My main TODO items for the next period of time are likely going to
be testing, testing, and testing. I hope to find and flush out any
corner case I can find, and make sure performance does not regress
with any workloads, and hopefully improves some.

Other TODO items:
- More code cleanups.
- Remove some more now unused code.
- Reimplement CONFIG_CFS_BANDWIDTH.

Plan for the CONFIG_CFS_BANDWIDTH reimplementation:
- When a cgroup gets throttled, mark the cgroup and its children
  as throttled.
- When pick_next_entity finds a task that is on a throttled cgroup,
  stash it on the cgroup runqueue (which is not used for runnable
  tasks any more). Leave the vruntime unchanged, and adjust that
  runqueue's vruntime to be that of the left-most task.
- When a cgroup gets unthrottled, and has tasks on it, place it on
  a vruntime ordered heap separate from the main runqueue.
- Have pick_next_task_fair grab one task off that heap every time it
  is called, and the min vruntime of that heap is lower than the
  vruntime of the CPU's cfs_rq (or the CPU has no other runnable tasks).
- Place that selected task on the CPU's cfs_rq, renormalizing its
  vruntime with the GENTLE_FAIR_SLEEPERS logic. That should help
  interleave the already runnable tasks with the recently unthrottled
  group, and prevent thundering herd issues.
- If the group gets throttled again before all of its task had a chance
  to run, vruntime sorting ensures all the tasks in the throttled cgroup
  get a chance to run over time.


Changes from v2:
- fixed the web server performance regression, in a way vaguely similar
  to what Josef Bacik suggested (blame me for the implementation)
- removed some code duplication so the diffstat is redder than before
- propagate sum_exec_runtime up the tree, in preparation for CFS_BANDWIDTH
- small cleanups left and right

Changes from v1:
- use task_se_h_weight instead of task_se_h_load in calc_delta_fair
  and sched_slice, this seems to improve performance a little, but
  I still have some remaining regression to chase with our web server
  workload
- implement a number of the changes suggested by Dietmar Eggemann
  (still holding out for a better name for group_cfs_rq_of_parent)

This series applies on top of 5.2

 include/linux/sched.h |    7 
 kernel/sched/core.c   |    3 
 kernel/sched/debug.c  |   17 -
 kernel/sched/fair.c   |  780 ++++++++++++++++++++------------------------------
 kernel/sched/pelt.c   |   69 ++--
 kernel/sched/pelt.h   |    3 
 kernel/sched/sched.h  |   11 
 7 files changed, 372 insertions(+), 518 deletions(-)



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 01/14] sched: introduce task_se_h_load helper
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-08-12 17:40   ` Dietmar Eggemann
  2019-07-22 17:33 ` [PATCH 02/14] sched: change /proc/sched_debug fields Rik van Riel
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel,
	Josef Bacik

Sometimes the hierarchical load of a sched_entity needs to be calculated.
Rename task_h_load to task_se_h_load, and directly pass a sched_entity to
that function.

Move the function declaration up above where it will be used later.

No functional changes.

Signed-off-by: Rik van Riel <riel@surriel.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel/sched/fair.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index f35930f5e528..eadf9a96b3e1 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -242,6 +242,7 @@ static u64 __calc_delta(u64 delta_exec, unsigned long weight, struct load_weight
 
 
 const struct sched_class fair_sched_class;
+static unsigned long task_se_h_load(struct sched_entity *se);
 
 /**************************************************************
  * CFS operations on generic schedulable entities:
@@ -706,7 +707,6 @@ static u64 sched_vslice(struct cfs_rq *cfs_rq, struct sched_entity *se)
 #ifdef CONFIG_SMP
 
 static int select_idle_sibling(struct task_struct *p, int prev_cpu, int cpu);
-static unsigned long task_h_load(struct task_struct *p);
 static unsigned long capacity_of(int cpu);
 
 /* Give new sched_entity start runnable values to heavy its load in infant time */
@@ -1668,7 +1668,7 @@ static void task_numa_compare(struct task_numa_env *env,
 	/*
 	 * In the overloaded case, try and keep the load balanced.
 	 */
-	load = task_h_load(env->p) - task_h_load(cur);
+	load = task_se_h_load(env->p->se) - task_se_h_load(cur->se);
 	if (!load)
 		goto assign;
 
@@ -1706,7 +1706,7 @@ static void task_numa_find_cpu(struct task_numa_env *env,
 	bool maymove = false;
 	int cpu;
 
-	load = task_h_load(env->p);
+	load = task_se_h_load(env->p->se);
 	dst_load = env->dst_stats.load + load;
 	src_load = env->src_stats.load - load;
 
@@ -3389,7 +3389,7 @@ static inline void add_tg_cfs_propagate(struct cfs_rq *cfs_rq, long runnable_sum
  * avg. The immediate corollary is that all (fair) tasks must be attached, see
  * post_init_entity_util_avg().
  *
- * cfs_rq->avg is used for task_h_load() and update_cfs_share() for example.
+ * cfs_rq->avg is used for task_se_h_load() and update_cfs_share() for example.
  *
  * Returns true if the load decayed or we removed load.
  *
@@ -3522,7 +3522,7 @@ static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 
 	/*
 	 * Track task load average for carrying it to new CPU after migrated, and
-	 * track group sched_entity load average for task_h_load calc in migration
+	 * track group sched_entity load average for task_se_h_load calc in migration
 	 */
 	if (se->avg.last_update_time && !(flags & SKIP_AGE_LOAD))
 		__update_load_avg_se(now, cfs_rq, se);
@@ -3751,7 +3751,7 @@ static inline void update_misfit_status(struct task_struct *p, struct rq *rq)
 		return;
 	}
 
-	rq->misfit_task_load = task_h_load(p);
+	rq->misfit_task_load = task_se_h_load(&p->se);
 }
 
 #else /* CONFIG_SMP */
@@ -5739,7 +5739,7 @@ wake_affine_weight(struct sched_domain *sd, struct task_struct *p,
 	this_eff_load = target_load(this_cpu, sd->wake_idx);
 
 	if (sync) {
-		unsigned long current_load = task_h_load(current);
+		unsigned long current_load = task_se_h_load(&current->se);
 
 		if (current_load > this_eff_load)
 			return this_cpu;
@@ -5747,7 +5747,7 @@ wake_affine_weight(struct sched_domain *sd, struct task_struct *p,
 		this_eff_load -= current_load;
 	}
 
-	task_load = task_h_load(p);
+	task_load = task_se_h_load(&p->se);
 
 	this_eff_load += task_load;
 	if (sched_feat(WA_BIAS))
@@ -7600,7 +7600,7 @@ static int detach_tasks(struct lb_env *env)
 		if (!can_migrate_task(p, env))
 			goto next;
 
-		load = task_h_load(p);
+		load = task_se_h_load(&p->se);
 
 		if (sched_feat(LB_MIN) && load < 16 && !env->sd->nr_balance_failed)
 			goto next;
@@ -7833,12 +7833,12 @@ static void update_cfs_rq_h_load(struct cfs_rq *cfs_rq)
 	}
 }
 
-static unsigned long task_h_load(struct task_struct *p)
+static unsigned long task_se_h_load(struct sched_entity *se)
 {
-	struct cfs_rq *cfs_rq = task_cfs_rq(p);
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
 
 	update_cfs_rq_h_load(cfs_rq);
-	return div64_ul(p->se.avg.load_avg * cfs_rq->h_load,
+	return div64_ul(se->avg.load_avg * cfs_rq->h_load,
 			cfs_rq_load_avg(cfs_rq) + 1);
 }
 #else
@@ -7865,9 +7865,9 @@ static inline void update_blocked_averages(int cpu)
 	rq_unlock_irqrestore(rq, &rf);
 }
 
-static unsigned long task_h_load(struct task_struct *p)
+static unsigned long task_se_h_load(struct sched_entity *se)
 {
-	return p->se.avg.load_avg;
+	return se->avg.load_avg;
 }
 #endif
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 02/14] sched: change /proc/sched_debug fields
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
  2019-07-22 17:33 ` [PATCH 01/14] sched: introduce task_se_h_load helper Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-22 17:33 ` [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load Rik van Riel
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel,
	Josef Bacik

Remove some fields from /proc/sched_debug that are removed from
sched_entity in a subsequent patch, and add h_load, which comes in
very handy to debug CPU controller weight distribution.

Signed-off-by: Rik van Riel <riel@surriel.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel/sched/debug.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 14c6a8716ba1..f6beaca97a09 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -416,11 +416,9 @@ static void print_cfs_group_stats(struct seq_file *m, int cpu, struct task_group
 	}
 
 	P(se->load.weight);
-	P(se->runnable_weight);
 #ifdef CONFIG_SMP
 	P(se->avg.load_avg);
 	P(se->avg.util_avg);
-	P(se->avg.runnable_load_avg);
 #endif
 
 #undef PN_SCHEDSTAT
@@ -538,7 +536,6 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
 	SEQ_printf(m, "  .%-30s: %d\n", "nr_running", cfs_rq->nr_running);
 	SEQ_printf(m, "  .%-30s: %ld\n", "load", cfs_rq->load.weight);
 #ifdef CONFIG_SMP
-	SEQ_printf(m, "  .%-30s: %ld\n", "runnable_weight", cfs_rq->runnable_weight);
 	SEQ_printf(m, "  .%-30s: %lu\n", "load_avg",
 			cfs_rq->avg.load_avg);
 	SEQ_printf(m, "  .%-30s: %lu\n", "runnable_load_avg",
@@ -547,17 +544,15 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
 			cfs_rq->avg.util_avg);
 	SEQ_printf(m, "  .%-30s: %u\n", "util_est_enqueued",
 			cfs_rq->avg.util_est.enqueued);
-	SEQ_printf(m, "  .%-30s: %ld\n", "removed.load_avg",
-			cfs_rq->removed.load_avg);
 	SEQ_printf(m, "  .%-30s: %ld\n", "removed.util_avg",
 			cfs_rq->removed.util_avg);
-	SEQ_printf(m, "  .%-30s: %ld\n", "removed.runnable_sum",
-			cfs_rq->removed.runnable_sum);
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	SEQ_printf(m, "  .%-30s: %lu\n", "tg_load_avg_contrib",
 			cfs_rq->tg_load_avg_contrib);
 	SEQ_printf(m, "  .%-30s: %ld\n", "tg_load_avg",
 			atomic_long_read(&cfs_rq->tg->load_avg));
+	SEQ_printf(m, "  .%-30s: %lu\n", "h_load",
+			cfs_rq->h_load);
 #endif
 #endif
 #ifdef CONFIG_CFS_BANDWIDTH
@@ -961,10 +956,8 @@ void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
 		   "nr_involuntary_switches", (long long)p->nivcsw);
 
 	P(se.load.weight);
-	P(se.runnable_weight);
 #ifdef CONFIG_SMP
 	P(se.avg.load_sum);
-	P(se.avg.runnable_load_sum);
 	P(se.avg.util_sum);
 	P(se.avg.load_avg);
 	P(se.avg.runnable_load_avg);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
  2019-07-22 17:33 ` [PATCH 01/14] sched: introduce task_se_h_load helper Rik van Riel
  2019-07-22 17:33 ` [PATCH 02/14] sched: change /proc/sched_debug fields Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-29 20:05   ` Peter Zijlstra
                     ` (2 more replies)
  2019-07-22 17:33 ` [PATCH 04/14] sched,fair: move runnable_load_avg to cfs_rq Rik van Riel
                   ` (10 subsequent siblings)
  13 siblings, 3 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel

The runnable_load magic is used to quickly propagate information about
runnable tasks up the hierarchy of runqueues. The runnable_load_avg is
mostly used for the load balancing code, which only examines the value at
the root cfs_rq.

Redefine the root cfs_rq runnable_load_avg to be the sum of task_h_loads
of the runnable tasks. This works because the hierarchical runnable_load of
a task is already equal to the task_se_h_load today. This provides enough
information to the load balancer.

The runnable_load_avg of the cgroup cfs_rqs does not appear to be
used for anything, so don't bother calculating those.

This removes one of the things that the code currently traverses the
cgroup hierarchy for, and getting rid of it brings us one step closer
to a flat runqueue for the CPU controller.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 include/linux/sched.h |   3 +-
 kernel/sched/core.c   |   2 -
 kernel/sched/debug.c  |   1 +
 kernel/sched/fair.c   | 125 +++++++++++++-----------------------------
 kernel/sched/pelt.c   |  49 ++++++-----------
 kernel/sched/sched.h  |   6 --
 6 files changed, 55 insertions(+), 131 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 11837410690f..f5bb6948e40c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -391,7 +391,6 @@ struct util_est {
 struct sched_avg {
 	u64				last_update_time;
 	u64				load_sum;
-	u64				runnable_load_sum;
 	u32				util_sum;
 	u32				period_contrib;
 	unsigned long			load_avg;
@@ -439,7 +438,6 @@ struct sched_statistics {
 struct sched_entity {
 	/* For load-balancing: */
 	struct load_weight		load;
-	unsigned long			runnable_weight;
 	struct rb_node			run_node;
 	struct list_head		group_node;
 	unsigned int			on_rq;
@@ -455,6 +453,7 @@ struct sched_entity {
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	int				depth;
+	unsigned long			enqueued_h_load;
 	struct sched_entity		*parent;
 	/* rq on which this entity is (to be) queued: */
 	struct cfs_rq			*cfs_rq;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 874c427742a9..fbd96900f715 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -744,7 +744,6 @@ static void set_load_weight(struct task_struct *p, bool update_load)
 	if (task_has_idle_policy(p)) {
 		load->weight = scale_load(WEIGHT_IDLEPRIO);
 		load->inv_weight = WMULT_IDLEPRIO;
-		p->se.runnable_weight = load->weight;
 		return;
 	}
 
@@ -757,7 +756,6 @@ static void set_load_weight(struct task_struct *p, bool update_load)
 	} else {
 		load->weight = scale_load(sched_prio_to_weight[prio]);
 		load->inv_weight = sched_prio_to_wmult[prio];
-		p->se.runnable_weight = load->weight;
 	}
 }
 
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index f6beaca97a09..cefc1b171c0b 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -962,6 +962,7 @@ void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
 	P(se.avg.load_avg);
 	P(se.avg.runnable_load_avg);
 	P(se.avg.util_avg);
+	P(se.enqueued_h_load);
 	P(se.avg.last_update_time);
 	P(se.avg.util_est.ewma);
 	P(se.avg.util_est.enqueued);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index eadf9a96b3e1..860708b687a7 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -723,9 +723,7 @@ void init_entity_runnable_average(struct sched_entity *se)
 	 * nothing has been attached to the task group yet.
 	 */
 	if (entity_is_task(se))
-		sa->runnable_load_avg = sa->load_avg = scale_load_down(se->load.weight);
-
-	se->runnable_weight = se->load.weight;
+		sa->load_avg = scale_load_down(se->load.weight);
 
 	/* when this task enqueue'ed, it will contribute to its cfs_rq's load_avg */
 }
@@ -2766,20 +2764,39 @@ account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)
 static inline void
 enqueue_runnable_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
-	cfs_rq->runnable_weight += se->runnable_weight;
+	if (entity_is_task(se)) {
+		struct cfs_rq *root_cfs_rq = &cfs_rq->rq->cfs;
+		se->enqueued_h_load = task_se_h_load(se);
 
-	cfs_rq->avg.runnable_load_avg += se->avg.runnable_load_avg;
-	cfs_rq->avg.runnable_load_sum += se_runnable(se) * se->avg.runnable_load_sum;
+		root_cfs_rq->avg.runnable_load_avg += se->enqueued_h_load;
+	}
 }
 
 static inline void
 dequeue_runnable_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
-	cfs_rq->runnable_weight -= se->runnable_weight;
+	if (entity_is_task(se)) {
+		struct cfs_rq *root_cfs_rq = &cfs_rq->rq->cfs;
+		sub_positive(&root_cfs_rq->avg.runnable_load_avg,
+					se->enqueued_h_load);
+	}
+}
+
+static inline void
+update_runnable_load_avg(struct sched_entity *se)
+{
+	struct cfs_rq *root_cfs_rq = &cfs_rq_of(se)->rq->cfs;
+	long new_h_load, delta;
+
+	SCHED_WARN_ON(!entity_is_task(se));
+
+	if (!se->on_rq)
+		return;
 
-	sub_positive(&cfs_rq->avg.runnable_load_avg, se->avg.runnable_load_avg);
-	sub_positive(&cfs_rq->avg.runnable_load_sum,
-		     se_runnable(se) * se->avg.runnable_load_sum);
+	new_h_load = task_se_h_load(se);
+	delta = new_h_load - se->enqueued_h_load;
+	root_cfs_rq->avg.runnable_load_avg += delta;
+	se->enqueued_h_load = new_h_load;
 }
 
 static inline void
@@ -2807,7 +2824,7 @@ dequeue_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) { }
 #endif
 
 static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
-			    unsigned long weight, unsigned long runnable)
+			    unsigned long weight)
 {
 	if (se->on_rq) {
 		/* commit outstanding execution time */
@@ -2818,7 +2835,6 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
 	}
 	dequeue_load_avg(cfs_rq, se);
 
-	se->runnable_weight = runnable;
 	update_load_set(&se->load, weight);
 
 #ifdef CONFIG_SMP
@@ -2826,8 +2842,6 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
 		u32 divider = LOAD_AVG_MAX - 1024 + se->avg.period_contrib;
 
 		se->avg.load_avg = div_u64(se_weight(se) * se->avg.load_sum, divider);
-		se->avg.runnable_load_avg =
-			div_u64(se_runnable(se) * se->avg.runnable_load_sum, divider);
 	} while (0);
 #endif
 
@@ -2845,7 +2859,7 @@ void reweight_task(struct task_struct *p, int prio)
 	struct load_weight *load = &se->load;
 	unsigned long weight = scale_load(sched_prio_to_weight[prio]);
 
-	reweight_entity(cfs_rq, se, weight, weight);
+	reweight_entity(cfs_rq, se, weight);
 	load->inv_weight = sched_prio_to_wmult[prio];
 }
 
@@ -2958,49 +2972,6 @@ static long calc_group_shares(struct cfs_rq *cfs_rq)
 	return clamp_t(long, shares, MIN_SHARES, tg_shares);
 }
 
-/*
- * This calculates the effective runnable weight for a group entity based on
- * the group entity weight calculated above.
- *
- * Because of the above approximation (2), our group entity weight is
- * an load_avg based ratio (3). This means that it includes blocked load and
- * does not represent the runnable weight.
- *
- * Approximate the group entity's runnable weight per ratio from the group
- * runqueue:
- *
- *					     grq->avg.runnable_load_avg
- *   ge->runnable_weight = ge->load.weight * -------------------------- (7)
- *						 grq->avg.load_avg
- *
- * However, analogous to above, since the avg numbers are slow, this leads to
- * transients in the from-idle case. Instead we use:
- *
- *   ge->runnable_weight = ge->load.weight *
- *
- *		max(grq->avg.runnable_load_avg, grq->runnable_weight)
- *		-----------------------------------------------------	(8)
- *		      max(grq->avg.load_avg, grq->load.weight)
- *
- * Where these max() serve both to use the 'instant' values to fix the slow
- * from-idle and avoid the /0 on to-idle, similar to (6).
- */
-static long calc_group_runnable(struct cfs_rq *cfs_rq, long shares)
-{
-	long runnable, load_avg;
-
-	load_avg = max(cfs_rq->avg.load_avg,
-		       scale_load_down(cfs_rq->load.weight));
-
-	runnable = max(cfs_rq->avg.runnable_load_avg,
-		       scale_load_down(cfs_rq->runnable_weight));
-
-	runnable *= shares;
-	if (load_avg)
-		runnable /= load_avg;
-
-	return clamp_t(long, runnable, MIN_SHARES, shares);
-}
 #endif /* CONFIG_SMP */
 
 static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
@@ -3012,25 +2983,24 @@ static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
 static void update_cfs_group(struct sched_entity *se)
 {
 	struct cfs_rq *gcfs_rq = group_cfs_rq(se);
-	long shares, runnable;
+	long shares;
 
-	if (!gcfs_rq)
+	if (!gcfs_rq) {
+		update_runnable_load_avg(se);
 		return;
+	}
 
 	if (throttled_hierarchy(gcfs_rq))
 		return;
 
 #ifndef CONFIG_SMP
-	runnable = shares = READ_ONCE(gcfs_rq->tg->shares);
-
 	if (likely(se->load.weight == shares))
 		return;
 #else
 	shares   = calc_group_shares(gcfs_rq);
-	runnable = calc_group_runnable(gcfs_rq, shares);
 #endif
 
-	reweight_entity(cfs_rq_of(se), se, shares, runnable);
+	reweight_entity(cfs_rq_of(se), se, shares);
 }
 
 #else /* CONFIG_FAIR_GROUP_SCHED */
@@ -3243,8 +3213,8 @@ static inline void
 update_tg_cfs_runnable(struct cfs_rq *cfs_rq, struct sched_entity *se, struct cfs_rq *gcfs_rq)
 {
 	long delta_avg, running_sum, runnable_sum = gcfs_rq->prop_runnable_sum;
-	unsigned long runnable_load_avg, load_avg;
-	u64 runnable_load_sum, load_sum = 0;
+	unsigned long load_avg;
+	u64 load_sum = 0;
 	s64 delta_sum;
 
 	if (!runnable_sum)
@@ -3292,19 +3262,6 @@ update_tg_cfs_runnable(struct cfs_rq *cfs_rq, struct sched_entity *se, struct cf
 	se->avg.load_avg = load_avg;
 	add_positive(&cfs_rq->avg.load_avg, delta_avg);
 	add_positive(&cfs_rq->avg.load_sum, delta_sum);
-
-	runnable_load_sum = (s64)se_runnable(se) * runnable_sum;
-	runnable_load_avg = div_s64(runnable_load_sum, LOAD_AVG_MAX);
-	delta_sum = runnable_load_sum - se_weight(se) * se->avg.runnable_load_sum;
-	delta_avg = runnable_load_avg - se->avg.runnable_load_avg;
-
-	se->avg.runnable_load_sum = runnable_sum;
-	se->avg.runnable_load_avg = runnable_load_avg;
-
-	if (se->on_rq) {
-		add_positive(&cfs_rq->avg.runnable_load_avg, delta_avg);
-		add_positive(&cfs_rq->avg.runnable_load_sum, delta_sum);
-	}
 }
 
 static inline void add_tg_cfs_propagate(struct cfs_rq *cfs_rq, long runnable_sum)
@@ -3399,7 +3356,7 @@ static inline void add_tg_cfs_propagate(struct cfs_rq *cfs_rq, long runnable_sum
 static inline int
 update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
 {
-	unsigned long removed_load = 0, removed_util = 0, removed_runnable_sum = 0;
+	unsigned long removed_load = 0, removed_util = 0;
 	struct sched_avg *sa = &cfs_rq->avg;
 	int decayed = 0;
 
@@ -3410,7 +3367,6 @@ update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
 		raw_spin_lock(&cfs_rq->removed.lock);
 		swap(cfs_rq->removed.util_avg, removed_util);
 		swap(cfs_rq->removed.load_avg, removed_load);
-		swap(cfs_rq->removed.runnable_sum, removed_runnable_sum);
 		cfs_rq->removed.nr = 0;
 		raw_spin_unlock(&cfs_rq->removed.lock);
 
@@ -3422,8 +3378,6 @@ update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
 		sub_positive(&sa->util_avg, r);
 		sub_positive(&sa->util_sum, r * divider);
 
-		add_tg_cfs_propagate(cfs_rq, -(long)removed_runnable_sum);
-
 		decayed = 1;
 	}
 
@@ -3477,8 +3431,6 @@ static void attach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 			div_u64(se->avg.load_avg * se->avg.load_sum, se_weight(se));
 	}
 
-	se->avg.runnable_load_sum = se->avg.load_sum;
-
 	enqueue_load_avg(cfs_rq, se);
 	cfs_rq->avg.util_avg += se->avg.util_avg;
 	cfs_rq->avg.util_sum += se->avg.util_sum;
@@ -7735,9 +7687,6 @@ static inline bool cfs_rq_is_decayed(struct cfs_rq *cfs_rq)
 	if (cfs_rq->avg.util_sum)
 		return false;
 
-	if (cfs_rq->avg.runnable_load_sum)
-		return false;
-
 	return true;
 }
 
diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c
index befce29bd882..190797e421dd 100644
--- a/kernel/sched/pelt.c
+++ b/kernel/sched/pelt.c
@@ -106,7 +106,7 @@ static u32 __accumulate_pelt_segments(u64 periods, u32 d1, u32 d3)
  */
 static __always_inline u32
 accumulate_sum(u64 delta, struct sched_avg *sa,
-	       unsigned long load, unsigned long runnable, int running)
+	       unsigned long load, int running)
 {
 	u32 contrib = (u32)delta; /* p == 0 -> delta < 1024 */
 	u64 periods;
@@ -119,8 +119,6 @@ accumulate_sum(u64 delta, struct sched_avg *sa,
 	 */
 	if (periods) {
 		sa->load_sum = decay_load(sa->load_sum, periods);
-		sa->runnable_load_sum =
-			decay_load(sa->runnable_load_sum, periods);
 		sa->util_sum = decay_load((u64)(sa->util_sum), periods);
 
 		/*
@@ -134,8 +132,6 @@ accumulate_sum(u64 delta, struct sched_avg *sa,
 
 	if (load)
 		sa->load_sum += load * contrib;
-	if (runnable)
-		sa->runnable_load_sum += runnable * contrib;
 	if (running)
 		sa->util_sum += contrib << SCHED_CAPACITY_SHIFT;
 
@@ -172,7 +168,7 @@ accumulate_sum(u64 delta, struct sched_avg *sa,
  */
 static __always_inline int
 ___update_load_sum(u64 now, struct sched_avg *sa,
-		  unsigned long load, unsigned long runnable, int running)
+		  unsigned long load, int running)
 {
 	u64 delta;
 
@@ -206,7 +202,7 @@ ___update_load_sum(u64 now, struct sched_avg *sa,
 	 * update_blocked_averages()
 	 */
 	if (!load)
-		runnable = running = 0;
+		running = 0;
 
 	/*
 	 * Now we know we crossed measurement unit boundaries. The *_avg
@@ -215,14 +211,14 @@ ___update_load_sum(u64 now, struct sched_avg *sa,
 	 * Step 1: accumulate *_sum since last_update_time. If we haven't
 	 * crossed period boundaries, finish.
 	 */
-	if (!accumulate_sum(delta, sa, load, runnable, running))
+	if (!accumulate_sum(delta, sa, load, running))
 		return 0;
 
 	return 1;
 }
 
 static __always_inline void
-___update_load_avg(struct sched_avg *sa, unsigned long load, unsigned long runnable)
+___update_load_avg(struct sched_avg *sa, unsigned long load)
 {
 	u32 divider = LOAD_AVG_MAX - 1024 + sa->period_contrib;
 
@@ -230,7 +226,6 @@ ___update_load_avg(struct sched_avg *sa, unsigned long load, unsigned long runna
 	 * Step 2: update *_avg.
 	 */
 	sa->load_avg = div_u64(load * sa->load_sum, divider);
-	sa->runnable_load_avg =	div_u64(runnable * sa->runnable_load_sum, divider);
 	WRITE_ONCE(sa->util_avg, sa->util_sum / divider);
 }
 
@@ -263,8 +258,8 @@ ___update_load_avg(struct sched_avg *sa, unsigned long load, unsigned long runna
 
 int __update_load_avg_blocked_se(u64 now, struct sched_entity *se)
 {
-	if (___update_load_sum(now, &se->avg, 0, 0, 0)) {
-		___update_load_avg(&se->avg, se_weight(se), se_runnable(se));
+	if (___update_load_sum(now, &se->avg, 0, 0)) {
+		___update_load_avg(&se->avg, se_weight(se));
 		return 1;
 	}
 
@@ -273,10 +268,10 @@ int __update_load_avg_blocked_se(u64 now, struct sched_entity *se)
 
 int __update_load_avg_se(u64 now, struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
-	if (___update_load_sum(now, &se->avg, !!se->on_rq, !!se->on_rq,
+	if (___update_load_sum(now, &se->avg, !!se->on_rq,
 				cfs_rq->curr == se)) {
 
-		___update_load_avg(&se->avg, se_weight(se), se_runnable(se));
+		___update_load_avg(&se->avg, se_weight(se));
 		cfs_se_util_change(&se->avg);
 		return 1;
 	}
@@ -288,10 +283,9 @@ int __update_load_avg_cfs_rq(u64 now, struct cfs_rq *cfs_rq)
 {
 	if (___update_load_sum(now, &cfs_rq->avg,
 				scale_load_down(cfs_rq->load.weight),
-				scale_load_down(cfs_rq->runnable_weight),
 				cfs_rq->curr != NULL)) {
 
-		___update_load_avg(&cfs_rq->avg, 1, 1);
+		___update_load_avg(&cfs_rq->avg, 1);
 		return 1;
 	}
 
@@ -303,20 +297,18 @@ int __update_load_avg_cfs_rq(u64 now, struct cfs_rq *cfs_rq)
  *
  *   util_sum = \Sum se->avg.util_sum but se->avg.util_sum is not tracked
  *   util_sum = cpu_scale * load_sum
- *   runnable_load_sum = load_sum
  *
- *   load_avg and runnable_load_avg are not supported and meaningless.
+ *   load_avg is not supported and meaningless.
  *
  */
 
 int update_rt_rq_load_avg(u64 now, struct rq *rq, int running)
 {
 	if (___update_load_sum(now, &rq->avg_rt,
-				running,
 				running,
 				running)) {
 
-		___update_load_avg(&rq->avg_rt, 1, 1);
+		___update_load_avg(&rq->avg_rt, 1);
 		return 1;
 	}
 
@@ -328,18 +320,16 @@ int update_rt_rq_load_avg(u64 now, struct rq *rq, int running)
  *
  *   util_sum = \Sum se->avg.util_sum but se->avg.util_sum is not tracked
  *   util_sum = cpu_scale * load_sum
- *   runnable_load_sum = load_sum
  *
  */
 
 int update_dl_rq_load_avg(u64 now, struct rq *rq, int running)
 {
 	if (___update_load_sum(now, &rq->avg_dl,
-				running,
 				running,
 				running)) {
 
-		___update_load_avg(&rq->avg_dl, 1, 1);
+		___update_load_avg(&rq->avg_dl, 1);
 		return 1;
 	}
 
@@ -352,7 +342,6 @@ int update_dl_rq_load_avg(u64 now, struct rq *rq, int running)
  *
  *   util_sum = \Sum se->avg.util_sum but se->avg.util_sum is not tracked
  *   util_sum = cpu_scale * load_sum
- *   runnable_load_sum = load_sum
  *
  */
 
@@ -379,17 +368,11 @@ int update_irq_load_avg(struct rq *rq, u64 running)
 	 * We can safely remove running from rq->clock because
 	 * rq->clock += delta with delta >= running
 	 */
-	ret = ___update_load_sum(rq->clock - running, &rq->avg_irq,
-				0,
-				0,
-				0);
-	ret += ___update_load_sum(rq->clock, &rq->avg_irq,
-				1,
-				1,
-				1);
+	ret = ___update_load_sum(rq->clock - running, &rq->avg_irq, 0, 0);
+	ret += ___update_load_sum(rq->clock, &rq->avg_irq, 1, 1);
 
 	if (ret)
-		___update_load_avg(&rq->avg_irq, 1, 1);
+		___update_load_avg(&rq->avg_irq, 1);
 
 	return ret;
 }
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index b52ed1ada0be..5be14cee61f9 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -487,7 +487,6 @@ struct cfs_bandwidth { };
 /* CFS-related fields in a runqueue */
 struct cfs_rq {
 	struct load_weight	load;
-	unsigned long		runnable_weight;
 	unsigned int		nr_running;
 	unsigned int		h_nr_running;
 
@@ -700,11 +699,6 @@ static inline long se_weight(struct sched_entity *se)
 	return scale_load_down(se->load.weight);
 }
 
-static inline long se_runnable(struct sched_entity *se)
-{
-	return scale_load_down(se->runnable_weight);
-}
-
 static inline bool sched_asym_prefer(int a, int b)
 {
 	return arch_asym_cpu_priority(a) > arch_asym_cpu_priority(b);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 04/14] sched,fair: move runnable_load_avg to cfs_rq
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (2 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-30  8:55   ` Peter Zijlstra
  2019-07-22 17:33 ` [PATCH 06/14] sched,cfs: use explicit cfs_rq of parent se helper Rik van Riel
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel

Since only the root cfs_rq runnable_load_avg field is used any more,
we can move the field from struct sched_avg, which every sched_entity
has one of, directly into the struct cfs_rq, of which we have way fewer.

No functional changes.

Suggested-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 include/linux/sched.h | 1 -
 kernel/sched/debug.c  | 3 +--
 kernel/sched/fair.c   | 8 ++++----
 kernel/sched/sched.h  | 1 +
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index f5bb6948e40c..84a6cc6f5c47 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -394,7 +394,6 @@ struct sched_avg {
 	u32				util_sum;
 	u32				period_contrib;
 	unsigned long			load_avg;
-	unsigned long			runnable_load_avg;
 	unsigned long			util_avg;
 	struct util_est			util_est;
 } ____cacheline_aligned;
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index cefc1b171c0b..6e7c8ff210a8 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -539,7 +539,7 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
 	SEQ_printf(m, "  .%-30s: %lu\n", "load_avg",
 			cfs_rq->avg.load_avg);
 	SEQ_printf(m, "  .%-30s: %lu\n", "runnable_load_avg",
-			cfs_rq->avg.runnable_load_avg);
+			cfs_rq->runnable_load_avg);
 	SEQ_printf(m, "  .%-30s: %lu\n", "util_avg",
 			cfs_rq->avg.util_avg);
 	SEQ_printf(m, "  .%-30s: %u\n", "util_est_enqueued",
@@ -960,7 +960,6 @@ void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
 	P(se.avg.load_sum);
 	P(se.avg.util_sum);
 	P(se.avg.load_avg);
-	P(se.avg.runnable_load_avg);
 	P(se.avg.util_avg);
 	P(se.enqueued_h_load);
 	P(se.avg.last_update_time);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 860708b687a7..63cb40253b26 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2768,7 +2768,7 @@ enqueue_runnable_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
 		struct cfs_rq *root_cfs_rq = &cfs_rq->rq->cfs;
 		se->enqueued_h_load = task_se_h_load(se);
 
-		root_cfs_rq->avg.runnable_load_avg += se->enqueued_h_load;
+		root_cfs_rq->runnable_load_avg += se->enqueued_h_load;
 	}
 }
 
@@ -2777,7 +2777,7 @@ dequeue_runnable_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
 	if (entity_is_task(se)) {
 		struct cfs_rq *root_cfs_rq = &cfs_rq->rq->cfs;
-		sub_positive(&root_cfs_rq->avg.runnable_load_avg,
+		sub_positive(&root_cfs_rq->runnable_load_avg,
 					se->enqueued_h_load);
 	}
 }
@@ -2795,7 +2795,7 @@ update_runnable_load_avg(struct sched_entity *se)
 
 	new_h_load = task_se_h_load(se);
 	delta = new_h_load - se->enqueued_h_load;
-	root_cfs_rq->avg.runnable_load_avg += delta;
+	root_cfs_rq->runnable_load_avg += delta;
 	se->enqueued_h_load = new_h_load;
 }
 
@@ -3559,7 +3559,7 @@ static void remove_entity_load_avg(struct sched_entity *se)
 
 static inline unsigned long cfs_rq_runnable_load_avg(struct cfs_rq *cfs_rq)
 {
-	return cfs_rq->avg.runnable_load_avg;
+	return cfs_rq->runnable_load_avg;
 }
 
 static inline unsigned long cfs_rq_load_avg(struct cfs_rq *cfs_rq)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 5be14cee61f9..32978a8de8ce 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -516,6 +516,7 @@ struct cfs_rq {
 	 * CFS load tracking
 	 */
 	struct sched_avg	avg;
+	unsigned long		runnable_load_avg;
 #ifndef CONFIG_64BIT
 	u64			load_last_update_time_copy;
 #endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 06/14] sched,cfs: use explicit cfs_rq of parent se helper
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (3 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 04/14] sched,fair: move runnable_load_avg to cfs_rq Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-22 17:33 ` [PATCH 07/14] sched,cfs: fix zero length timeslice calculation Rik van Riel
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel

Use an explicit "cfs_rq of parent sched_entity" helper in a few
strategic places, where cfs_rq_of(se) may no longer point at the
right runqueue once we flatten the hierarchical cgroup runqueues.

No functional change.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 kernel/sched/fair.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d6a8aa78a948..39f7a2d810e1 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -276,6 +276,15 @@ static inline struct cfs_rq *group_cfs_rq(struct sched_entity *grp)
 	return grp->my_q;
 }
 
+/* runqueue owned by the parent entity; the root cfs_rq for a top level se */
+static inline struct cfs_rq *group_cfs_rq_of_parent(struct sched_entity *se)
+{
+	if (se->parent)
+		return group_cfs_rq(se->parent);
+
+	return cfs_rq_of(se);
+}
+
 static inline bool list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq)
 {
 	struct rq *rq = rq_of(cfs_rq);
@@ -3317,7 +3326,7 @@ static inline int propagate_entity_load_avg(struct sched_entity *se)
 
 	gcfs_rq->propagate = 0;
 
-	cfs_rq = cfs_rq_of(se);
+	cfs_rq = group_cfs_rq_of_parent(se);
 
 	add_tg_cfs_propagate(cfs_rq, gcfs_rq->prop_runnable_sum);
 
@@ -7794,7 +7803,7 @@ static void update_cfs_rq_h_load(struct cfs_rq *cfs_rq)
 
 	WRITE_ONCE(cfs_rq->h_load_next, NULL);
 	for_each_sched_entity(se) {
-		cfs_rq = cfs_rq_of(se);
+		cfs_rq = group_cfs_rq_of_parent(se);
 		WRITE_ONCE(cfs_rq->h_load_next, se);
 		if (cfs_rq->last_h_load_update == now)
 			break;
@@ -7817,7 +7826,7 @@ static void update_cfs_rq_h_load(struct cfs_rq *cfs_rq)
 
 static unsigned long task_se_h_load(struct sched_entity *se)
 {
-	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+	struct cfs_rq *cfs_rq = group_cfs_rq_of_parent(se);
 
 	update_cfs_rq_h_load(cfs_rq);
 	return div64_ul(se->avg.load_avg * cfs_rq->h_load,
@@ -10164,7 +10173,7 @@ static void task_tick_fair(struct rq *rq, struct task_struct *curr, int queued)
 	struct sched_entity *se = &curr->se;
 
 	for_each_sched_entity(se) {
-		cfs_rq = cfs_rq_of(se);
+		cfs_rq = group_cfs_rq_of_parent(se);
 		entity_tick(cfs_rq, se, queued);
 	}
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 07/14] sched,cfs: fix zero length timeslice calculation
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (4 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 06/14] sched,cfs: use explicit cfs_rq of parent se helper Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-22 17:33 ` [PATCH 08/14] sched,fair: simplify timeslice length code Rik van Riel
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel

The way the time slice length is currently calculated, not only do high
priority tasks get longer time slices than low priority tasks, but due
to fixed point math, low priority tasks could end up with a zero length
time slice. This can lead to cache thrashing and other inefficiencies.

Cap the minimum time slice length to sysctl_sched_min_granularity.

Tasks that end up getting a time slice length too long for their relative
priority will simply end up having their vruntime advanced much faster than
other tasks, resulting in them receiving time slices less frequently.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 kernel/sched/fair.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 39f7a2d810e1..9ff69b927a3c 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -732,6 +732,13 @@ static u64 sched_slice(struct cfs_rq *cfs_rq, struct sched_entity *se)
 		}
 		slice = __calc_delta(slice, se->load.weight, load);
 	}
+
+	/*
+	 * To avoid cache thrashing, run at least sysctl_sched_min_granularity.
+	 * The vruntime of a low priority task advances faster; those tasks
+	 * will simply get time slices less frequently.
+	 */
+	slice = max_t(u64, slice, sysctl_sched_min_granularity);
 	return slice;
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 08/14] sched,fair: simplify timeslice length code
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (5 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 07/14] sched,cfs: fix zero length timeslice calculation Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-22 17:33 ` [PATCH 09/14] sched,fair: refactor enqueue/dequeue_entity Rik van Riel
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel

The idea behind __sched_period makes sense, but the results do not always.

When a CPU has one high priority task and a large number of low priority
tasks, __sched_period will return a value larger than sysctl_sched_latency,
and the one high priority task may end up getting a timeslice all for itself
that is also much larger than sysctl_sched_latency.

The low priority tasks will have their time slices rounded up to
sysctl_sched_min_granularity, resulting in an even larger scheduling
latency than targeted by __sched_period.

Simplify the code by simply ripping out __sched_period and always taking
fractions of sysctl_sched_latency.

If a high priority task ends up getting a "too small" time slice compared
to low priority tasks, the vruntime scaling ensures that it will simply
get scheduled more frequently than low priority tasks.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 kernel/sched/fair.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 9ff69b927a3c..b4fc328032e6 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -691,22 +691,6 @@ static inline u64 calc_delta_fair(u64 delta, struct sched_entity *se)
 	return delta;
 }
 
-/*
- * The idea is to set a period in which each task runs once.
- *
- * When there are too many tasks (sched_nr_latency) we have to stretch
- * this period because otherwise the slices get too small.
- *
- * p = (nr <= nl) ? l : l*nr/nl
- */
-static u64 __sched_period(unsigned long nr_running)
-{
-	if (unlikely(nr_running > sched_nr_latency))
-		return nr_running * sysctl_sched_min_granularity;
-	else
-		return sysctl_sched_latency;
-}
-
 /*
  * We calculate the wall-time slice from the period by taking a part
  * proportional to the weight.
@@ -715,7 +699,7 @@ static u64 __sched_period(unsigned long nr_running)
  */
 static u64 sched_slice(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
-	u64 slice = __sched_period(cfs_rq->nr_running + !se->on_rq);
+	u64 slice = sysctl_sched_latency;
 
 	for_each_sched_entity(se) {
 		struct load_weight *load;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 09/14] sched,fair: refactor enqueue/dequeue_entity
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (6 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 08/14] sched,fair: simplify timeslice length code Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-30  9:36   ` Peter Zijlstra
  2019-07-22 17:33 ` [PATCH 10/14] sched,fair: add helper functions for flattened runqueue Rik van Riel
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel

Refactor enqueue_entity, dequeue_entity, and update_load_avg, in order
to split out the things we still want to happen at every level in the
cgroup hierarchy with a flat runqueue from the things we only need to
happen once.

No functional changes.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 kernel/sched/fair.c | 64 +++++++++++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 22 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b4fc328032e6..c944729423bd 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3500,7 +3500,7 @@ static void detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 #define DO_ATTACH	0x4
 
 /* Update task and its cfs_rq load average */
-static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
+static inline bool update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
 	u64 now = cfs_rq_clock_pelt(cfs_rq);
 	int decayed;
@@ -3529,6 +3529,8 @@ static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 
 	} else if (decayed && (flags & UPDATE_TG))
 		update_tg_load_avg(cfs_rq, 0);
+
+	return decayed;
 }
 
 #ifndef CONFIG_64BIT
@@ -3745,9 +3747,10 @@ static inline void update_misfit_status(struct task_struct *p, struct rq *rq)
 #define SKIP_AGE_LOAD	0x0
 #define DO_ATTACH	0x0
 
-static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int not_used1)
+static inline bool update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int not_used1)
 {
 	cfs_rq_util_change(cfs_rq, 0);
+	return false;
 }
 
 static inline void remove_entity_load_avg(struct sched_entity *se) {}
@@ -3870,6 +3873,24 @@ static inline void check_schedstat_required(void)
  * CPU and an up-to-date min_vruntime on the destination CPU.
  */
 
+static bool
+enqueue_entity_groups(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
+{
+	/*
+	 * When enqueuing a sched_entity, we must:
+	 *   - Update loads to have both entity and cfs_rq synced with now.
+	 *   - Add its load to cfs_rq->runnable_avg
+	 *   - For group_entity, update its weight to reflect the new share of
+	 *     its group cfs_rq
+	 *   - Add its new weight to cfs_rq->load.weight
+	 */
+	if (!update_load_avg(cfs_rq, se, UPDATE_TG | DO_ATTACH))
+		return false;
+
+	update_cfs_group(se);
+	return true;
+}
+
 static void
 enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
@@ -3894,16 +3915,6 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 	if (renorm && !curr)
 		se->vruntime += cfs_rq->min_vruntime;
 
-	/*
-	 * When enqueuing a sched_entity, we must:
-	 *   - Update loads to have both entity and cfs_rq synced with now.
-	 *   - Add its load to cfs_rq->runnable_avg
-	 *   - For group_entity, update its weight to reflect the new share of
-	 *     its group cfs_rq
-	 *   - Add its new weight to cfs_rq->load.weight
-	 */
-	update_load_avg(cfs_rq, se, UPDATE_TG | DO_ATTACH);
-	update_cfs_group(se);
 	enqueue_runnable_load_avg(cfs_rq, se);
 	account_entity_enqueue(cfs_rq, se);
 
@@ -3970,14 +3981,9 @@ static void clear_buddies(struct cfs_rq *cfs_rq, struct sched_entity *se)
 
 static __always_inline void return_cfs_rq_runtime(struct cfs_rq *cfs_rq);
 
-static void
-dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
+static bool
+dequeue_entity_groups(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
-	/*
-	 * Update run-time statistics of the 'current'.
-	 */
-	update_curr(cfs_rq);
-
 	/*
 	 * When dequeuing a sched_entity, we must:
 	 *   - Update loads to have both entity and cfs_rq synced with now.
@@ -3986,7 +3992,21 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 	 *   - For group entity, update its weight to reflect the new share
 	 *     of its group cfs_rq.
 	 */
-	update_load_avg(cfs_rq, se, UPDATE_TG);
+	if (!update_load_avg(cfs_rq, se, UPDATE_TG))
+		return false;
+	update_cfs_group(se);
+
+	return true;
+}
+
+static void
+dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
+{
+	/*
+	 * Update run-time statistics of the 'current'.
+	 */
+	update_curr(cfs_rq);
+
 	dequeue_runnable_load_avg(cfs_rq, se);
 
 	update_stats_dequeue(cfs_rq, se, flags);
@@ -4010,8 +4030,6 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 	/* return excess runtime on last dequeue */
 	return_cfs_rq_runtime(cfs_rq);
 
-	update_cfs_group(se);
-
 	/*
 	 * Now advance min_vruntime if @se was the entity holding it back,
 	 * except when: DEQUEUE_SAVE && !DEQUEUE_MOVE, in this case we'll be
@@ -5176,6 +5194,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 		if (se->on_rq)
 			break;
 		cfs_rq = cfs_rq_of(se);
+		enqueue_entity_groups(cfs_rq, se, flags);
 		enqueue_entity(cfs_rq, se, flags);
 
 		/*
@@ -5258,6 +5277,7 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 
 	for_each_sched_entity(se) {
 		cfs_rq = cfs_rq_of(se);
+		dequeue_entity_groups(cfs_rq, se, flags);
 		dequeue_entity(cfs_rq, se, flags);
 
 		/*
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 10/14] sched,fair: add helper functions for flattened runqueue
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (7 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 09/14] sched,fair: refactor enqueue/dequeue_entity Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-22 17:33 ` [PATCH 11/14] sched,fair: flatten hierarchical runqueues Rik van Riel
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel

Add helper functions to make the flattened runqueue patch a little smaller.

The task_se_h_weight function is similar to task_se_h_load, but scales the
task weight by the group weight, without taking the task's duty cycle into
account.

The task_se_in_cgroup helper is functionally identical to parent_entity,
but directly calling a function with that name obscures what the other
code is trying to use it for, and would make the code harder to understand.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 kernel/sched/fair.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index c944729423bd..6073fabb171b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -243,6 +243,7 @@ static u64 __calc_delta(u64 delta_exec, unsigned long weight, struct load_weight
 
 const struct sched_class fair_sched_class;
 static unsigned long task_se_h_load(struct sched_entity *se);
+static unsigned long task_se_h_weight(struct sched_entity *se);
 
 /**************************************************************
  * CFS operations on generic schedulable entities:
@@ -431,6 +432,12 @@ static inline struct sched_entity *parent_entity(struct sched_entity *se)
 	return se->parent;
 }
 
+/* Is this (task) sched_entity in a non-root cgroup? */
+static inline bool task_se_in_cgroup(struct sched_entity *se)
+{
+	return parent_entity(se);
+}
+
 static void
 find_matching_se(struct sched_entity **se, struct sched_entity **pse)
 {
@@ -513,6 +520,11 @@ static inline struct sched_entity *parent_entity(struct sched_entity *se)
 	return NULL;
 }
 
+static inline bool task_se_in_cgroup(struct sched_entity *se)
+{
+	return false;
+}
+
 static inline void
 find_matching_se(struct sched_entity **se, struct sched_entity **pse)
 {
@@ -7835,6 +7847,20 @@ static void update_cfs_rq_h_load(struct cfs_rq *cfs_rq)
 	}
 }
 
+static unsigned long task_se_h_weight(struct sched_entity *se)
+{
+	struct cfs_rq *cfs_rq;
+
+	if (!task_se_in_cgroup(se))
+		return se->load.weight;
+
+	cfs_rq = group_cfs_rq_of_parent(se);
+	update_cfs_rq_h_load(cfs_rq);
+
+	/* Reduce the load.weight by the h_load of the group the task is in. */
+	return (cfs_rq->h_load * se->load.weight) >> SCHED_FIXEDPOINT_SHIFT;
+}
+
 static unsigned long task_se_h_load(struct sched_entity *se)
 {
 	struct cfs_rq *cfs_rq = group_cfs_rq_of_parent(se);
@@ -7871,6 +7897,11 @@ static unsigned long task_se_h_load(struct sched_entity *se)
 {
 	return se->avg.load_avg;
 }
+
+static unsigned long task_se_h_weight(struct sched_entity *se)
+{
+	return se->load.weight;
+}
 #endif
 
 /********** Helpers for find_busiest_group ************************/
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 11/14] sched,fair: flatten hierarchical runqueues
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (8 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 10/14] sched,fair: add helper functions for flattened runqueue Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-22 17:33 ` [PATCH 12/14] sched,fair: track cfs_rq->max_h_load for more legitimate h_weight Rik van Riel
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel

Flatten the hierarchical runqueues into just the per CPU rq.cfs runqueue.

Iteration of the sched_entity hierarchy is rate limited to once per jiffy
per sched_entity, which is a smaller change than it seems, because load
average adjustments were already rate limited to once per jiffy before this
patch series.

This patch breaks CONFIG_CFS_BANDWIDTH. The plan for that is to park tasks
from throttled cgroups onto their cgroup runqueues, and slowly (using the
GENTLE_FAIR_SLEEPERS) wake them back up, in vruntime order, once the cgroup
gets unthrottled, to prevent thundering herd issues.

Signed-off-by: Rik van Riel <riel@surriel.com>

Header from folded patch 'fix-attach-detach_enticy_cfs_rq.patch~':

Subject: sched,fair: fix attach/detach_entity_cfs_rq

While attach_entity_cfs_rq and detach_entity_cfs_rq should iterate over
the hierarchy, they do not need to so that twice.

Passing flags into propagate_entity_cfs_rq allows us to reuse that same
loop from other functions.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 include/linux/sched.h |   2 +
 kernel/sched/fair.c   | 478 +++++++++++++-----------------------------
 kernel/sched/pelt.c   |   6 +-
 kernel/sched/pelt.h   |   2 +-
 kernel/sched/sched.h  |   2 +-
 5 files changed, 150 insertions(+), 340 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 84a6cc6f5c47..901c710363e7 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -453,6 +453,8 @@ struct sched_entity {
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	int				depth;
 	unsigned long			enqueued_h_load;
+	unsigned long			enqueued_h_weight;
+	struct load_weight		h_load;
 	struct sched_entity		*parent;
 	/* rq on which this entity is (to be) queued: */
 	struct cfs_rq			*cfs_rq;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 6073fabb171b..1d5145cb6096 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -470,6 +470,19 @@ find_matching_se(struct sched_entity **se, struct sched_entity **pse)
 	}
 }
 
+/* Add the cgroup cfs_rqs to the list, for update_blocked_averages */
+static void enqueue_entity_cfs_rqs(struct sched_entity *se)
+{
+	SCHED_WARN_ON(!entity_is_task(se));
+
+	for_each_sched_entity(se) {
+		struct cfs_rq *cfs_rq = group_cfs_rq_of_parent(se);
+
+		if (list_add_leaf_cfs_rq(cfs_rq))
+			break;
+	}
+}
+
 #else	/* !CONFIG_FAIR_GROUP_SCHED */
 
 static inline struct task_struct *task_of(struct sched_entity *se)
@@ -697,8 +710,14 @@ int sched_proc_update_handler(struct ctl_table *table, int write,
  */
 static inline u64 calc_delta_fair(u64 delta, struct sched_entity *se)
 {
-	if (unlikely(se->load.weight != NICE_0_LOAD))
+	if (task_se_in_cgroup(se)) {
+		unsigned long h_weight = task_se_h_weight(se);
+		if (h_weight != se->h_load.weight)
+			update_load_set(&se->h_load, h_weight);
+		delta = __calc_delta(delta, NICE_0_LOAD, &se->h_load);
+	} else if (unlikely(se->load.weight != NICE_0_LOAD)) {
 		delta = __calc_delta(delta, NICE_0_LOAD, &se->load);
+	}
 
 	return delta;
 }
@@ -712,22 +731,16 @@ static inline u64 calc_delta_fair(u64 delta, struct sched_entity *se)
 static u64 sched_slice(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
 	u64 slice = sysctl_sched_latency;
+	struct load_weight *load = &cfs_rq->load;
+	struct load_weight lw;
 
-	for_each_sched_entity(se) {
-		struct load_weight *load;
-		struct load_weight lw;
-
-		cfs_rq = cfs_rq_of(se);
-		load = &cfs_rq->load;
-
-		if (unlikely(!se->on_rq)) {
-			lw = cfs_rq->load;
+	if (unlikely(!se->on_rq)) {
+		lw = cfs_rq->load;
 
-			update_load_add(&lw, se->load.weight);
-			load = &lw;
-		}
-		slice = __calc_delta(slice, se->load.weight, load);
+		update_load_add(&lw, task_se_h_weight(se));
+		load = &lw;
 	}
+	slice = __calc_delta(slice, task_se_h_weight(se), load);
 
 	/*
 	 * To avoid cache thrashing, run at least sysctl_sched_min_granularity.
@@ -775,6 +788,7 @@ void init_entity_runnable_average(struct sched_entity *se)
 
 static inline u64 cfs_rq_clock_task(struct cfs_rq *cfs_rq);
 static void attach_entity_cfs_rq(struct sched_entity *se);
+static void propagate_entity_cfs_rq(struct sched_entity *se, int flags);
 
 /*
  * With new tasks being created, their initial util_avgs are extrapolated
@@ -2728,16 +2742,28 @@ static inline void update_scan_period(struct task_struct *p, int new_cpu)
 static void
 account_entity_enqueue(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
-	update_load_add(&cfs_rq->load, se->load.weight);
-	if (!parent_entity(se))
+	struct rq *rq;
+
+	if (task_se_in_cgroup(se)) {
+		struct cfs_rq *cgroup_rq = group_cfs_rq_of_parent(se);
+		unsigned long h_weight;
+
+		update_load_add(&cgroup_rq->load, se->load.weight);
+		cgroup_rq->nr_running++;
+
+		/* Add the hierarchical weight to the CPU rq */
+		h_weight = task_se_h_weight(se);
+		se->enqueued_h_weight = h_weight;
+		update_load_add(&rq_of(cfs_rq)->load, h_weight);
+	} else {
+		update_load_add(&cfs_rq->load, se->load.weight);
 		update_load_add(&rq_of(cfs_rq)->load, se->load.weight);
+	}
 #ifdef CONFIG_SMP
-	if (entity_is_task(se)) {
-		struct rq *rq = rq_of(cfs_rq);
+	rq = rq_of(cfs_rq);
 
-		account_numa_enqueue(rq, task_of(se));
-		list_add(&se->group_node, &rq->cfs_tasks);
-	}
+	account_numa_enqueue(rq, task_of(se));
+	list_add(&se->group_node, &rq->cfs_tasks);
 #endif
 	cfs_rq->nr_running++;
 }
@@ -2745,14 +2771,20 @@ account_entity_enqueue(struct cfs_rq *cfs_rq, struct sched_entity *se)
 static void
 account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
-	update_load_sub(&cfs_rq->load, se->load.weight);
-	if (!parent_entity(se))
+	if (task_se_in_cgroup(se)) {
+		struct cfs_rq *cgroup_rq = group_cfs_rq_of_parent(se);
+
+		update_load_sub(&cgroup_rq->load, se->load.weight);
+		cgroup_rq->nr_running--;
+
+		update_load_sub(&rq_of(cfs_rq)->load, se->enqueued_h_weight);
+	} else {
+		update_load_sub(&cfs_rq->load, se->load.weight);
 		update_load_sub(&rq_of(cfs_rq)->load, se->load.weight);
-#ifdef CONFIG_SMP
-	if (entity_is_task(se)) {
-		account_numa_dequeue(rq_of(cfs_rq), task_of(se));
-		list_del_init(&se->group_node);
 	}
+#ifdef CONFIG_SMP
+	account_numa_dequeue(rq_of(cfs_rq), task_of(se));
+	list_del_init(&se->group_node);
 #endif
 	cfs_rq->nr_running--;
 }
@@ -2847,6 +2879,9 @@ update_runnable_load_avg(struct sched_entity *se)
 static inline void
 enqueue_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
+	if (task_se_in_cgroup(se))
+		cfs_rq = group_cfs_rq_of_parent(se);
+
 	cfs_rq->avg.load_avg += se->avg.load_avg;
 	cfs_rq->avg.load_sum += se_weight(se) * se->avg.load_sum;
 }
@@ -2854,6 +2889,9 @@ enqueue_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
 static inline void
 dequeue_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
+	if (task_se_in_cgroup(se))
+		cfs_rq = group_cfs_rq_of_parent(se);
+
 	sub_positive(&cfs_rq->avg.load_avg, se->avg.load_avg);
 	sub_positive(&cfs_rq->avg.load_sum, se_weight(se) * se->avg.load_sum);
 }
@@ -3480,7 +3518,9 @@ static void attach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 	cfs_rq->avg.util_avg += se->avg.util_avg;
 	cfs_rq->avg.util_sum += se->avg.util_sum;
 
-	add_tg_cfs_propagate(cfs_rq, se->avg.load_sum);
+	if (task_se_in_cgroup(se))
+		add_tg_cfs_propagate(group_cfs_rq_of_parent(se),
+						se->avg.load_sum);
 
 	cfs_rq_util_change(cfs_rq, flags);
 }
@@ -3499,7 +3539,9 @@ static void detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 	sub_positive(&cfs_rq->avg.util_avg, se->avg.util_avg);
 	sub_positive(&cfs_rq->avg.util_sum, se->avg.util_sum);
 
-	add_tg_cfs_propagate(cfs_rq, -se->avg.load_sum);
+	if (task_se_in_cgroup(se))
+		add_tg_cfs_propagate(group_cfs_rq_of_parent(se),
+						-se->avg.load_sum);
 
 	cfs_rq_util_change(cfs_rq, 0);
 }
@@ -3510,11 +3552,13 @@ static void detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 #define UPDATE_TG	0x1
 #define SKIP_AGE_LOAD	0x2
 #define DO_ATTACH	0x4
+#define SE_IS_CURRENT	0x8
 
 /* Update task and its cfs_rq load average */
 static inline bool update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
 	u64 now = cfs_rq_clock_pelt(cfs_rq);
+	bool curr = flags & SE_IS_CURRENT;
 	int decayed;
 
 	/*
@@ -3522,7 +3566,7 @@ static inline bool update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 	 * track group sched_entity load average for task_se_h_load calc in migration
 	 */
 	if (se->avg.last_update_time && !(flags & SKIP_AGE_LOAD))
-		__update_load_avg_se(now, cfs_rq, se);
+		__update_load_avg_se(now, cfs_rq, se, curr, curr);
 
 	decayed  = update_cfs_rq_load_avg(now, cfs_rq);
 	decayed |= propagate_entity_load_avg(se);
@@ -3588,6 +3632,9 @@ static void remove_entity_load_avg(struct sched_entity *se)
 	struct cfs_rq *cfs_rq = cfs_rq_of(se);
 	unsigned long flags;
 
+	if (task_se_in_cgroup(se))
+		cfs_rq = group_cfs_rq_of_parent(se);
+
 	/*
 	 * tasks cannot exit without having gone through wake_up_new_task() ->
 	 * post_init_entity_util_avg() which will have added things to the
@@ -3758,6 +3805,7 @@ static inline void update_misfit_status(struct task_struct *p, struct rq *rq)
 #define UPDATE_TG	0x0
 #define SKIP_AGE_LOAD	0x0
 #define DO_ATTACH	0x0
+#define SE_IS_CURRENT	0x0
 
 static inline bool update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int not_used1)
 {
@@ -3885,24 +3933,6 @@ static inline void check_schedstat_required(void)
  * CPU and an up-to-date min_vruntime on the destination CPU.
  */
 
-static bool
-enqueue_entity_groups(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
-{
-	/*
-	 * When enqueuing a sched_entity, we must:
-	 *   - Update loads to have both entity and cfs_rq synced with now.
-	 *   - Add its load to cfs_rq->runnable_avg
-	 *   - For group_entity, update its weight to reflect the new share of
-	 *     its group cfs_rq
-	 *   - Add its new weight to cfs_rq->load.weight
-	 */
-	if (!update_load_avg(cfs_rq, se, UPDATE_TG | DO_ATTACH))
-		return false;
-
-	update_cfs_group(se);
-	return true;
-}
-
 static void
 enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
@@ -3940,77 +3970,24 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 		__enqueue_entity(cfs_rq, se);
 	se->on_rq = 1;
 
-	if (cfs_rq->nr_running == 1) {
-		list_add_leaf_cfs_rq(cfs_rq);
-		check_enqueue_throttle(cfs_rq);
-	}
-}
-
-static void __clear_buddies_last(struct sched_entity *se)
-{
-	for_each_sched_entity(se) {
-		struct cfs_rq *cfs_rq = cfs_rq_of(se);
-		if (cfs_rq->last != se)
-			break;
-
-		cfs_rq->last = NULL;
-	}
-}
-
-static void __clear_buddies_next(struct sched_entity *se)
-{
-	for_each_sched_entity(se) {
-		struct cfs_rq *cfs_rq = cfs_rq_of(se);
-		if (cfs_rq->next != se)
-			break;
-
-		cfs_rq->next = NULL;
-	}
-}
-
-static void __clear_buddies_skip(struct sched_entity *se)
-{
-	for_each_sched_entity(se) {
-		struct cfs_rq *cfs_rq = cfs_rq_of(se);
-		if (cfs_rq->skip != se)
-			break;
-
-		cfs_rq->skip = NULL;
-	}
+	if (task_se_in_cgroup(se))
+		enqueue_entity_cfs_rqs(se);
 }
 
 static void clear_buddies(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
 	if (cfs_rq->last == se)
-		__clear_buddies_last(se);
+		cfs_rq->last = NULL;
 
 	if (cfs_rq->next == se)
-		__clear_buddies_next(se);
+		cfs_rq->next = NULL;
 
 	if (cfs_rq->skip == se)
-		__clear_buddies_skip(se);
+		cfs_rq->skip = NULL;
 }
 
 static __always_inline void return_cfs_rq_runtime(struct cfs_rq *cfs_rq);
 
-static bool
-dequeue_entity_groups(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
-{
-	/*
-	 * When dequeuing a sched_entity, we must:
-	 *   - Update loads to have both entity and cfs_rq synced with now.
-	 *   - Subtract its load from the cfs_rq->runnable_avg.
-	 *   - Subtract its previous weight from cfs_rq->load.weight.
-	 *   - For group entity, update its weight to reflect the new share
-	 *     of its group cfs_rq.
-	 */
-	if (!update_load_avg(cfs_rq, se, UPDATE_TG))
-		return false;
-	update_cfs_group(se);
-
-	return true;
-}
-
 static void
 dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
@@ -4104,7 +4081,7 @@ set_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
 		 */
 		update_stats_wait_end(cfs_rq, se);
 		__dequeue_entity(cfs_rq, se);
-		update_load_avg(cfs_rq, se, UPDATE_TG);
+		propagate_entity_cfs_rq(se, UPDATE_TG);
 	}
 
 	update_stats_curr_start(cfs_rq, se);
@@ -4202,11 +4179,12 @@ static void put_prev_entity(struct cfs_rq *cfs_rq, struct sched_entity *prev)
 	check_spread(cfs_rq, prev);
 
 	if (prev->on_rq) {
+		struct sched_entity *se = prev;
 		update_stats_wait_start(cfs_rq, prev);
 		/* Put 'current' back into the tree. */
 		__enqueue_entity(cfs_rq, prev);
 		/* in !on_rq case, update occurred at dequeue */
-		update_load_avg(cfs_rq, prev, 0);
+		propagate_entity_cfs_rq(se, SE_IS_CURRENT);
 	}
 	cfs_rq->curr = NULL;
 }
@@ -4222,7 +4200,7 @@ entity_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr, int queued)
 	/*
 	 * Ensure that runnable average is periodically updated.
 	 */
-	update_load_avg(cfs_rq, curr, UPDATE_TG);
+	update_load_avg(cfs_rq, curr, UPDATE_TG|SE_IS_CURRENT);
 	update_cfs_group(curr);
 
 #ifdef CONFIG_SCHED_HRTICK
@@ -4241,9 +4219,6 @@ entity_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr, int queued)
 			hrtimer_active(&rq_of(cfs_rq)->hrtick_timer))
 		return;
 #endif
-
-	if (cfs_rq->nr_running > 1)
-		check_preempt_tick(cfs_rq, curr);
 }
 
 
@@ -5118,7 +5093,7 @@ static void hrtick_start_fair(struct rq *rq, struct task_struct *p)
 
 	SCHED_WARN_ON(task_rq(p) != rq);
 
-	if (rq->cfs.h_nr_running > 1) {
+	if (rq->cfs.nr_running > 1) {
 		u64 slice = sched_slice(cfs_rq, se);
 		u64 ran = se->sum_exec_runtime - se->prev_sum_exec_runtime;
 		s64 delta = slice - ran;
@@ -5183,7 +5158,7 @@ static inline void update_overutilized_status(struct rq *rq) { }
 static void
 enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 {
-	struct cfs_rq *cfs_rq;
+	struct cfs_rq *cfs_rq = & rq->cfs;
 	struct sched_entity *se = &p->se;
 
 	/*
@@ -5192,7 +5167,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	 * Let's add the task's estimated utilization to the cfs_rq's
 	 * estimated utilization, before we update schedutil.
 	 */
-	util_est_enqueue(&rq->cfs, p);
+	util_est_enqueue(cfs_rq, p);
 
 	/*
 	 * If in_iowait is set, the code below may not trigger any cpufreq
@@ -5202,38 +5177,10 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	if (p->in_iowait)
 		cpufreq_update_util(rq, SCHED_CPUFREQ_IOWAIT);
 
-	for_each_sched_entity(se) {
-		if (se->on_rq)
-			break;
-		cfs_rq = cfs_rq_of(se);
-		enqueue_entity_groups(cfs_rq, se, flags);
-		enqueue_entity(cfs_rq, se, flags);
+	propagate_entity_cfs_rq(se, UPDATE_TG|DO_ATTACH);
 
-		/*
-		 * end evaluation on encountering a throttled cfs_rq
-		 *
-		 * note: in the case of encountering a throttled cfs_rq we will
-		 * post the final h_nr_running increment below.
-		 */
-		if (cfs_rq_throttled(cfs_rq))
-			break;
-		cfs_rq->h_nr_running++;
+	enqueue_entity(cfs_rq, se, flags);
 
-		flags = ENQUEUE_WAKEUP;
-	}
-
-	for_each_sched_entity(se) {
-		cfs_rq = cfs_rq_of(se);
-		cfs_rq->h_nr_running++;
-
-		if (cfs_rq_throttled(cfs_rq))
-			break;
-
-		update_load_avg(cfs_rq, se, UPDATE_TG);
-		update_cfs_group(se);
-	}
-
-	if (!se) {
 		add_nr_running(rq, 1);
 		/*
 		 * Since new tasks are assigned an initial util_avg equal to
@@ -5252,23 +5199,6 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 		if (flags & ENQUEUE_WAKEUP)
 			update_overutilized_status(rq);
 
-	}
-
-	if (cfs_bandwidth_used()) {
-		/*
-		 * When bandwidth control is enabled; the cfs_rq_throttled()
-		 * breaks in the above iteration can result in incomplete
-		 * leaf list maintenance, resulting in triggering the assertion
-		 * below.
-		 */
-		for_each_sched_entity(se) {
-			cfs_rq = cfs_rq_of(se);
-
-			if (list_add_leaf_cfs_rq(cfs_rq))
-				break;
-		}
-	}
-
 	assert_list_leaf_cfs_rq(rq);
 
 	hrtick_update(rq);
@@ -5283,55 +5213,17 @@ static void set_next_buddy(struct sched_entity *se);
  */
 static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 {
-	struct cfs_rq *cfs_rq;
+	struct cfs_rq *cfs_rq = &rq->cfs;
 	struct sched_entity *se = &p->se;
 	int task_sleep = flags & DEQUEUE_SLEEP;
 
-	for_each_sched_entity(se) {
-		cfs_rq = cfs_rq_of(se);
-		dequeue_entity_groups(cfs_rq, se, flags);
-		dequeue_entity(cfs_rq, se, flags);
-
-		/*
-		 * end evaluation on encountering a throttled cfs_rq
-		 *
-		 * note: in the case of encountering a throttled cfs_rq we will
-		 * post the final h_nr_running decrement below.
-		*/
-		if (cfs_rq_throttled(cfs_rq))
-			break;
-		cfs_rq->h_nr_running--;
-
-		/* Don't dequeue parent if it has other entities besides us */
-		if (cfs_rq->load.weight) {
-			/* Avoid re-evaluating load for this entity: */
-			se = parent_entity(se);
-			/*
-			 * Bias pick_next to pick a task from this cfs_rq, as
-			 * p is sleeping when it is within its sched_slice.
-			 */
-			if (task_sleep && se && !throttled_hierarchy(cfs_rq))
-				set_next_buddy(se);
-			break;
-		}
-		flags |= DEQUEUE_SLEEP;
-	}
-
-	for_each_sched_entity(se) {
-		cfs_rq = cfs_rq_of(se);
-		cfs_rq->h_nr_running--;
-
-		if (cfs_rq_throttled(cfs_rq))
-			break;
+	propagate_entity_cfs_rq(se, UPDATE_TG|SE_IS_CURRENT);
 
-		update_load_avg(cfs_rq, se, UPDATE_TG);
-		update_cfs_group(se);
-	}
+	dequeue_entity(cfs_rq, &p->se, flags);
 
-	if (!se)
-		sub_nr_running(rq, 1);
+	sub_nr_running(rq, 1);
 
-	util_est_dequeue(&rq->cfs, p, task_sleep);
+	util_est_dequeue(cfs_rq, p, task_sleep);
 	hrtick_update(rq);
 }
 
@@ -5654,7 +5546,7 @@ static unsigned long capacity_of(int cpu)
 static unsigned long cpu_avg_load_per_task(int cpu)
 {
 	struct rq *rq = cpu_rq(cpu);
-	unsigned long nr_running = READ_ONCE(rq->cfs.h_nr_running);
+	unsigned long nr_running = READ_ONCE(rq->cfs.nr_running);
 	unsigned long load_avg = weighted_cpuload(rq);
 
 	if (nr_running)
@@ -6873,11 +6765,9 @@ static void set_last_buddy(struct sched_entity *se)
 	if (entity_is_task(se) && unlikely(task_has_idle_policy(task_of(se))))
 		return;
 
-	for_each_sched_entity(se) {
-		if (SCHED_WARN_ON(!se->on_rq))
-			return;
-		cfs_rq_of(se)->last = se;
-	}
+	if (SCHED_WARN_ON(!se->on_rq))
+		return;
+	cfs_rq_of(se)->last = se;
 }
 
 static void set_next_buddy(struct sched_entity *se)
@@ -6885,17 +6775,14 @@ static void set_next_buddy(struct sched_entity *se)
 	if (entity_is_task(se) && unlikely(task_has_idle_policy(task_of(se))))
 		return;
 
-	for_each_sched_entity(se) {
-		if (SCHED_WARN_ON(!se->on_rq))
-			return;
-		cfs_rq_of(se)->next = se;
-	}
+	if (SCHED_WARN_ON(!se->on_rq))
+		return;
+	cfs_rq_of(se)->next = se;
 }
 
 static void set_skip_buddy(struct sched_entity *se)
 {
-	for_each_sched_entity(se)
-		cfs_rq_of(se)->skip = se;
+	cfs_rq_of(se)->skip = se;
 }
 
 /*
@@ -6951,7 +6838,6 @@ static void check_preempt_wakeup(struct rq *rq, struct task_struct *p, int wake_
 	if (unlikely(p->policy != SCHED_NORMAL) || !sched_feat(WAKEUP_PREEMPTION))
 		return;
 
-	find_matching_se(&se, &pse);
 	update_curr(cfs_rq_of(se));
 	BUG_ON(!pse);
 	if (wakeup_preempt_entity(se, pse) == 1) {
@@ -6992,100 +6878,18 @@ pick_next_task_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf
 	struct task_struct *p;
 	int new_tasks;
 
+	put_prev_task(rq, prev);
 again:
 	if (!cfs_rq->nr_running)
 		goto idle;
 
-#ifdef CONFIG_FAIR_GROUP_SCHED
-	if (prev->sched_class != &fair_sched_class)
-		goto simple;
-
-	/*
-	 * Because of the set_next_buddy() in dequeue_task_fair() it is rather
-	 * likely that a next task is from the same cgroup as the current.
-	 *
-	 * Therefore attempt to avoid putting and setting the entire cgroup
-	 * hierarchy, only change the part that actually changes.
-	 */
-
-	do {
-		struct sched_entity *curr = cfs_rq->curr;
-
-		/*
-		 * Since we got here without doing put_prev_entity() we also
-		 * have to consider cfs_rq->curr. If it is still a runnable
-		 * entity, update_curr() will update its vruntime, otherwise
-		 * forget we've ever seen it.
-		 */
-		if (curr) {
-			if (curr->on_rq)
-				update_curr(cfs_rq);
-			else
-				curr = NULL;
-
-			/*
-			 * This call to check_cfs_rq_runtime() will do the
-			 * throttle and dequeue its entity in the parent(s).
-			 * Therefore the nr_running test will indeed
-			 * be correct.
-			 */
-			if (unlikely(check_cfs_rq_runtime(cfs_rq))) {
-				cfs_rq = &rq->cfs;
-
-				if (!cfs_rq->nr_running)
-					goto idle;
-
-				goto simple;
-			}
-		}
-
-		se = pick_next_entity(cfs_rq, curr);
-		cfs_rq = group_cfs_rq(se);
-	} while (cfs_rq);
-
-	p = task_of(se);
-
-	/*
-	 * Since we haven't yet done put_prev_entity and if the selected task
-	 * is a different task than we started out with, try and touch the
-	 * least amount of cfs_rqs.
-	 */
-	if (prev != p) {
-		struct sched_entity *pse = &prev->se;
-
-		while (!(cfs_rq = is_same_group(se, pse))) {
-			int se_depth = se->depth;
-			int pse_depth = pse->depth;
-
-			if (se_depth <= pse_depth) {
-				put_prev_entity(cfs_rq_of(pse), pse);
-				pse = parent_entity(pse);
-			}
-			if (se_depth >= pse_depth) {
-				set_next_entity(cfs_rq_of(se), se);
-				se = parent_entity(se);
-			}
-		}
-
-		put_prev_entity(cfs_rq, pse);
-		set_next_entity(cfs_rq, se);
-	}
-
-	goto done;
-simple:
-#endif
-
-	put_prev_task(rq, prev);
-
-	do {
-		se = pick_next_entity(cfs_rq, NULL);
-		set_next_entity(cfs_rq, se);
-		cfs_rq = group_cfs_rq(se);
-	} while (cfs_rq);
+	se = pick_next_entity(cfs_rq, NULL);
+	if (!se)
+		goto idle;
 
+	set_next_entity(cfs_rq, se);
 	p = task_of(se);
 
-done: __maybe_unused;
 #ifdef CONFIG_SMP
 	/*
 	 * Move the next running task to the front of
@@ -7134,10 +6938,8 @@ static void put_prev_task_fair(struct rq *rq, struct task_struct *prev)
 	struct sched_entity *se = &prev->se;
 	struct cfs_rq *cfs_rq;
 
-	for_each_sched_entity(se) {
-		cfs_rq = cfs_rq_of(se);
-		put_prev_entity(cfs_rq, se);
-	}
+	cfs_rq = cfs_rq_of(se);
+	put_prev_entity(cfs_rq, se);
 }
 
 /*
@@ -7898,6 +7700,11 @@ static unsigned long task_se_h_load(struct sched_entity *se)
 	return se->avg.load_avg;
 }
 
+static unsigned long task_se_h_weight(struct sched_entity *se)
+{
+	return se->load.weight;
+}
+
 static unsigned long task_se_h_weight(struct sched_entity *se)
 {
 	return se->load.weight;
@@ -8303,7 +8110,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
 
 		sgs->group_load += load;
 		sgs->group_util += cpu_util(i);
-		sgs->sum_nr_running += rq->cfs.h_nr_running;
+		sgs->sum_nr_running += rq->cfs.nr_running;
 
 		nr_running = rq->nr_running;
 		if (nr_running > 1)
@@ -8994,7 +8801,7 @@ voluntary_active_balance(struct lb_env *env)
 	 * available on dst_cpu.
 	 */
 	if ((env->idle != CPU_NOT_IDLE) &&
-	    (env->src_rq->cfs.h_nr_running == 1)) {
+	    (env->src_rq->cfs.nr_running == 1)) {
 		if ((check_cpu_capacity(env->src_rq, sd)) &&
 		    (capacity_of(env->src_cpu)*sd->imbalance_pct < capacity_of(env->dst_cpu)*100))
 			return 1;
@@ -9675,7 +9482,7 @@ static void nohz_balancer_kick(struct rq *rq)
 		 * capacity; kick the ILB to see if there's a better CPU to run
 		 * on.
 		 */
-		if (rq->cfs.h_nr_running >= 1 && check_cpu_capacity(rq, sd)) {
+		if (rq->cfs.nr_running >= 1 && check_cpu_capacity(rq, sd)) {
 			flags = NOHZ_KICK_MASK;
 			goto unlock;
 		}
@@ -10124,7 +9931,7 @@ static int idle_balance(struct rq *this_rq, struct rq_flags *rf)
 	 * have been enqueued in the meantime. Since we're not going idle,
 	 * pretend we pulled a task.
 	 */
-	if (this_rq->cfs.h_nr_running && !pulled_task)
+	if (this_rq->cfs.nr_running && !pulled_task)
 		pulled_task = 1;
 
 	/* Move the next balance forward */
@@ -10132,7 +9939,7 @@ static int idle_balance(struct rq *this_rq, struct rq_flags *rf)
 		this_rq->next_balance = next_balance;
 
 	/* Is there a task of a high priority class? */
-	if (this_rq->nr_running != this_rq->cfs.h_nr_running)
+	if (this_rq->nr_running != this_rq->cfs.nr_running)
 		pulled_task = -1;
 
 	if (pulled_task)
@@ -10219,6 +10026,10 @@ static void task_tick_fair(struct rq *rq, struct task_struct *curr, int queued)
 		entity_tick(cfs_rq, se, queued);
 	}
 
+	cfs_rq = &rq->cfs;
+	if (cfs_rq->nr_running > 1)
+		check_preempt_tick(cfs_rq, &curr->se);
+
 	if (static_branch_unlikely(&sched_numa_balancing))
 		task_tick_numa(rq, curr);
 
@@ -10317,40 +10128,40 @@ static inline bool vruntime_normalized(struct task_struct *p)
  * Propagate the changes of the sched_entity across the tg tree to make it
  * visible to the root
  */
-static void propagate_entity_cfs_rq(struct sched_entity *se)
+static void propagate_entity_cfs_rq(struct sched_entity *se, int flags)
 {
 	struct cfs_rq *cfs_rq;
 
-	/* Start to propagate at parent */
-	se = se->parent;
-
 	for_each_sched_entity(se) {
-		cfs_rq = cfs_rq_of(se);
+		cfs_rq = group_cfs_rq_of_parent(se);
 
 		if (cfs_rq_throttled(cfs_rq))
 			break;
 
-		update_load_avg(cfs_rq, se, UPDATE_TG);
+		if (!update_load_avg(cfs_rq, se, flags))
+			break;
+
+		update_cfs_group(se);
 	}
 }
 #else
-static void propagate_entity_cfs_rq(struct sched_entity *se) { }
+static void propagate_entity_cfs_rq(struct sched_entity *se, int flags) { }
 #endif
 
 static void detach_entity_cfs_rq(struct sched_entity *se)
 {
-	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+	struct cfs_rq *cfs_rq = group_cfs_rq_of_parent(se);
 
-	/* Catch up with the cfs_rq and remove our load when we leave */
 	update_load_avg(cfs_rq, se, 0);
 	detach_entity_load_avg(cfs_rq, se);
 	update_tg_load_avg(cfs_rq, false);
-	propagate_entity_cfs_rq(se);
+	propagate_entity_cfs_rq(se->parent, UPDATE_TG);
 }
 
 static void attach_entity_cfs_rq(struct sched_entity *se)
 {
-	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+	int flags = sched_feat(ATTACH_AGE_LOAD) ? 0 : SKIP_AGE_LOAD;
+	struct cfs_rq *cfs_rq = group_cfs_rq_of_parent(se);
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	/*
@@ -10361,10 +10172,10 @@ static void attach_entity_cfs_rq(struct sched_entity *se)
 #endif
 
 	/* Synchronize entity with its cfs_rq */
-	update_load_avg(cfs_rq, se, sched_feat(ATTACH_AGE_LOAD) ? 0 : SKIP_AGE_LOAD);
+	update_load_avg(cfs_rq, se, flags);
 	attach_entity_load_avg(cfs_rq, se, 0);
 	update_tg_load_avg(cfs_rq, false);
-	propagate_entity_cfs_rq(se);
+	propagate_entity_cfs_rq(se->parent, flags | UPDATE_TG);
 }
 
 static void detach_task_cfs_rq(struct task_struct *p)
@@ -10425,14 +10236,11 @@ static void switched_to_fair(struct rq *rq, struct task_struct *p)
 static void set_curr_task_fair(struct rq *rq)
 {
 	struct sched_entity *se = &rq->curr->se;
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
 
-	for_each_sched_entity(se) {
-		struct cfs_rq *cfs_rq = cfs_rq_of(se);
-
-		set_next_entity(cfs_rq, se);
-		/* ensure bandwidth has been allocated on our new cfs_rq */
-		account_cfs_rq_runtime(cfs_rq, 0);
-	}
+	set_next_entity(cfs_rq, se);
+	/* ensure bandwidth has been allocated on our new cfs_rq */
+	account_cfs_rq_runtime(cfs_rq, 0);
 }
 
 void init_cfs_rq(struct cfs_rq *cfs_rq)
diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c
index 190797e421dd..2c901e0f9fbd 100644
--- a/kernel/sched/pelt.c
+++ b/kernel/sched/pelt.c
@@ -266,10 +266,10 @@ int __update_load_avg_blocked_se(u64 now, struct sched_entity *se)
 	return 0;
 }
 
-int __update_load_avg_se(u64 now, struct cfs_rq *cfs_rq, struct sched_entity *se)
+int __update_load_avg_se(u64 now, struct cfs_rq *cfs_rq, struct sched_entity *se, bool load, bool running)
 {
-	if (___update_load_sum(now, &se->avg, !!se->on_rq,
-				cfs_rq->curr == se)) {
+	if (___update_load_sum(now, &se->avg, (!!se->on_rq || load),
+				(cfs_rq->curr == se) || running)) {
 
 		___update_load_avg(&se->avg, se_weight(se));
 		cfs_se_util_change(&se->avg);
diff --git a/kernel/sched/pelt.h b/kernel/sched/pelt.h
index 7489d5f56960..1152c4ebf314 100644
--- a/kernel/sched/pelt.h
+++ b/kernel/sched/pelt.h
@@ -2,7 +2,7 @@
 #include "sched-pelt.h"
 
 int __update_load_avg_blocked_se(u64 now, struct sched_entity *se);
-int __update_load_avg_se(u64 now, struct cfs_rq *cfs_rq, struct sched_entity *se);
+int __update_load_avg_se(u64 now, struct cfs_rq *cfs_rq, struct sched_entity *se, bool load, bool running);
 int __update_load_avg_cfs_rq(u64 now, struct cfs_rq *cfs_rq);
 int update_rt_rq_load_avg(u64 now, struct rq *rq, int running);
 int update_dl_rq_load_avg(u64 now, struct rq *rq, int running);
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 32978a8de8ce..b05fd87cf8b5 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1443,7 +1443,7 @@ static inline void set_task_rq(struct task_struct *p, unsigned int cpu)
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	set_task_rq_fair(&p->se, p->se.cfs_rq, tg->cfs_rq[cpu]);
-	p->se.cfs_rq = tg->cfs_rq[cpu];
+	p->se.cfs_rq = &cpu_rq(cpu)->cfs;
 	p->se.parent = tg->se[cpu];
 #endif
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 12/14] sched,fair: track cfs_rq->max_h_load for more legitimate h_weight
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (9 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 11/14] sched,fair: flatten hierarchical runqueues Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-22 17:33 ` [PATCH 13/14] sched,fair: flatten update_curr functionality Rik van Riel
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel,
	Josef Bacik

The cfs_rq->h_load decays to zero quite rapidly, which can easily lead
to tasks getting enqueued with an h_weight and h_load of zero, confusing
the load balancer, the preemption code, and other parts of the kernel.

Add a slow moving cfs_rq->max_h_load, with a half life of just over a minute,
to help tasks get enqueued with more legitimate h_weight values.

Suggested-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 kernel/sched/debug.c |  2 ++
 kernel/sched/fair.c  |  6 +++++-
 kernel/sched/pelt.c  | 16 ++++++++++++++++
 kernel/sched/pelt.h  |  1 +
 kernel/sched/sched.h |  2 ++
 5 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 6e7c8ff210a8..88ed6d4429e6 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -553,6 +553,8 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
 			atomic_long_read(&cfs_rq->tg->load_avg));
 	SEQ_printf(m, "  .%-30s: %lu\n", "h_load",
 			cfs_rq->h_load);
+	SEQ_printf(m, "  .%-30s: %lu\n", "max_h_load",
+			cfs_rq->max_h_load);
 #endif
 #endif
 #ifdef CONFIG_CFS_BANDWIDTH
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1d5145cb6096..224cd9b20887 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7636,6 +7636,8 @@ static void update_cfs_rq_h_load(struct cfs_rq *cfs_rq)
 
 	if (!se) {
 		cfs_rq->h_load = cfs_rq_load_avg(cfs_rq);
+		slow_decay(&cfs_rq->max_h_load, &cfs_rq->last_max_h_decay);
+		cfs_rq->max_h_load = max(cfs_rq->max_h_load, cfs_rq->h_load);
 		cfs_rq->last_h_load_update = now;
 	}
 
@@ -7645,6 +7647,8 @@ static void update_cfs_rq_h_load(struct cfs_rq *cfs_rq)
 			cfs_rq_load_avg(cfs_rq) + 1);
 		cfs_rq = group_cfs_rq(se);
 		cfs_rq->h_load = load;
+		slow_decay(&cfs_rq->max_h_load, &cfs_rq->last_max_h_decay);
+		cfs_rq->max_h_load = max(load, cfs_rq->max_h_load);
 		cfs_rq->last_h_load_update = now;
 	}
 }
@@ -7660,7 +7664,7 @@ static unsigned long task_se_h_weight(struct sched_entity *se)
 	update_cfs_rq_h_load(cfs_rq);
 
 	/* Reduce the load.weight by the h_load of the group the task is in. */
-	return (cfs_rq->h_load * se->load.weight) >> SCHED_FIXEDPOINT_SHIFT;
+	return (cfs_rq->max_h_load * se->load.weight) >> SCHED_FIXEDPOINT_SHIFT;
 }
 
 static unsigned long task_se_h_load(struct sched_entity *se)
diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c
index 2c901e0f9fbd..2173db8c6209 100644
--- a/kernel/sched/pelt.c
+++ b/kernel/sched/pelt.c
@@ -58,6 +58,22 @@ static u64 decay_load(u64 val, u64 n)
 	return val;
 }
 
+/*
+ * Like above, with a period of close to a minute for very slow decay.
+ * At 2 seconds per period, LOAD_AVG_PERIOD periods is just over a minute.
+ */
+void slow_decay(unsigned long *load, unsigned long *time)
+{
+	unsigned long periods = (jiffies - *time) / 2;
+
+	if (!periods)
+		return;
+
+	*time += periods * 2;
+
+	*load = decay_load(*load, periods);
+}
+
 static u32 __accumulate_pelt_segments(u64 periods, u32 d1, u32 d3)
 {
 	u32 c1, c2, c3 = d3; /* y^0 == 1 */
diff --git a/kernel/sched/pelt.h b/kernel/sched/pelt.h
index 1152c4ebf314..f16042e2aa06 100644
--- a/kernel/sched/pelt.h
+++ b/kernel/sched/pelt.h
@@ -6,6 +6,7 @@ int __update_load_avg_se(u64 now, struct cfs_rq *cfs_rq, struct sched_entity *se
 int __update_load_avg_cfs_rq(u64 now, struct cfs_rq *cfs_rq);
 int update_rt_rq_load_avg(u64 now, struct rq *rq, int running);
 int update_dl_rq_load_avg(u64 now, struct rq *rq, int running);
+void slow_decay(unsigned long *load, unsigned long *time);
 
 #ifdef CONFIG_HAVE_SCHED_AVG_IRQ
 int update_irq_load_avg(struct rq *rq, u64 running);
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index b05fd87cf8b5..b627ca594f97 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -540,6 +540,8 @@ struct cfs_rq {
 	 * this group.
 	 */
 	unsigned long		h_load;
+	unsigned long		max_h_load;
+	unsigned long		last_max_h_decay;
 	u64			last_h_load_update;
 	struct sched_entity	*h_load_next;
 #endif /* CONFIG_FAIR_GROUP_SCHED */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 13/14] sched,fair: flatten update_curr functionality
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (10 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 12/14] sched,fair: track cfs_rq->max_h_load for more legitimate h_weight Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-22 17:33 ` [PATCH 14/14] sched,fair: propagate sum_exec_runtime up the hierarchy Rik van Riel
  2019-07-30 16:29 ` [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Peter Zijlstra
  13 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel

Make it clear that update_curr only works on tasks any more.

There is no need for task_tick_fair to call it on every sched entity up
the hierarchy, so move the call out of entity_tick.

Signed-off-by: Rik van Riel <riel@surriel.com>`

Header from folded patch 'fix-attach-detach_enticy_cfs_rq.patch':

Subject: sched,fair: fix attach/detach_entity_cfs_rq

While attach_entity_cfs_rq and detach_entity_cfs_rq should iterate over
the hierarchy, they do not need to so that twice.

Passing flags into propagate_entity_cfs_rq allows us to reuse that same
loop from other functions.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 kernel/sched/fair.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 224cd9b20887..4c7e1818efba 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -872,10 +872,11 @@ static void update_tg_load_avg(struct cfs_rq *cfs_rq, int force)
 static void update_curr(struct cfs_rq *cfs_rq)
 {
 	struct sched_entity *curr = cfs_rq->curr;
+	struct task_struct *curtask;
 	u64 now = rq_clock_task(rq_of(cfs_rq));
 	u64 delta_exec;
 
-	if (unlikely(!curr))
+	if (unlikely(!curr) || !entity_is_task(curr))
 		return;
 
 	delta_exec = now - curr->exec_start;
@@ -893,13 +894,10 @@ static void update_curr(struct cfs_rq *cfs_rq)
 	curr->vruntime += calc_delta_fair(delta_exec, curr);
 	update_min_vruntime(cfs_rq);
 
-	if (entity_is_task(curr)) {
-		struct task_struct *curtask = task_of(curr);
-
-		trace_sched_stat_runtime(curtask, delta_exec, curr->vruntime);
-		cgroup_account_cputime(curtask, delta_exec);
-		account_group_exec_runtime(curtask, delta_exec);
-	}
+	curtask = task_of(curr);
+	trace_sched_stat_runtime(curtask, delta_exec, curr->vruntime);
+	cgroup_account_cputime(curtask, delta_exec);
+	account_group_exec_runtime(curtask, delta_exec);
 
 	account_cfs_rq_runtime(cfs_rq, delta_exec);
 }
@@ -4192,11 +4190,6 @@ static void put_prev_entity(struct cfs_rq *cfs_rq, struct sched_entity *prev)
 static void
 entity_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr, int queued)
 {
-	/*
-	 * Update run-time statistics of the 'current'.
-	 */
-	update_curr(cfs_rq);
-
 	/*
 	 * Ensure that runnable average is periodically updated.
 	 */
@@ -10025,6 +10018,11 @@ static void task_tick_fair(struct rq *rq, struct task_struct *curr, int queued)
 	struct cfs_rq *cfs_rq;
 	struct sched_entity *se = &curr->se;
 
+	/*
+	 * Update run-time statistics of the 'current'.
+	 */
+	update_curr(&rq->cfs);
+
 	for_each_sched_entity(se) {
 		cfs_rq = group_cfs_rq_of_parent(se);
 		entity_tick(cfs_rq, se, queued);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 14/14] sched,fair: propagate sum_exec_runtime up the hierarchy
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (11 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 13/14] sched,fair: flatten update_curr functionality Rik van Riel
@ 2019-07-22 17:33 ` Rik van Riel
  2019-07-30 16:29 ` [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Peter Zijlstra
  13 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-22 17:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, pjt, dietmar.eggemann, peterz, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot, Rik van Riel

Now that enqueue_task_fair and dequeue_task_fair no longer iterate up
the hierarchy all the time, a method to lazily propagate sum_exec_runtime
up the hierarchy is necessary.

Once a tick, propagate the newly accumulated exec_runtime up the hierarchy,
and feed it into CFS bandwidth control.

Remove the pointless call to account_cfs_rq_runtime from update_curr,
which is always called with a root cfs_rq.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 include/linux/sched.h |  1 +
 kernel/sched/core.c   |  1 +
 kernel/sched/fair.c   | 23 +++++++++++++++++++++--
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 901c710363e7..bdca15b3afe7 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -454,6 +454,7 @@ struct sched_entity {
 	int				depth;
 	unsigned long			enqueued_h_load;
 	unsigned long			enqueued_h_weight;
+	u64				propagated_exec_runtime;
 	struct load_weight		h_load;
 	struct sched_entity		*parent;
 	/* rq on which this entity is (to be) queued: */
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index fbd96900f715..9915d20e84a9 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2137,6 +2137,7 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p)
 	INIT_LIST_HEAD(&p->se.group_node);
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
+	p->se.propagated_exec_runtime	= 0;
 	p->se.cfs_rq			= NULL;
 #endif
 
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4c7e1818efba..0bb8a7e92f07 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -898,8 +898,6 @@ static void update_curr(struct cfs_rq *cfs_rq)
 	trace_sched_stat_runtime(curtask, delta_exec, curr->vruntime);
 	cgroup_account_cputime(curtask, delta_exec);
 	account_group_exec_runtime(curtask, delta_exec);
-
-	account_cfs_rq_runtime(cfs_rq, delta_exec);
 }
 
 static void update_curr_fair(struct rq *rq)
@@ -3405,6 +3403,20 @@ static inline bool skip_blocked_update(struct sched_entity *se)
 	return true;
 }
 
+static void propagate_exec_runtime(struct cfs_rq *cfs_rq,
+				struct sched_entity *se)
+{
+	struct sched_entity *parent = se->parent;
+	u64 diff = se->sum_exec_runtime - se->propagated_exec_runtime;
+
+	if (parent) {
+		parent->sum_exec_runtime += diff;
+		account_cfs_rq_runtime(cfs_rq, diff);
+	}
+
+	se->propagated_exec_runtime = se->sum_exec_runtime;
+}
+
 #else /* CONFIG_FAIR_GROUP_SCHED */
 
 static inline void update_tg_load_avg(struct cfs_rq *cfs_rq, int force) {}
@@ -3416,6 +3428,11 @@ static inline int propagate_entity_load_avg(struct sched_entity *se)
 
 static inline void add_tg_cfs_propagate(struct cfs_rq *cfs_rq, long runnable_sum) {}
 
+static void propagate_exec_runtime(struct cfs_rq *cfs_rq,
+				struct sched_entity *se);
+{
+}
+
 #endif /* CONFIG_FAIR_GROUP_SCHED */
 
 /**
@@ -10140,9 +10157,11 @@ static void propagate_entity_cfs_rq(struct sched_entity *se, int flags)
 		if (cfs_rq_throttled(cfs_rq))
 			break;
 
+		/* Walk the hierarchy while PELT says there is work to do.  */
 		if (!update_load_avg(cfs_rq, se, flags))
 			break;
 
+		propagate_exec_runtime(cfs_rq, se);
 		update_cfs_group(se);
 	}
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load
  2019-07-22 17:33 ` [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load Rik van Riel
@ 2019-07-29 20:05   ` Peter Zijlstra
  2019-07-29 21:13     ` Rik van Riel
  2019-07-29 20:11   ` Peter Zijlstra
  2019-07-29 20:26   ` Peter Zijlstra
  2 siblings, 1 reply; 27+ messages in thread
From: Peter Zijlstra @ 2019-07-29 20:05 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

On Mon, Jul 22, 2019 at 01:33:37PM -0400, Rik van Riel wrote:
> @@ -3012,25 +2983,24 @@ static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
>  static void update_cfs_group(struct sched_entity *se)
>  {
>  	struct cfs_rq *gcfs_rq = group_cfs_rq(se);
> -	long shares, runnable;
> +	long shares;
>  
> -	if (!gcfs_rq)
> +	if (!gcfs_rq) {
> +		update_runnable_load_avg(se);
>  		return;
> +	}
>  
>  	if (throttled_hierarchy(gcfs_rq))
>  		return;
>  
>  #ifndef CONFIG_SMP
> -	runnable = shares = READ_ONCE(gcfs_rq->tg->shares);
> -
>  	if (likely(se->load.weight == shares))

I'm thinking this uses @shares uninitialized...

>  		return;
>  #else
>  	shares   = calc_group_shares(gcfs_rq);
> -	runnable = calc_group_runnable(gcfs_rq, shares);
>  #endif
>  
> -	reweight_entity(cfs_rq_of(se), se, shares, runnable);
> +	reweight_entity(cfs_rq_of(se), se, shares);
>  }

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load
  2019-07-22 17:33 ` [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load Rik van Riel
  2019-07-29 20:05   ` Peter Zijlstra
@ 2019-07-29 20:11   ` Peter Zijlstra
  2019-07-29 20:26   ` Peter Zijlstra
  2 siblings, 0 replies; 27+ messages in thread
From: Peter Zijlstra @ 2019-07-29 20:11 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

On Mon, Jul 22, 2019 at 01:33:37PM -0400, Rik van Riel wrote:
> The runnable_load magic is used to quickly propagate information about
> runnable tasks up the hierarchy of runqueues. The runnable_load_avg is
> mostly used for the load balancing code, which only examines the value at
> the root cfs_rq.
> 
> Redefine the root cfs_rq runnable_load_avg to be the sum of task_h_loads
> of the runnable tasks. This works because the hierarchical runnable_load of
> a task is already equal to the task_se_h_load today. This provides enough
> information to the load balancer.
> 
> The runnable_load_avg of the cgroup cfs_rqs does not appear to be
> used for anything, so don't bother calculating those.
> 
> This removes one of the things that the code currently traverses the
> cgroup hierarchy for, and getting rid of it brings us one step closer
> to a flat runqueue for the CPU controller.

Nice!

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load
  2019-07-22 17:33 ` [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load Rik van Riel
  2019-07-29 20:05   ` Peter Zijlstra
  2019-07-29 20:11   ` Peter Zijlstra
@ 2019-07-29 20:26   ` Peter Zijlstra
  2 siblings, 0 replies; 27+ messages in thread
From: Peter Zijlstra @ 2019-07-29 20:26 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

On Mon, Jul 22, 2019 at 01:33:37PM -0400, Rik van Riel wrote:
> @@ -263,8 +258,8 @@ ___update_load_avg(struct sched_avg *sa, unsigned long load, unsigned long runna
>  
>  int __update_load_avg_blocked_se(u64 now, struct sched_entity *se)
>  {
> -	if (___update_load_sum(now, &se->avg, 0, 0, 0)) {
> -		___update_load_avg(&se->avg, se_weight(se), se_runnable(se));
> +	if (___update_load_sum(now, &se->avg, 0, 0)) {
> +		___update_load_avg(&se->avg, se_weight(se));
>  		return 1;
>  	}
>  

The comment above that needs adjustment too, I think.

--- a/kernel/sched/pelt.c
+++ b/kernel/sched/pelt.c
@@ -234,28 +234,13 @@ ___update_load_avg(struct sched_avg *sa,
 /*
  * sched_entity:
  *
- *   task:
- *     se_runnable() == se_weight()
- *
- *   group: [ see update_cfs_group() ]
- *     se_weight()   = tg->weight * grq->load_avg / tg->load_avg
- *     se_runnable() = se_weight(se) * grq->runnable_load_avg / grq->load_avg
- *
  *   load_sum := runnable_sum
  *   load_avg = se_weight(se) * runnable_avg
  *
- *   runnable_load_sum := runnable_sum
- *   runnable_load_avg = se_runnable(se) * runnable_avg
- *
- * XXX collapse load_sum and runnable_load_sum
- *
  * cfq_rq:
  *
  *   load_sum = \Sum se_weight(se) * se->avg.load_sum
  *   load_avg = \Sum se->avg.load_avg
- *
- *   runnable_load_sum = \Sum se_runnable(se) * se->avg.runnable_load_sum
- *   runnable_load_avg = \Sum se->avg.runable_load_avg
  */
 
 int __update_load_avg_blocked_se(u64 now, struct sched_entity *se)

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load
  2019-07-29 20:05   ` Peter Zijlstra
@ 2019-07-29 21:13     ` Rik van Riel
  0 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-29 21:13 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]

On Mon, 2019-07-29 at 22:05 +0200, Peter Zijlstra wrote:
> On Mon, Jul 22, 2019 at 01:33:37PM -0400, Rik van Riel wrote:
> > @@ -3012,25 +2983,24 @@ static inline int
> > throttled_hierarchy(struct cfs_rq *cfs_rq);
> >  static void update_cfs_group(struct sched_entity *se)
> >  {
> >  	struct cfs_rq *gcfs_rq = group_cfs_rq(se);
> > -	long shares, runnable;
> > +	long shares;
> >  
> > -	if (!gcfs_rq)
> > +	if (!gcfs_rq) {
> > +		update_runnable_load_avg(se);
> >  		return;
> > +	}
> >  
> >  	if (throttled_hierarchy(gcfs_rq))
> >  		return;
> >  
> >  #ifndef CONFIG_SMP
> > -	runnable = shares = READ_ONCE(gcfs_rq->tg->shares);
> > -
> >  	if (likely(se->load.weight == shares))
> 
> I'm thinking this uses @shares uninitialized...

Oops indeed. Let me put the shares = assignment
back for the !SMP case, and edit that comment.

> >  		return;
> >  #else
> >  	shares   = calc_group_shares(gcfs_rq);
> > -	runnable = calc_group_runnable(gcfs_rq, shares);
> >  #endif
> >  
> > -	reweight_entity(cfs_rq_of(se), se, shares, runnable);
> > +	reweight_entity(cfs_rq_of(se), se, shares);
> >  }
-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 04/14] sched,fair: move runnable_load_avg to cfs_rq
  2019-07-22 17:33 ` [PATCH 04/14] sched,fair: move runnable_load_avg to cfs_rq Rik van Riel
@ 2019-07-30  8:55   ` Peter Zijlstra
  0 siblings, 0 replies; 27+ messages in thread
From: Peter Zijlstra @ 2019-07-30  8:55 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

On Mon, Jul 22, 2019 at 01:33:38PM -0400, Rik van Riel wrote:
> Since only the root cfs_rq runnable_load_avg field is used any more,
> we can move the field from struct sched_avg, which every sched_entity
> has one of, directly into the struct cfs_rq, of which we have way fewer.
> 
> No functional changes.
> 
> Suggested-by: Dietmar Eggemann <dietmar.eggemann@arm.com>

I think he's praying on space in that cacheline ;-)

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/14] sched,fair: refactor enqueue/dequeue_entity
  2019-07-22 17:33 ` [PATCH 09/14] sched,fair: refactor enqueue/dequeue_entity Rik van Riel
@ 2019-07-30  9:36   ` Peter Zijlstra
  2019-07-30 12:58     ` Rik van Riel
  2019-07-31  9:35     ` Peter Zijlstra
  0 siblings, 2 replies; 27+ messages in thread
From: Peter Zijlstra @ 2019-07-30  9:36 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

On Mon, Jul 22, 2019 at 01:33:43PM -0400, Rik van Riel wrote:
> Refactor enqueue_entity, dequeue_entity, and update_load_avg, in order
> to split out the things we still want to happen at every level in the
> cgroup hierarchy with a flat runqueue from the things we only need to
> happen once.
> 
> No functional changes.

> @@ -3500,7 +3500,7 @@ static void detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
>  #define DO_ATTACH	0x4
>  
>  /* Update task and its cfs_rq load average */
> -static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> +static inline bool update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  {
>  	u64 now = cfs_rq_clock_pelt(cfs_rq);
>  	int decayed;
> @@ -3529,6 +3529,8 @@ static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
>  
>  	} else if (decayed && (flags & UPDATE_TG))
>  		update_tg_load_avg(cfs_rq, 0);
> +
> +	return decayed;
>  }
>  
>  #ifndef CONFIG_64BIT
> @@ -3745,9 +3747,10 @@ static inline void update_misfit_status(struct task_struct *p, struct rq *rq)
>  #define SKIP_AGE_LOAD	0x0
>  #define DO_ATTACH	0x0
>  
> -static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int not_used1)
> +static inline bool update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int not_used1)
>  {
>  	cfs_rq_util_change(cfs_rq, 0);
> +	return false;
>  }
>  
>  static inline void remove_entity_load_avg(struct sched_entity *se) {}
> @@ -3870,6 +3873,24 @@ static inline void check_schedstat_required(void)
>   * CPU and an up-to-date min_vruntime on the destination CPU.
>   */
>  
> +static bool
> +enqueue_entity_groups(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> +{
> +	/*
> +	 * When enqueuing a sched_entity, we must:
> +	 *   - Update loads to have both entity and cfs_rq synced with now.
> +	 *   - Add its load to cfs_rq->runnable_avg
> +	 *   - For group_entity, update its weight to reflect the new share of
> +	 *     its group cfs_rq
> +	 *   - Add its new weight to cfs_rq->load.weight
> +	 */
> +	if (!update_load_avg(cfs_rq, se, UPDATE_TG | DO_ATTACH))
> +		return false;
> +
> +	update_cfs_group(se);
> +	return true;
> +}
> +
>  static void
>  enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  {
> @@ -3894,16 +3915,6 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>  	if (renorm && !curr)
>  		se->vruntime += cfs_rq->min_vruntime;
>  
> -	/*
> -	 * When enqueuing a sched_entity, we must:
> -	 *   - Update loads to have both entity and cfs_rq synced with now.
> -	 *   - Add its load to cfs_rq->runnable_avg
> -	 *   - For group_entity, update its weight to reflect the new share of
> -	 *     its group cfs_rq
> -	 *   - Add its new weight to cfs_rq->load.weight
> -	 */
> -	update_load_avg(cfs_rq, se, UPDATE_TG | DO_ATTACH);
> -	update_cfs_group(se);
>  	enqueue_runnable_load_avg(cfs_rq, se);
>  	account_entity_enqueue(cfs_rq, se);
>  

No functional, but you did make update_cfs_group() conditional. Now that
looks OK, but maybe you can do that part in a separate patch with a
little justification of its own.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/14] sched,fair: refactor enqueue/dequeue_entity
  2019-07-30  9:36   ` Peter Zijlstra
@ 2019-07-30 12:58     ` Rik van Riel
  2019-07-31  9:35     ` Peter Zijlstra
  1 sibling, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-30 12:58 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]

On Tue, 2019-07-30 at 11:36 +0200, Peter Zijlstra wrote:
> On Mon, Jul 22, 2019 at 01:33:43PM -0400, Rik van Riel wrote:
> > @@ -3870,6 +3873,24 @@ static inline void
> > check_schedstat_required(void)
> >   * CPU and an up-to-date min_vruntime on the destination CPU.
> >   */
> >  
> > +static bool
> > +enqueue_entity_groups(struct cfs_rq *cfs_rq, struct sched_entity
> > *se, int flags)
> > +{
> > +	/*
> > +	 * When enqueuing a sched_entity, we must:
> > +	 *   - Update loads to have both entity and cfs_rq synced with
> > now.
> > +	 *   - Add its load to cfs_rq->runnable_avg
> > +	 *   - For group_entity, update its weight to reflect the new
> > share of
> > +	 *     its group cfs_rq
> > +	 *   - Add its new weight to cfs_rq->load.weight
> > +	 */
> > +	if (!update_load_avg(cfs_rq, se, UPDATE_TG | DO_ATTACH))
> > +		return false;
> > +
> > +	update_cfs_group(se);
> > +	return true;
> > +}
> >  
> 
> No functional, but you did make update_cfs_group() conditional. Now
> that
> looks OK, but maybe you can do that part in a separate patch with a
> little justification of its own.

Good idea, I will split that out.

-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues
  2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
                   ` (12 preceding siblings ...)
  2019-07-22 17:33 ` [PATCH 14/14] sched,fair: propagate sum_exec_runtime up the hierarchy Rik van Riel
@ 2019-07-30 16:29 ` Peter Zijlstra
  2019-07-30 18:27   ` Rik van Riel
  13 siblings, 1 reply; 27+ messages in thread
From: Peter Zijlstra @ 2019-07-30 16:29 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

On Mon, Jul 22, 2019 at 01:33:34PM -0400, Rik van Riel wrote:
> Plan for the CONFIG_CFS_BANDWIDTH reimplementation:
> - When a cgroup gets throttled, mark the cgroup and its children
>   as throttled.
> - When pick_next_entity finds a task that is on a throttled cgroup,
>   stash it on the cgroup runqueue (which is not used for runnable
>   tasks any more). Leave the vruntime unchanged, and adjust that
>   runqueue's vruntime to be that of the left-most task.

and ignore such tasks for the load-balancer; I suppose

> - When a cgroup gets unthrottled, and has tasks on it, place it on
>   a vruntime ordered heap separate from the main runqueue.

and expose said heap to the load-balancer..

Now, I suppose you do this because merging heaps is easier than merging
RB trees? (not in complexity iirc, but probably in code)

> - Have pick_next_task_fair grab one task off that heap every time it
>   is called, and the min vruntime of that heap is lower than the
>   vruntime of the CPU's cfs_rq (or the CPU has no other runnable tasks).

That's like a smeared out merge :-) But since the other tasks kept on
running, this CPUs vruntime will be (much) advanced vs those throttled
tasks and we'll most likely end up picking them all before we pick a
'normal' task.

> - Place that selected task on the CPU's cfs_rq, renormalizing its
>   vruntime with the GENTLE_FAIR_SLEEPERS logic. That should help
>   interleave the already runnable tasks with the recently unthrottled
>   group, and prevent thundering herd issues.
> - If the group gets throttled again before all of its task had a chance
>   to run, vruntime sorting ensures all the tasks in the throttled cgroup
>   get a chance to run over time.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues
  2019-07-30 16:29 ` [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Peter Zijlstra
@ 2019-07-30 18:27   ` Rik van Riel
  0 siblings, 0 replies; 27+ messages in thread
From: Rik van Riel @ 2019-07-30 18:27 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

[-- Attachment #1: Type: text/plain, Size: 2296 bytes --]

On Tue, 2019-07-30 at 18:29 +0200, Peter Zijlstra wrote:
> On Mon, Jul 22, 2019 at 01:33:34PM -0400, Rik van Riel wrote:
> > Plan for the CONFIG_CFS_BANDWIDTH reimplementation:
> > - When a cgroup gets throttled, mark the cgroup and its children
> >   as throttled.
> > - When pick_next_entity finds a task that is on a throttled cgroup,
> >   stash it on the cgroup runqueue (which is not used for runnable
> >   tasks any more). Leave the vruntime unchanged, and adjust that
> >   runqueue's vruntime to be that of the left-most task.
> 
> and ignore such tasks for the load-balancer; I suppose

Good point. I suppose we need to find a lazy way of doing
this, too...

> > - When a cgroup gets unthrottled, and has tasks on it, place it on
> >   a vruntime ordered heap separate from the main runqueue.
> 
> and expose said heap to the load-balancer..
> 
> Now, I suppose you do this because merging heaps is easier than
> merging
> RB trees? (not in complexity iirc, but probably in code)
> 
> > - Have pick_next_task_fair grab one task off that heap every time
> > it
> >   is called, and the min vruntime of that heap is lower than the
> >   vruntime of the CPU's cfs_rq (or the CPU has no other runnable
> > tasks).
> 
> That's like a smeared out merge :-) But since the other tasks kept on
> running, this CPUs vruntime will be (much) advanced vs those
> throttled
> tasks and we'll most likely end up picking them all before we pick a
> 'normal' task.

The GENTLE_FAIR_SLEEPERS code should place the vruntime
of newly unthrottled tasks to the right of that of the
current top task on the runqueue, to prevent thundering
herd effects (and the throttled group immediately going
over its quota again, while causing bad latency for others).

> > - Place that selected task on the CPU's cfs_rq, renormalizing its
> >   vruntime with the GENTLE_FAIR_SLEEPERS logic. That should help
> >   interleave the already runnable tasks with the recently
> > unthrottled
> >   group, and prevent thundering herd issues.
> > - If the group gets throttled again before all of its task had a
> > chance
> >   to run, vruntime sorting ensures all the tasks in the throttled
> > cgroup
> >   get a chance to run over time.
> 
> 
-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/14] sched,fair: refactor enqueue/dequeue_entity
  2019-07-30  9:36   ` Peter Zijlstra
  2019-07-30 12:58     ` Rik van Riel
@ 2019-07-31  9:35     ` Peter Zijlstra
  2019-07-31 15:03       ` Rik van Riel
  1 sibling, 1 reply; 27+ messages in thread
From: Peter Zijlstra @ 2019-07-31  9:35 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

On Tue, Jul 30, 2019 at 11:36:17AM +0200, Peter Zijlstra wrote:
> On Mon, Jul 22, 2019 at 01:33:43PM -0400, Rik van Riel wrote:

> > +static bool
> > +enqueue_entity_groups(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> > +{
> > +	/*
> > +	 * When enqueuing a sched_entity, we must:
> > +	 *   - Update loads to have both entity and cfs_rq synced with now.
> > +	 *   - Add its load to cfs_rq->runnable_avg
> > +	 *   - For group_entity, update its weight to reflect the new share of
> > +	 *     its group cfs_rq
> > +	 *   - Add its new weight to cfs_rq->load.weight
> > +	 */
> > +	if (!update_load_avg(cfs_rq, se, UPDATE_TG | DO_ATTACH))
> > +		return false;
> > +
> > +	update_cfs_group(se);
> > +	return true;
> > +}

> No functional, but you did make update_cfs_group() conditional. Now that
> looks OK, but maybe you can do that part in a separate patch with a
> little justification of its own.

To record (and extend) our discussion from IRC yesterday; I now do think
the above is in fact a problem.

The thing is that update_cfs_group() does not soly rely on the tg state;
it also contains magic to deal with ramp up; for which you later
introduce that max_h_load thing.

Specifically (re)read the second part of the comment describing
calc_group_shares() where it explains the ramp up:

 * The problem with it is that because the average is slow -- it was designed
 * to be exactly that of course -- this leads to transients in boundary
 * conditions. In specific, the case where the group was idle and we start the
 * one task. It takes time for our CPU's grq->avg.load_avg to build up,
 * yielding bad latency etc..

 (and further)

So by not always calling this (and not updating h_load) you fail to take
advantage of this.

So I would suggest keeping update_cfs_group() unconditional, and
recomputing the h_load for the entire hierarchy on enqueue.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/14] sched,fair: refactor enqueue/dequeue_entity
  2019-07-31  9:35     ` Peter Zijlstra
@ 2019-07-31 15:03       ` Rik van Riel
  2019-07-31 15:30         ` Peter Zijlstra
  0 siblings, 1 reply; 27+ messages in thread
From: Rik van Riel @ 2019-07-31 15:03 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

[-- Attachment #1: Type: text/plain, Size: 2492 bytes --]

On Wed, 2019-07-31 at 11:35 +0200, Peter Zijlstra wrote:
> On Tue, Jul 30, 2019 at 11:36:17AM +0200, Peter Zijlstra wrote:
> > On Mon, Jul 22, 2019 at 01:33:43PM -0400, Rik van Riel wrote:
> > > +static bool
> > > +enqueue_entity_groups(struct cfs_rq *cfs_rq, struct sched_entity
> > > *se, int flags)
> > > +{
> > > +	/*
> > > +	 * When enqueuing a sched_entity, we must:
> > > +	 *   - Update loads to have both entity and cfs_rq synced with
> > > now.
> > > +	 *   - Add its load to cfs_rq->runnable_avg
> > > +	 *   - For group_entity, update its weight to reflect the new
> > > share of
> > > +	 *     its group cfs_rq
> > > +	 *   - Add its new weight to cfs_rq->load.weight
> > > +	 */
> > > +	if (!update_load_avg(cfs_rq, se, UPDATE_TG | DO_ATTACH))
> > > +		return false;
> > > +
> > > +	update_cfs_group(se);
> > > +	return true;
> > > +}
> > No functional, but you did make update_cfs_group() conditional. Now
> > that
> > looks OK, but maybe you can do that part in a separate patch with a
> > little justification of its own.
> 
> To record (and extend) our discussion from IRC yesterday; I now do
> think
> the above is in fact a problem.
> 
> The thing is that update_cfs_group() does not soly rely on the tg
> state;
> it also contains magic to deal with ramp up; for which you later
> introduce that max_h_load thing.
> 
> Specifically (re)read the second part of the comment describing
> calc_group_shares() where it explains the ramp up:
> 
>  * The problem with it is that because the average is slow -- it was
> designed
>  * to be exactly that of course -- this leads to transients in
> boundary
>  * conditions. In specific, the case where the group was idle and we
> start the
>  * one task. It takes time for our CPU's grq->avg.load_avg to build
> up,
>  * yielding bad latency etc..
> 
>  (and further)
> 
> So by not always calling this (and not updating h_load) you fail to
> take
> advantage of this.
> 
> So I would suggest keeping update_cfs_group() unconditional, and
> recomputing the h_load for the entire hierarchy on enqueue.

I think I understand the problem you are pointing
out, but if update_load_avg() keeps the load average
for the runqueue unchanged (because that is rate limited
to once a jiffy, and has been like that for a while),
why would calc_group_shares() result in a different value
than what it returned the last time?

What am I overlooking?

-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/14] sched,fair: refactor enqueue/dequeue_entity
  2019-07-31 15:03       ` Rik van Riel
@ 2019-07-31 15:30         ` Peter Zijlstra
  0 siblings, 0 replies; 27+ messages in thread
From: Peter Zijlstra @ 2019-07-31 15:30 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, kernel-team, pjt, dietmar.eggemann, mingo,
	morten.rasmussen, tglx, mgorman, vincent.guittot

On Wed, Jul 31, 2019 at 11:03:01AM -0400, Rik van Riel wrote:

> I think I understand the problem you are pointing out, but if
> update_load_avg() keeps the load average for the runqueue unchanged
> (because that is rate limited to once a jiffy, and has been like that
> for a while), why would calc_group_shares() result in a different
> value than what it returned the last time?
> 
> What am I overlooking?

I'm thinking you're thinking (3):

           tg->weight * grq->avg.load_avg
  shares = ------------------------------
                 tg->load_avg

Where: tg->load_avg ~= \Sum grq->avg.load_avg


Which is the straight forward shares calculation, and purely depends on
the load averages (which haven't been changed etc..)

But what we actually do is (6):

                                    tg->weight * grq->avg.load_avg
  shares = ---------------------------------------------------------------------------
           tg->load_avg - grq->avg.load_avg + max(grq->load.weight, grq->avg.load_avg)

And even if tg->load_avg and grq->avg.load_avg haven't changed,
grq->load.weight most certainly has.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 01/14] sched: introduce task_se_h_load helper
  2019-07-22 17:33 ` [PATCH 01/14] sched: introduce task_se_h_load helper Rik van Riel
@ 2019-08-12 17:40   ` Dietmar Eggemann
  0 siblings, 0 replies; 27+ messages in thread
From: Dietmar Eggemann @ 2019-08-12 17:40 UTC (permalink / raw)
  To: Rik van Riel, linux-kernel
  Cc: kernel-team, pjt, peterz, mingo, morten.rasmussen, tglx, mgorman,
	vincent.guittot, Josef Bacik

On 7/22/19 7:33 PM, Rik van Riel wrote:
> Sometimes the hierarchical load of a sched_entity needs to be calculated.
> Rename task_h_load to task_se_h_load, and directly pass a sched_entity to
> that function.
> 
> Move the function declaration up above where it will be used later.
> 
> No functional changes.
> 
> Signed-off-by: Rik van Riel <riel@surriel.com>
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>

[...]

>  /* Give new sched_entity start runnable values to heavy its load in infant time */
> @@ -1668,7 +1668,7 @@ static void task_numa_compare(struct task_numa_env *env,
>  	/*
>  	 * In the overloaded case, try and keep the load balanced.
>  	 */
> -	load = task_h_load(env->p) - task_h_load(cur);
> +	load = task_se_h_load(env->p->se) - task_se_h_load(cur->se);

Shouldn't this be:

load = task_se_h_load(&env->p->se) - task_se_h_load(&cur->se);

>  	if (!load)
>  		goto assign;
>  
> @@ -1706,7 +1706,7 @@ static void task_numa_find_cpu(struct task_numa_env *env,
>  	bool maymove = false;
>  	int cpu;
>  
> -	load = task_h_load(env->p);
> +	load = task_se_h_load(env->p->se);

load = task_se_h_load(&env->p->se);

[...]

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2019-08-12 17:49 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-22 17:33 [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Rik van Riel
2019-07-22 17:33 ` [PATCH 01/14] sched: introduce task_se_h_load helper Rik van Riel
2019-08-12 17:40   ` Dietmar Eggemann
2019-07-22 17:33 ` [PATCH 02/14] sched: change /proc/sched_debug fields Rik van Riel
2019-07-22 17:33 ` [PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load Rik van Riel
2019-07-29 20:05   ` Peter Zijlstra
2019-07-29 21:13     ` Rik van Riel
2019-07-29 20:11   ` Peter Zijlstra
2019-07-29 20:26   ` Peter Zijlstra
2019-07-22 17:33 ` [PATCH 04/14] sched,fair: move runnable_load_avg to cfs_rq Rik van Riel
2019-07-30  8:55   ` Peter Zijlstra
2019-07-22 17:33 ` [PATCH 06/14] sched,cfs: use explicit cfs_rq of parent se helper Rik van Riel
2019-07-22 17:33 ` [PATCH 07/14] sched,cfs: fix zero length timeslice calculation Rik van Riel
2019-07-22 17:33 ` [PATCH 08/14] sched,fair: simplify timeslice length code Rik van Riel
2019-07-22 17:33 ` [PATCH 09/14] sched,fair: refactor enqueue/dequeue_entity Rik van Riel
2019-07-30  9:36   ` Peter Zijlstra
2019-07-30 12:58     ` Rik van Riel
2019-07-31  9:35     ` Peter Zijlstra
2019-07-31 15:03       ` Rik van Riel
2019-07-31 15:30         ` Peter Zijlstra
2019-07-22 17:33 ` [PATCH 10/14] sched,fair: add helper functions for flattened runqueue Rik van Riel
2019-07-22 17:33 ` [PATCH 11/14] sched,fair: flatten hierarchical runqueues Rik van Riel
2019-07-22 17:33 ` [PATCH 12/14] sched,fair: track cfs_rq->max_h_load for more legitimate h_weight Rik van Riel
2019-07-22 17:33 ` [PATCH 13/14] sched,fair: flatten update_curr functionality Rik van Riel
2019-07-22 17:33 ` [PATCH 14/14] sched,fair: propagate sum_exec_runtime up the hierarchy Rik van Riel
2019-07-30 16:29 ` [PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues Peter Zijlstra
2019-07-30 18:27   ` Rik van Riel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).