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

Groups > linux.kernel > #1687425 > unrolled thread

[PATCH 0/7][RESEND] Fix cpu imbalance with equal weighted groups

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7][RESEND] Fix cpu imbalance with equal weighted groups Josef Bacik <josef@toxicpanda.com> - 2017-07-14 15:30 +0200
    [PATCH 3/7] sched/fair: fix definitions of effective load Josef Bacik <josef@toxicpanda.com> - 2017-07-14 15:30 +0200
    [PATCH 7/7] sched/fair: don't wake affine recently load balanced tasks Josef Bacik <josef@toxicpanda.com> - 2017-07-14 15:30 +0200
    [PATCH 1/7] sched/fair: use reweight_entity to reweight tasks Josef Bacik <josef@toxicpanda.com> - 2017-07-14 15:30 +0200

#1687425 — [PATCH 0/7][RESEND] Fix cpu imbalance with equal weighted groups

(sorry to anybody who got this already, fb email is acting weird and ate the
linux-kernel submission so I have no idea who got this and who didn't.)

Hello,
 
In testing stacked services we noticed that if you started a normal CPU heavy
application in one cgroup, and a cpu stress test in another cgroup of equal
weight, the cpu stress group would get significantly more cpu time, usually
around 50% more.

Peter fixed some load propagation issues for Tejun a few months ago, and they
fixed the latency issues that Tejun was seeing, however they regressed this
imbalance problem more, so the cpu stress group was now getting more like 70%
more CPU time.

The following patches are to fix the regression introduced by Peter's patches
and then to fix the imbalance itself.  Part of the imbalance fix is from Peter's
propagation patches, we just needed the runnable weight to be calculated
differently to fix the regression.

Essentially what happens is the "stress" group has tasks that never leave the
CPU, so the load average and runnable load average skews towards their
load.weight.  However the interactive tasks obviously go on and off the CPU,
resulting in a lower load average.  With Peter's changes to use the runnable
load average more this exacerbated the problem.

To solve this problem I've done a few things.  First we use the max of the
weight or average for our cfs_rq weight calculations.  This allows tasks that
have a lower load average but a higher weight to have an appropriate effect on
the cfs_rq when enqueue'ing.

The second part of the fix is to fix how we decide to do wake affinity.  If I
simply disabled wake affinity and had the other patches the imbalance
disappeared as well.  Fixing the wake affinity involves a few things.

First we need to change effective_load() to re-calculate the historic weight in
addition to the new weight with the new process.  This is because simply using
our old weight/load_avg would be inaccurate if the load_avg for the task_group
had changed at all since we calculated our load.  In practice this meant that
effective_load would often (almost always for my testcase) return a negative
delta for adding the process to the given CPU.  This meant we always did wake
affine, even though the load on the current CPU was too high.

Those patches get us 95% there, the final patch is probably the more
controversial one, but brings us to complete balance between the two groups.
One thing that was observed was we would wake affine, and then promptly load
balance things off of the CPU that we woke to.  You'd see tasks bounce around
CPU's constantly.  So to avoid this thrashing record the last time we were load
balanced, and wait HZ duration before allowing a affinity wake up to occur.
This reduced the thrashing quite a bit, and brought our CPU usage to equality.

I have a stripped down reproducer here

https://github.com/josefbacik/debug-scripts/tree/master/unbalanced-reproducer

unbalanced.sh uses the cgroup2 interface which requires Tejun's cgroup2 cpu
controller patch, and unbalanced-v1.sh uses the old cgroupsv1 interface, and
assumes you have cpuacct,cpu mounted at /sys/fs/cgroup/cpuacct.  You also need
rt-app installed.

Thanks,

Josef

[toc] | [next] | [standalone]

#1687429 — [PATCH 3/7] sched/fair: fix definitions of effective load

From: Josef Bacik <jbacik@fb.com>

It appears as though we've reversed the definitions of 'this_effective_load' and
'prev_effective_load'.  We seem to be using the wrong CPU capacity for both of
these parameters, and we want to add the imbalance percentage to the current CPU
to make sure we're meeting a high enough load imbalance threshold to justify
moving the task.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 kernel/sched/fair.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 5d4489e..d958634 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5679,11 +5679,11 @@ static int wake_affine(struct sched_domain *sd, struct task_struct *p,
 	 * Otherwise check if either cpus are near enough in load to allow this
 	 * task to be woken on this_cpu.
 	 */
-	this_eff_load = 100;
-	this_eff_load *= capacity_of(prev_cpu);
+	prev_eff_load = 100;
+	prev_eff_load *= capacity_of(prev_cpu);
 
-	prev_eff_load = 100 + (sd->imbalance_pct - 100) / 2;
-	prev_eff_load *= capacity_of(this_cpu);
+	this_eff_load = 100 + (sd->imbalance_pct - 100) / 2;
+	this_eff_load *= capacity_of(this_cpu);
 
 	if (this_load > 0) {
 		this_eff_load *= this_load +
-- 
2.9.3

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

#1687430 — [PATCH 7/7] sched/fair: don't wake affine recently load balanced tasks

From: Josef Bacik <jbacik@fb.com>

The wake affinity logic will move tasks between two cpu's that appear to be
loaded equally at the current time, with a slight bias towards cache locality.
However on a heavily loaded system the load balancer has a better insight into
what needs to be moved around, so instead keep track of the last time a task was
migrated by the load balancer.  If it was recent, opt to let the process stay on
it's current CPU (or an idle sibling).

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 include/linux/sched.h |  1 +
 kernel/sched/fair.c   | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 1a0eadd..d872780 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -528,6 +528,7 @@ struct task_struct {
 	unsigned long			wakee_flip_decay_ts;
 	struct task_struct		*last_wakee;
 
+	unsigned long			last_balance_ts;
 	int				wake_cpu;
 #endif
 	int				on_rq;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 034d5df..6a98a38 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5604,6 +5604,16 @@ static int wake_wide(struct task_struct *p)
 	unsigned int slave = p->wakee_flips;
 	int factor = this_cpu_read(sd_llc_size);
 
+	/*
+	 * If we've balanced this task recently we don't want to undo all of
+	 * that hard work by the load balancer and move it to the current cpu.
+	 * Constantly overriding the load balancers decisions is going to make
+	 * it question its purpose in life and give it anxiety and self worth
+	 * issues, and nobody wants that.
+	 */
+	if (time_before(jiffies, p->last_balance_ts + HZ))
+		return 1;
+
 	if (master < slave)
 		swap(master, slave);
 	if (slave < factor || master < slave * factor)
@@ -7097,6 +7107,7 @@ static int detach_tasks(struct lb_env *env)
 			goto next;
 
 		detach_task(p, env);
+		p->last_balance_ts = jiffies;
 		list_add(&p->se.group_node, &env->tasks);
 
 		detached++;
-- 
2.9.3

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

#1687433 — [PATCH 1/7] sched/fair: use reweight_entity to reweight tasks

From: Josef Bacik <jbacik@fb.com>

reweight_task only accounts for the load average change in the cfs_rq, but
doesn't account for the runnable_average change in the cfs_rq.  We need to do
everything reweight_entity does, and then we just set our inv_weight
appropriately.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 kernel/sched/fair.c | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b13b451..326bc55 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2808,26 +2808,6 @@ __sub_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	sub_positive(&cfs_rq->avg.load_sum, se_weight(se) * se->avg.load_sum);
 }
 
-void reweight_task(struct task_struct *p, int prio)
-{
-	struct sched_entity *se = &p->se;
-	struct cfs_rq *cfs_rq = cfs_rq_of(se);
-	struct load_weight *load = &p->se.load;
-
-	u32 divider = LOAD_AVG_MAX - 1024 + se->avg.period_contrib;
-
-	__sub_load_avg(cfs_rq, se);
-
-	load->weight = scale_load(sched_prio_to_weight[prio]);
-	load->inv_weight = sched_prio_to_wmult[prio];
-
-	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);
-
-	__add_load_avg(cfs_rq, se);
-}
-
 static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
 			    unsigned long weight, unsigned long runnable)
 {
@@ -2857,6 +2837,17 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
 	}
 }
 
+void reweight_task(struct task_struct *p, int prio)
+{
+	struct sched_entity *se = &p->se;
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+	struct load_weight *load = &se->load;
+	unsigned long weight = scale_load(sched_prio_to_weight[prio]);
+
+	reweight_entity(cfs_rq, se, weight, weight);
+	load->inv_weight = sched_prio_to_wmult[prio];
+}
+
 static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
 
 /*
-- 
2.9.3

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web