Hi Peter, Rik,
Running sysbench measurements in a 16CPU/30GB KVM guest on a 20CPU/40GB
s390x host, we noticed a throughput degradation (anywhere between 13%
and 40%, depending on test) when moving the host from kernel 4.12 to
4.13. The rest of the host and the entire guest remain unchanged; it is
only the host kernel that changes. Bisecting the host kernel blames
commit 3fed382b46ba ("sched/numa: Implement NUMA node level wake_affine()").
Reverting 3fed382b46ba and 815abf5af45f ("sched/fair: Remove
effective_load()") from a clean 4.13.0 build erases the throughput
degradation and returns us to what we see in 4.12.0.
A little poking around points us to a fix/improvement to this, commit
90001d67be2f ("sched/fair: Fix wake_affine() for !NUMA_BALANCING"),
which went in the 4.14 merge window and an unmerged fix [1] that
corrects a small error in that patch. Hopeful, since we were running
!NUMA_BALANCING, I applied these two patches to a clean 4.13.0 tree but
continue to see the performance degradation. Pulling current master or
linux-next shows no improvement lurking in the shadows.
Running perf stat on the host during the guest sysbench run shows a
significant increase in cpu-migrations over the 4.12.0 run. Abbreviated
examples follow:
# 4.12.0
# perf stat -p 11473 -- sleep 5
62305.199305 task-clock (msec) # 12.458 CPUs
368,607 context-switches
4,084 cpu-migrations
416 page-faults
# 4.13.0
# perf stat -p 11444 -- sleep 5
35892.653243 task-clock (msec) # 7.176 CPUs
249,251 context-switches
56,850 cpu-migrations
804 page-faults
# 4.13.0-revert-3fed382b46ba-and-815abf5af45f
# perf stat -p 11441 -- sleep 5
62321.767146 task-clock (msec) # 12.459 CPUs
387,661 context-switches
5,687 cpu-migrations
1,652 page-faults
# 4.13.0-apply-90001d67be2f
# perf stat -p 11438 -- sleep 5
48654.988291 task-clock (msec) # 9.729 CPUs
363,150 context-switches
43,778 cpu-migrations
641 page-faults
I'm not sure what doc to supply here and am unfamiliar with this code or
its recent changes, but I'd be happy to pull/try whatever is needed to
help debug things. Looking forward to hearing what I can do.
Thanks,
Eric
[1] https://lkml.org/lkml/2017/9/6/196
2017-09-12 16:14 GMT+02:00 Eric Farman <[email protected]>:
> Hi Peter, Rik,
>
> Running sysbench measurements in a 16CPU/30GB KVM guest on a 20CPU/40GB
> s390x host, we noticed a throughput degradation (anywhere between 13% and
> 40%, depending on test) when moving the host from kernel 4.12 to 4.13. The
> rest of the host and the entire guest remain unchanged; it is only the host
> kernel that changes. Bisecting the host kernel blames commit 3fed382b46ba
> ("sched/numa: Implement NUMA node level wake_affine()").
>
> Reverting 3fed382b46ba and 815abf5af45f ("sched/fair: Remove
> effective_load()") from a clean 4.13.0 build erases the throughput
> degradation and returns us to what we see in 4.12.0.
>
> A little poking around points us to a fix/improvement to this, commit
> 90001d67be2f ("sched/fair: Fix wake_affine() for !NUMA_BALANCING"), which
> went in the 4.14 merge window and an unmerged fix [1] that corrects a small
> error in that patch. Hopeful, since we were running !NUMA_BALANCING, I
> applied these two patches to a clean 4.13.0 tree but continue to see the
> performance degradation. Pulling current master or linux-next shows no
> improvement lurking in the shadows.
>
> Running perf stat on the host during the guest sysbench run shows a
> significant increase in cpu-migrations over the 4.12.0 run. Abbreviated
> examples follow:
>
> # 4.12.0
> # perf stat -p 11473 -- sleep 5
> 62305.199305 task-clock (msec) # 12.458 CPUs
> 368,607 context-switches
> 4,084 cpu-migrations
> 416 page-faults
>
> # 4.13.0
> # perf stat -p 11444 -- sleep 5
> 35892.653243 task-clock (msec) # 7.176 CPUs
> 249,251 context-switches
> 56,850 cpu-migrations
> 804 page-faults
>
> # 4.13.0-revert-3fed382b46ba-and-815abf5af45f
> # perf stat -p 11441 -- sleep 5
> 62321.767146 task-clock (msec) # 12.459 CPUs
> 387,661 context-switches
> 5,687 cpu-migrations
> 1,652 page-faults
>
> # 4.13.0-apply-90001d67be2f
> # perf stat -p 11438 -- sleep 5
> 48654.988291 task-clock (msec) # 9.729 CPUs
> 363,150 context-switches
> 43,778 cpu-migrations
> 641 page-faults
>
> I'm not sure what doc to supply here and am unfamiliar with this code or its
> recent changes, but I'd be happy to pull/try whatever is needed to help
> debug things. Looking forward to hearing what I can do.
>
> Thanks,
> Eric
>
> [1] https://lkml.org/lkml/2017/9/6/196
>
+cc: [email protected]
He reported a performance degradation also on 4.13-rc7, it might be
the same cause.
Best,
Jack
On 09/13/2017 04:24 AM, 王金浦 wrote:
> 2017-09-12 16:14 GMT+02:00 Eric Farman <[email protected]>:
>> Hi Peter, Rik,
>>
>> Running sysbench measurements in a 16CPU/30GB KVM guest on a 20CPU/40GB
>> s390x host, we noticed a throughput degradation (anywhere between 13% and
>> 40%, depending on test) when moving the host from kernel 4.12 to 4.13. The
>> rest of the host and the entire guest remain unchanged; it is only the host
>> kernel that changes. Bisecting the host kernel blames commit 3fed382b46ba
>> ("sched/numa: Implement NUMA node level wake_affine()").
>>
>> Reverting 3fed382b46ba and 815abf5af45f ("sched/fair: Remove
>> effective_load()") from a clean 4.13.0 build erases the throughput
>> degradation and returns us to what we see in 4.12.0.
>>
>> A little poking around points us to a fix/improvement to this, commit
>> 90001d67be2f ("sched/fair: Fix wake_affine() for !NUMA_BALANCING"), which
>> went in the 4.14 merge window and an unmerged fix [1] that corrects a small
>> error in that patch. Hopeful, since we were running !NUMA_BALANCING, I
>> applied these two patches to a clean 4.13.0 tree but continue to see the
>> performance degradation. Pulling current master or linux-next shows no
>> improvement lurking in the shadows.
>>
>> Running perf stat on the host during the guest sysbench run shows a
>> significant increase in cpu-migrations over the 4.12.0 run. Abbreviated
>> examples follow:
>>
>> # 4.12.0
>> # perf stat -p 11473 -- sleep 5
>> 62305.199305 task-clock (msec) # 12.458 CPUs
>> 368,607 context-switches
>> 4,084 cpu-migrations
>> 416 page-faults
>>
>> # 4.13.0
>> # perf stat -p 11444 -- sleep 5
>> 35892.653243 task-clock (msec) # 7.176 CPUs
>> 249,251 context-switches
>> 56,850 cpu-migrations
>> 804 page-faults
>>
>> # 4.13.0-revert-3fed382b46ba-and-815abf5af45f
>> # perf stat -p 11441 -- sleep 5
>> 62321.767146 task-clock (msec) # 12.459 CPUs
>> 387,661 context-switches
>> 5,687 cpu-migrations
>> 1,652 page-faults
>>
>> # 4.13.0-apply-90001d67be2f
>> # perf stat -p 11438 -- sleep 5
>> 48654.988291 task-clock (msec) # 9.729 CPUs
>> 363,150 context-switches
>> 43,778 cpu-migrations
>> 641 page-faults
>>
>> I'm not sure what doc to supply here and am unfamiliar with this code or its
>> recent changes, but I'd be happy to pull/try whatever is needed to help
>> debug things. Looking forward to hearing what I can do.
>>
>> Thanks,
>> Eric
>>
>> [1] https://lkml.org/lkml/2017/9/6/196
>>
> +cc: [email protected]
> He reported a performance degradation also on 4.13-rc7, it might be
> the same cause.
>
> Best,
> Jack
>
Hi Peter, Rik,
With OSS last week, I'm sure this got lost in the deluge, so here's a
friendly ping. I picked up 4.14.0-rc1 earlier this week, and still see
the degradation described above. Not really a surprise, since I don't
see any other commits in this area beyond the ones I mentioned in my
original note.
Anyway, I'm unsure what else to try or what doc to pull to help debug
this, and would appreciate your expertise here. We can repro this
pretty easily as necessary to help get to the bottom of this.
Many thanks in advance,
- Eric
(also, +cc Matt to help when I'm out of office myself.)
On Fri, Sep 22, 2017 at 11:03:39AM -0400, Eric Farman wrote:
> Hi Peter, Rik,
>
> With OSS last week, I'm sure this got lost in the deluge, so here's a
> friendly ping.
Very much so, inbox is a giant trainwreck ;-)
> I picked up 4.14.0-rc1 earlier this week, and still see the
> degradation described above. Not really a surprise, since I don't see any
> other commits in this area beyond the ones I mentioned in my original note.
>
> Anyway, I'm unsure what else to try or what doc to pull to help debug this,
> and would appreciate your expertise here. We can repro this pretty easily
> as necessary to help get to the bottom of this.
>
> Many thanks in advance,
Could you describe your sysbench setup? Are you running it on mysql or
postgresql, what other options?
On 09/22/2017 11:53 AM, Peter Zijlstra wrote:
> On Fri, Sep 22, 2017 at 11:03:39AM -0400, Eric Farman wrote:
>> Hi Peter, Rik,
>>
>> With OSS last week, I'm sure this got lost in the deluge, so here's a
>> friendly ping.
>
> Very much so, inbox is a giant trainwreck ;-)
My apologies. :)
>
>> I picked up 4.14.0-rc1 earlier this week, and still see the
>> degradation described above. Not really a surprise, since I don't see any
>> other commits in this area beyond the ones I mentioned in my original note.
>>
>> Anyway, I'm unsure what else to try or what doc to pull to help debug this,
>> and would appreciate your expertise here. We can repro this pretty easily
>> as necessary to help get to the bottom of this.
>>
>> Many thanks in advance,
>
> Could you describe your sysbench setup? Are you running it on mysql or
> postgresql, what other options?
>
MySQL. We've tried a few different configs with both test=oltp and
test=threads, but both show the same behavior. What I have settled on
for my repro is the following:
sudo sysbench --db-driver=mysql --mysql-host=localhost --mysql-user=root
--mysql-db=test1 --max-time=180 --max-requests=100000000 --num-threads=8
--test=oltp prepare
sudo sysbench --db-driver=mysql --mysql-host=localhost --mysql-user=root
--mysql-db=test1 --max-time=180 --max-requests=100000000 --num-threads=8
--test=oltp run
Have also some environments where multiple sysbench instances are run
concurrently (I've tried up to 8, with db's test1-8 being specified),
but doesn't appear to much matter either.
- Eric
On Fri, Sep 22, 2017 at 12:12:45PM -0400, Eric Farman wrote:
>
> MySQL. We've tried a few different configs with both test=oltp and
> test=threads, but both show the same behavior. What I have settled on for
> my repro is the following:
>
Right, didn't even need to run it in a guest to observe a regression.
So the below cures native sysbench and NAS bench for me, does it also
work for you virt thingy?
PRE (current tip/master):
ivb-ex sysbench:
2: [30 secs] transactions: 64110 (2136.94 per sec.)
5: [30 secs] transactions: 143644 (4787.99 per sec.)
10: [30 secs] transactions: 274298 (9142.93 per sec.)
20: [30 secs] transactions: 418683 (13955.45 per sec.)
40: [30 secs] transactions: 320731 (10690.15 per sec.)
80: [30 secs] transactions: 355096 (11834.28 per sec.)
hsw-ex NAS:
OMP_PROC_BIND/lu.C.x_threads_144_run_1.log: Time in seconds = 18.01
OMP_PROC_BIND/lu.C.x_threads_144_run_2.log: Time in seconds = 17.89
OMP_PROC_BIND/lu.C.x_threads_144_run_3.log: Time in seconds = 17.93
lu.C.x_threads_144_run_1.log: Time in seconds = 434.68
lu.C.x_threads_144_run_2.log: Time in seconds = 405.36
lu.C.x_threads_144_run_3.log: Time in seconds = 433.83
POST (+patch):
ivb-ex sysbench:
2: [30 secs] transactions: 64494 (2149.75 per sec.)
5: [30 secs] transactions: 145114 (4836.99 per sec.)
10: [30 secs] transactions: 278311 (9276.69 per sec.)
20: [30 secs] transactions: 437169 (14571.60 per sec.)
40: [30 secs] transactions: 669837 (22326.73 per sec.)
80: [30 secs] transactions: 631739 (21055.88 per sec.)
hsw-ex NAS:
lu.C.x_threads_144_run_1.log: Time in seconds = 23.36
lu.C.x_threads_144_run_2.log: Time in seconds = 22.96
lu.C.x_threads_144_run_3.log: Time in seconds = 22.52
This patch takes out all the shiny wake_affine stuff and goes back to
utter basics. Rik was there another NUMA benchmark that wanted your
fancy stuff? Because NAS isn't it.
(the previous, slightly fancier wake_affine was basically a !idle
extension of this, in that it would pick the 'shortest' of the 2 queues
and thereby run quickest, in approximation)
I'll try and run a number of other benchmarks I have around to see if
there's anything that shows a difference between the below trivial
wake_affine and the old 2-cpu-load one.
---
include/linux/sched/topology.h | 8 ---
kernel/sched/fair.c | 125 ++---------------------------------------
2 files changed, 6 insertions(+), 127 deletions(-)
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index d7b6dab956ec..7d065abc7a47 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -71,14 +71,6 @@ struct sched_domain_shared {
atomic_t ref;
atomic_t nr_busy_cpus;
int has_idle_cores;
-
- /*
- * Some variables from the most recent sd_lb_stats for this domain,
- * used by wake_affine().
- */
- unsigned long nr_running;
- unsigned long load;
- unsigned long capacity;
};
struct sched_domain {
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 70ba32e08a23..66930ce338af 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5356,115 +5356,19 @@ static int wake_wide(struct task_struct *p)
return 1;
}
-struct llc_stats {
- unsigned long nr_running;
- unsigned long load;
- unsigned long capacity;
- int has_capacity;
-};
-
-static bool get_llc_stats(struct llc_stats *stats, int cpu)
-{
- struct sched_domain_shared *sds = rcu_dereference(per_cpu(sd_llc_shared, cpu));
-
- if (!sds)
- return false;
-
- stats->nr_running = READ_ONCE(sds->nr_running);
- stats->load = READ_ONCE(sds->load);
- stats->capacity = READ_ONCE(sds->capacity);
- stats->has_capacity = stats->nr_running < per_cpu(sd_llc_size, cpu);
-
- return true;
-}
-
-/*
- * Can a task be moved from prev_cpu to this_cpu without causing a load
- * imbalance that would trigger the load balancer?
- *
- * Since we're running on 'stale' values, we might in fact create an imbalance
- * but recomputing these values is expensive, as that'd mean iteration 2 cache
- * domains worth of CPUs.
- */
-static bool
-wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
- int this_cpu, int prev_cpu, int sync)
-{
- struct llc_stats prev_stats, this_stats;
- s64 this_eff_load, prev_eff_load;
- unsigned long task_load;
-
- if (!get_llc_stats(&prev_stats, prev_cpu) ||
- !get_llc_stats(&this_stats, this_cpu))
- return false;
-
- /*
- * If sync wakeup then subtract the (maximum possible)
- * effect of the currently running task from the load
- * of the current LLC.
- */
- if (sync) {
- unsigned long current_load = task_h_load(current);
-
- /* in this case load hits 0 and this LLC is considered 'idle' */
- if (current_load > this_stats.load)
- return true;
-
- this_stats.load -= current_load;
- }
-
- /*
- * The has_capacity stuff is not SMT aware, but by trying to balance
- * the nr_running on both ends we try and fill the domain at equal
- * rates, thereby first consuming cores before siblings.
- */
-
- /* if the old cache has capacity, stay there */
- if (prev_stats.has_capacity && prev_stats.nr_running < this_stats.nr_running+1)
- return false;
-
- /* if this cache has capacity, come here */
- if (this_stats.has_capacity && this_stats.nr_running+1 < prev_stats.nr_running)
- return true;
-
- /*
- * Check to see if we can move the load without causing too much
- * imbalance.
- */
- task_load = task_h_load(p);
-
- this_eff_load = 100;
- this_eff_load *= prev_stats.capacity;
-
- prev_eff_load = 100 + (sd->imbalance_pct - 100) / 2;
- prev_eff_load *= this_stats.capacity;
-
- this_eff_load *= this_stats.load + task_load;
- prev_eff_load *= prev_stats.load - task_load;
-
- return this_eff_load <= prev_eff_load;
-}
-
static int wake_affine(struct sched_domain *sd, struct task_struct *p,
int prev_cpu, int sync)
{
int this_cpu = smp_processor_id();
- bool affine;
-
- /*
- * Default to no affine wakeups; wake_affine() should not effect a task
- * placement the load-balancer feels inclined to undo. The conservative
- * option is therefore to not move tasks when they wake up.
- */
- affine = false;
+ bool affine = false;
/*
- * If the wakeup is across cache domains, try to evaluate if movement
- * makes sense, otherwise rely on select_idle_siblings() to do
- * placement inside the cache domain.
+ * If we can run _now_ on the waking CPU, go there, otherwise meh.
*/
- if (!cpus_share_cache(prev_cpu, this_cpu))
- affine = wake_affine_llc(sd, p, this_cpu, prev_cpu, sync);
+ if (idle_cpu(this_cpu))
+ affine = true;
+ else if (sync && cpu_rq(this_cpu)->nr_running == 1)
+ affine = true;
schedstat_inc(p->se.statistics.nr_wakeups_affine_attempts);
if (affine) {
@@ -7600,7 +7504,6 @@ static inline enum fbq_type fbq_classify_rq(struct rq *rq)
*/
static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sds)
{
- struct sched_domain_shared *shared = env->sd->shared;
struct sched_domain *child = env->sd->child;
struct sched_group *sg = env->sd->groups;
struct sg_lb_stats *local = &sds->local_stat;
@@ -7672,22 +7575,6 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
if (env->dst_rq->rd->overload != overload)
env->dst_rq->rd->overload = overload;
}
-
- if (!shared)
- return;
-
- /*
- * Since these are sums over groups they can contain some CPUs
- * multiple times for the NUMA domains.
- *
- * Currently only wake_affine_llc() and find_busiest_group()
- * uses these numbers, only the last is affected by this problem.
- *
- * XXX fix that.
- */
- WRITE_ONCE(shared->nr_running, sds->total_running);
- WRITE_ONCE(shared->load, sds->total_load);
- WRITE_ONCE(shared->capacity, sds->total_capacity);
}
/**
On 09/27/2017 05:35 AM, Peter Zijlstra wrote:
> On Fri, Sep 22, 2017 at 12:12:45PM -0400, Eric Farman wrote:
>>
>> MySQL. We've tried a few different configs with both test=oltp and
>> test=threads, but both show the same behavior. What I have settled on for
>> my repro is the following:
>>
>
> Right, didn't even need to run it in a guest to observe a regression.
>
> So the below cures native sysbench and NAS bench for me, does it also
> work for you virt thingy?
>
Ran a quick test this morning with 4.13.0 + 90001d67be2f + a731ebe6f17b
and then with/without this patch. An oltp sysbench run shows that guest
cpu migrations decreased significantly, from ~27K to ~2K over 5 seconds.
So, we applied this patch to linux-next (next-20170926) and ran it
against a couple sysbench tests:
--test=oltp
Baseline: 5655.26 transactions/second
Patched: 9618.13 transactions/second
--test=threads
Baseline: 25482.9 events/sec
Patched: 29577.9 events/sec
That's good! With that...
Tested-by: Eric Farman <[email protected]>
Thanks!
- Eric
>
> PRE (current tip/master):
>
> ivb-ex sysbench:
>
> 2: [30 secs] transactions: 64110 (2136.94 per sec.)
> 5: [30 secs] transactions: 143644 (4787.99 per sec.)
> 10: [30 secs] transactions: 274298 (9142.93 per sec.)
> 20: [30 secs] transactions: 418683 (13955.45 per sec.)
> 40: [30 secs] transactions: 320731 (10690.15 per sec.)
> 80: [30 secs] transactions: 355096 (11834.28 per sec.)
>
> hsw-ex NAS:
>
> OMP_PROC_BIND/lu.C.x_threads_144_run_1.log: Time in seconds = 18.01
> OMP_PROC_BIND/lu.C.x_threads_144_run_2.log: Time in seconds = 17.89
> OMP_PROC_BIND/lu.C.x_threads_144_run_3.log: Time in seconds = 17.93
> lu.C.x_threads_144_run_1.log: Time in seconds = 434.68
> lu.C.x_threads_144_run_2.log: Time in seconds = 405.36
> lu.C.x_threads_144_run_3.log: Time in seconds = 433.83
>
>
> POST (+patch):
>
> ivb-ex sysbench:
>
> 2: [30 secs] transactions: 64494 (2149.75 per sec.)
> 5: [30 secs] transactions: 145114 (4836.99 per sec.)
> 10: [30 secs] transactions: 278311 (9276.69 per sec.)
> 20: [30 secs] transactions: 437169 (14571.60 per sec.)
> 40: [30 secs] transactions: 669837 (22326.73 per sec.)
> 80: [30 secs] transactions: 631739 (21055.88 per sec.)
>
> hsw-ex NAS:
>
> lu.C.x_threads_144_run_1.log: Time in seconds = 23.36
> lu.C.x_threads_144_run_2.log: Time in seconds = 22.96
> lu.C.x_threads_144_run_3.log: Time in seconds = 22.52
>
>
> This patch takes out all the shiny wake_affine stuff and goes back to
> utter basics. Rik was there another NUMA benchmark that wanted your
> fancy stuff? Because NAS isn't it.
>
> (the previous, slightly fancier wake_affine was basically a !idle
> extension of this, in that it would pick the 'shortest' of the 2 queues
> and thereby run quickest, in approximation)
>
> I'll try and run a number of other benchmarks I have around to see if
> there's anything that shows a difference between the below trivial
> wake_affine and the old 2-cpu-load one.
>
> ---
> include/linux/sched/topology.h | 8 ---
> kernel/sched/fair.c | 125 ++---------------------------------------
> 2 files changed, 6 insertions(+), 127 deletions(-)
>
> diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> index d7b6dab956ec..7d065abc7a47 100644
> --- a/include/linux/sched/topology.h
> +++ b/include/linux/sched/topology.h
> @@ -71,14 +71,6 @@ struct sched_domain_shared {
> atomic_t ref;
> atomic_t nr_busy_cpus;
> int has_idle_cores;
> -
> - /*
> - * Some variables from the most recent sd_lb_stats for this domain,
> - * used by wake_affine().
> - */
> - unsigned long nr_running;
> - unsigned long load;
> - unsigned long capacity;
> };
>
> struct sched_domain {
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 70ba32e08a23..66930ce338af 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5356,115 +5356,19 @@ static int wake_wide(struct task_struct *p)
> return 1;
> }
>
> -struct llc_stats {
> - unsigned long nr_running;
> - unsigned long load;
> - unsigned long capacity;
> - int has_capacity;
> -};
> -
> -static bool get_llc_stats(struct llc_stats *stats, int cpu)
> -{
> - struct sched_domain_shared *sds = rcu_dereference(per_cpu(sd_llc_shared, cpu));
> -
> - if (!sds)
> - return false;
> -
> - stats->nr_running = READ_ONCE(sds->nr_running);
> - stats->load = READ_ONCE(sds->load);
> - stats->capacity = READ_ONCE(sds->capacity);
> - stats->has_capacity = stats->nr_running < per_cpu(sd_llc_size, cpu);
> -
> - return true;
> -}
> -
> -/*
> - * Can a task be moved from prev_cpu to this_cpu without causing a load
> - * imbalance that would trigger the load balancer?
> - *
> - * Since we're running on 'stale' values, we might in fact create an imbalance
> - * but recomputing these values is expensive, as that'd mean iteration 2 cache
> - * domains worth of CPUs.
> - */
> -static bool
> -wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
> - int this_cpu, int prev_cpu, int sync)
> -{
> - struct llc_stats prev_stats, this_stats;
> - s64 this_eff_load, prev_eff_load;
> - unsigned long task_load;
> -
> - if (!get_llc_stats(&prev_stats, prev_cpu) ||
> - !get_llc_stats(&this_stats, this_cpu))
> - return false;
> -
> - /*
> - * If sync wakeup then subtract the (maximum possible)
> - * effect of the currently running task from the load
> - * of the current LLC.
> - */
> - if (sync) {
> - unsigned long current_load = task_h_load(current);
> -
> - /* in this case load hits 0 and this LLC is considered 'idle' */
> - if (current_load > this_stats.load)
> - return true;
> -
> - this_stats.load -= current_load;
> - }
> -
> - /*
> - * The has_capacity stuff is not SMT aware, but by trying to balance
> - * the nr_running on both ends we try and fill the domain at equal
> - * rates, thereby first consuming cores before siblings.
> - */
> -
> - /* if the old cache has capacity, stay there */
> - if (prev_stats.has_capacity && prev_stats.nr_running < this_stats.nr_running+1)
> - return false;
> -
> - /* if this cache has capacity, come here */
> - if (this_stats.has_capacity && this_stats.nr_running+1 < prev_stats.nr_running)
> - return true;
> -
> - /*
> - * Check to see if we can move the load without causing too much
> - * imbalance.
> - */
> - task_load = task_h_load(p);
> -
> - this_eff_load = 100;
> - this_eff_load *= prev_stats.capacity;
> -
> - prev_eff_load = 100 + (sd->imbalance_pct - 100) / 2;
> - prev_eff_load *= this_stats.capacity;
> -
> - this_eff_load *= this_stats.load + task_load;
> - prev_eff_load *= prev_stats.load - task_load;
> -
> - return this_eff_load <= prev_eff_load;
> -}
> -
> static int wake_affine(struct sched_domain *sd, struct task_struct *p,
> int prev_cpu, int sync)
> {
> int this_cpu = smp_processor_id();
> - bool affine;
> -
> - /*
> - * Default to no affine wakeups; wake_affine() should not effect a task
> - * placement the load-balancer feels inclined to undo. The conservative
> - * option is therefore to not move tasks when they wake up.
> - */
> - affine = false;
> + bool affine = false;
>
> /*
> - * If the wakeup is across cache domains, try to evaluate if movement
> - * makes sense, otherwise rely on select_idle_siblings() to do
> - * placement inside the cache domain.
> + * If we can run _now_ on the waking CPU, go there, otherwise meh.
> */
> - if (!cpus_share_cache(prev_cpu, this_cpu))
> - affine = wake_affine_llc(sd, p, this_cpu, prev_cpu, sync);
> + if (idle_cpu(this_cpu))
> + affine = true;
> + else if (sync && cpu_rq(this_cpu)->nr_running == 1)
> + affine = true;
>
> schedstat_inc(p->se.statistics.nr_wakeups_affine_attempts);
> if (affine) {
> @@ -7600,7 +7504,6 @@ static inline enum fbq_type fbq_classify_rq(struct rq *rq)
> */
> static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sds)
> {
> - struct sched_domain_shared *shared = env->sd->shared;
> struct sched_domain *child = env->sd->child;
> struct sched_group *sg = env->sd->groups;
> struct sg_lb_stats *local = &sds->local_stat;
> @@ -7672,22 +7575,6 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
> if (env->dst_rq->rd->overload != overload)
> env->dst_rq->rd->overload = overload;
> }
> -
> - if (!shared)
> - return;
> -
> - /*
> - * Since these are sums over groups they can contain some CPUs
> - * multiple times for the NUMA domains.
> - *
> - * Currently only wake_affine_llc() and find_busiest_group()
> - * uses these numbers, only the last is affected by this problem.
> - *
> - * XXX fix that.
> - */
> - WRITE_ONCE(shared->nr_running, sds->total_running);
> - WRITE_ONCE(shared->load, sds->total_load);
> - WRITE_ONCE(shared->capacity, sds->total_capacity);
> }
>
> /**
>
On Wed, 27 Sep 2017 11:35:30 +0200
Peter Zijlstra <[email protected]> wrote:
> On Fri, Sep 22, 2017 at 12:12:45PM -0400, Eric Farman wrote:
> >
> > MySQL. We've tried a few different configs with both test=oltp and
> > test=threads, but both show the same behavior. What I have settled on for
> > my repro is the following:
> >
>
> Right, didn't even need to run it in a guest to observe a regression.
>
> So the below cures native sysbench and NAS bench for me, does it also
> work for you virt thingy?
>
>
> PRE (current tip/master):
>
> ivb-ex sysbench:
>
> 2: [30 secs] transactions: 64110 (2136.94 per sec.)
> 5: [30 secs] transactions: 143644 (4787.99 per sec.)
> 10: [30 secs] transactions: 274298 (9142.93 per sec.)
> 20: [30 secs] transactions: 418683 (13955.45 per sec.)
> 40: [30 secs] transactions: 320731 (10690.15 per sec.)
> 80: [30 secs] transactions: 355096 (11834.28 per sec.)
>
> hsw-ex NAS:
>
> OMP_PROC_BIND/lu.C.x_threads_144_run_1.log: Time in seconds = 18.01
> OMP_PROC_BIND/lu.C.x_threads_144_run_2.log: Time in seconds = 17.89
> OMP_PROC_BIND/lu.C.x_threads_144_run_3.log: Time in seconds = 17.93
> lu.C.x_threads_144_run_1.log: Time in seconds = 434.68
> lu.C.x_threads_144_run_2.log: Time in seconds = 405.36
> lu.C.x_threads_144_run_3.log: Time in seconds = 433.83
>
>
> POST (+patch):
>
> ivb-ex sysbench:
>
> 2: [30 secs] transactions: 64494 (2149.75 per sec.)
> 5: [30 secs] transactions: 145114 (4836.99 per sec.)
> 10: [30 secs] transactions: 278311 (9276.69 per sec.)
> 20: [30 secs] transactions: 437169 (14571.60 per sec.)
> 40: [30 secs] transactions: 669837 (22326.73 per sec.)
> 80: [30 secs] transactions: 631739 (21055.88 per sec.)
>
> hsw-ex NAS:
>
> lu.C.x_threads_144_run_1.log: Time in seconds = 23.36
> lu.C.x_threads_144_run_2.log: Time in seconds = 22.96
> lu.C.x_threads_144_run_3.log: Time in seconds = 22.52
>
>
> This patch takes out all the shiny wake_affine stuff and goes back to
> utter basics. Rik was there another NUMA benchmark that wanted your
> fancy stuff? Because NAS isn't it.
I like the simplicity of your approach! I hope it does not break
stuff like netperf...
I have been working on the patch below, which is much less optimistic
about when to do an affine wakeup than before.
It may be worth testing, in case it works better with some workload,
though relying on cached values still makes me somewhat uneasy.
I will try to get kernels tested here that implement both approaches,
to see what ends up working best.
---8<---
Subject: sched: make wake_affine_llc less eager
With the wake_affine_llc logic, tasks get moved around too eagerly,
and then moved back later, leading to poor performance for some
workloads.
Make wake_affine_llc less eager by comparing the minimum load of
the source LLC with the maximum load of the destination LLC, similar
to how source_load and target_load work for regular migration.
Also, get rid of an overly optimistic test that could potentially
pull across a lot of tasks if the target LLC happened to have fewer
runnable tasks at load balancing time.
Conversely, sync wakeups could happen without taking LLC loads
into account, if the waker would leave an idle CPU behind on
the target LLC.
Signed-off-by: Rik van Riel <[email protected]>
---
include/linux/sched/topology.h | 3 ++-
kernel/sched/fair.c | 56 +++++++++++++++++++++++++++++++++---------
2 files changed, 46 insertions(+), 13 deletions(-)
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index d7b6dab956ec..0c295ff5049b 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -77,7 +77,8 @@ struct sched_domain_shared {
* used by wake_affine().
*/
unsigned long nr_running;
- unsigned long load;
+ unsigned long min_load;
+ unsigned long max_load;
unsigned long capacity;
};
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 86195add977f..7740c6776e08 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5239,6 +5239,23 @@ static unsigned long target_load(int cpu, int type)
return max(rq->cpu_load[type-1], total);
}
+static void min_max_load(int cpu, unsigned long *min_load,
+ unsigned long *max_load)
+{
+ struct rq *rq = cpu_rq(cpu);
+ unsigned long minl = ULONG_MAX;
+ unsigned long maxl = 0;
+ int i;
+
+ for (i = 0; i < CPU_LOAD_IDX_MAX; i++) {
+ minl = min(minl, rq->cpu_load[i]);
+ maxl = max(maxl, rq->cpu_load[i]);
+ }
+
+ *min_load = minl;
+ *max_load = maxl;
+}
+
static unsigned long capacity_of(int cpu)
{
return cpu_rq(cpu)->cpu_capacity;
@@ -5310,7 +5327,8 @@ static int wake_wide(struct task_struct *p)
struct llc_stats {
unsigned long nr_running;
- unsigned long load;
+ unsigned long min_load;
+ unsigned long max_load;
unsigned long capacity;
int has_capacity;
};
@@ -5323,7 +5341,8 @@ static bool get_llc_stats(struct llc_stats *stats, int cpu)
return false;
stats->nr_running = READ_ONCE(sds->nr_running);
- stats->load = READ_ONCE(sds->load);
+ stats->min_load = READ_ONCE(sds->min_load);
+ stats->max_load = READ_ONCE(sds->max_load);
stats->capacity = READ_ONCE(sds->capacity);
stats->has_capacity = stats->nr_running < per_cpu(sd_llc_size, cpu);
@@ -5359,10 +5378,14 @@ wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
unsigned long current_load = task_h_load(current);
/* in this case load hits 0 and this LLC is considered 'idle' */
- if (current_load > this_stats.load)
+ if (current_load > this_stats.max_load)
+ return true;
+
+ /* allow if the CPU would go idle, regardless of LLC load */
+ if (current_load >= target_load(this_cpu, sd->wake_idx))
return true;
- this_stats.load -= current_load;
+ this_stats.max_load -= current_load;
}
/*
@@ -5375,10 +5398,6 @@ wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
if (prev_stats.has_capacity && prev_stats.nr_running < this_stats.nr_running+1)
return false;
- /* if this cache has capacity, come here */
- if (this_stats.has_capacity && this_stats.nr_running+1 < prev_stats.nr_running)
- return true;
-
/*
* Check to see if we can move the load without causing too much
* imbalance.
@@ -5391,8 +5410,8 @@ wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
prev_eff_load = 100 + (sd->imbalance_pct - 100) / 2;
prev_eff_load *= this_stats.capacity;
- this_eff_load *= this_stats.load + task_load;
- prev_eff_load *= prev_stats.load - task_load;
+ this_eff_load *= this_stats.max_load + task_load;
+ prev_eff_load *= prev_stats.min_load - task_load;
return this_eff_load <= prev_eff_load;
}
@@ -7033,6 +7052,8 @@ enum group_type {
struct sg_lb_stats {
unsigned long avg_load; /*Avg load across the CPUs of the group */
unsigned long group_load; /* Total load over the CPUs of the group */
+ unsigned long min_load;
+ unsigned long max_load;
unsigned long sum_weighted_load; /* Weighted load of group's tasks */
unsigned long load_per_task;
unsigned long group_capacity;
@@ -7059,6 +7080,8 @@ struct sd_lb_stats {
unsigned long total_load; /* Total load of all groups in sd */
unsigned long total_capacity; /* Total capacity of all groups in sd */
unsigned long avg_load; /* Average load across all groups in sd */
+ unsigned long min_load; /* Sum of lowest loadavg on CPUs */
+ unsigned long max_load; /* Sum of highest loadavg on CPUs */
struct sg_lb_stats busiest_stat;/* Statistics of the busiest group */
struct sg_lb_stats local_stat; /* Statistics of the local group */
@@ -7077,6 +7100,8 @@ static inline void init_sd_lb_stats(struct sd_lb_stats *sds)
.local = NULL,
.total_running = 0UL,
.total_load = 0UL,
+ .min_load = 0UL,
+ .max_load = 0UL,
.total_capacity = 0UL,
.busiest_stat = {
.avg_load = 0UL,
@@ -7358,7 +7383,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
int local_group, struct sg_lb_stats *sgs,
bool *overload)
{
- unsigned long load;
+ unsigned long load, min_load, max_load;
int i, nr_running;
memset(sgs, 0, sizeof(*sgs));
@@ -7372,7 +7397,11 @@ static inline void update_sg_lb_stats(struct lb_env *env,
else
load = source_load(i, load_idx);
+ min_max_load(i, &min_load, &max_load);
+
sgs->group_load += load;
+ sgs->min_load += min_load;
+ sgs->max_load += max_load;
sgs->group_util += cpu_util(i);
sgs->sum_nr_running += rq->cfs.h_nr_running;
@@ -7569,6 +7598,8 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
/* Now, start updating sd_lb_stats */
sds->total_running += sgs->sum_nr_running;
sds->total_load += sgs->group_load;
+ sds->min_load += sgs->min_load;
+ sds->max_load += sgs->max_load;
sds->total_capacity += sgs->group_capacity;
sg = sg->next;
@@ -7596,7 +7627,8 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
* XXX fix that.
*/
WRITE_ONCE(shared->nr_running, sds->total_running);
- WRITE_ONCE(shared->load, sds->total_load);
+ WRITE_ONCE(shared->min_load, sds->min_load);
+ WRITE_ONCE(shared->max_load, sds->max_load);
WRITE_ONCE(shared->capacity, sds->total_capacity);
}
--
All Rights Reversed
On 09/27/2017 06:27 PM, Eric Farman wrote:
>
>
> On 09/27/2017 05:35 AM, Peter Zijlstra wrote:
>> On Fri, Sep 22, 2017 at 12:12:45PM -0400, Eric Farman wrote:
>>>
>>> MySQL. We've tried a few different configs with both test=oltp and
>>> test=threads, but both show the same behavior. What I have settled on for
>>> my repro is the following:
>>>
>>
>> Right, didn't even need to run it in a guest to observe a regression.
>>
>> So the below cures native sysbench and NAS bench for me, does it also
>> work for you virt thingy?
[...]
> --test=oltp
> Baseline: 5655.26 transactions/second
> Patched: 9618.13 transactions/second
>
> --test=threads
> Baseline: 25482.9 events/sec
> Patched: 29577.9 events/sec
>
> That's good! With that...
>
> Tested-by: Eric Farman <[email protected]>
>
> Thanks!
>
> - Eric
Assuming that we settle on this or Riks alternative patch.
Are we going to schedule this for 4.13 stable as well?
On 09/27/2017 01:58 PM, Rik van Riel wrote:
> On Wed, 27 Sep 2017 11:35:30 +0200
> Peter Zijlstra <[email protected]> wrote:
>
>> On Fri, Sep 22, 2017 at 12:12:45PM -0400, Eric Farman wrote:
>>>
>>> MySQL. We've tried a few different configs with both test=oltp and
>>> test=threads, but both show the same behavior. What I have settled on for
>>> my repro is the following:
>>>
>>
>> Right, didn't even need to run it in a guest to observe a regression.
>>
>> So the below cures native sysbench and NAS bench for me, does it also
>> work for you virt thingy?
>>
>>
>> PRE (current tip/master):
>>
>> ivb-ex sysbench:
>>
>> 2: [30 secs] transactions: 64110 (2136.94 per sec.)
>> 5: [30 secs] transactions: 143644 (4787.99 per sec.)
>> 10: [30 secs] transactions: 274298 (9142.93 per sec.)
>> 20: [30 secs] transactions: 418683 (13955.45 per sec.)
>> 40: [30 secs] transactions: 320731 (10690.15 per sec.)
>> 80: [30 secs] transactions: 355096 (11834.28 per sec.)
>>
>> hsw-ex NAS:
>>
>> OMP_PROC_BIND/lu.C.x_threads_144_run_1.log: Time in seconds = 18.01
>> OMP_PROC_BIND/lu.C.x_threads_144_run_2.log: Time in seconds = 17.89
>> OMP_PROC_BIND/lu.C.x_threads_144_run_3.log: Time in seconds = 17.93
>> lu.C.x_threads_144_run_1.log: Time in seconds = 434.68
>> lu.C.x_threads_144_run_2.log: Time in seconds = 405.36
>> lu.C.x_threads_144_run_3.log: Time in seconds = 433.83
>>
>>
>> POST (+patch):
>>
>> ivb-ex sysbench:
>>
>> 2: [30 secs] transactions: 64494 (2149.75 per sec.)
>> 5: [30 secs] transactions: 145114 (4836.99 per sec.)
>> 10: [30 secs] transactions: 278311 (9276.69 per sec.)
>> 20: [30 secs] transactions: 437169 (14571.60 per sec.)
>> 40: [30 secs] transactions: 669837 (22326.73 per sec.)
>> 80: [30 secs] transactions: 631739 (21055.88 per sec.)
>>
>> hsw-ex NAS:
>>
>> lu.C.x_threads_144_run_1.log: Time in seconds = 23.36
>> lu.C.x_threads_144_run_2.log: Time in seconds = 22.96
>> lu.C.x_threads_144_run_3.log: Time in seconds = 22.52
>>
>>
>> This patch takes out all the shiny wake_affine stuff and goes back to
>> utter basics. Rik was there another NUMA benchmark that wanted your
>> fancy stuff? Because NAS isn't it.
>
> I like the simplicity of your approach! I hope it does not break
> stuff like netperf...
>
> I have been working on the patch below, which is much less optimistic
> about when to do an affine wakeup than before.
>
> It may be worth testing, in case it works better with some workload,
> though relying on cached values still makes me somewhat uneasy.
>
Here are numbers for our environment, to compare the two patches:
sysbench --test=threads:
next-20170926: 25470.8
-with-Peters-patch: 29559.1
-with-Riks-patch: 29283
sysbench --test=oltp:
next-20170926: 5722.37
-with-Peters-patch: 9623.45
-with-Riks-patch: 9360.59
Didn't record host cpu migrations in all scenarios, but a spot check
showed a similar reduction in both patches.
- Eric
> I will try to get kernels tested here that implement both approaches,
> to see what ends up working best.
>
> ---8<---
> Subject: sched: make wake_affine_llc less eager
>
> With the wake_affine_llc logic, tasks get moved around too eagerly,
> and then moved back later, leading to poor performance for some
> workloads.
>
> Make wake_affine_llc less eager by comparing the minimum load of
> the source LLC with the maximum load of the destination LLC, similar
> to how source_load and target_load work for regular migration.
>
> Also, get rid of an overly optimistic test that could potentially
> pull across a lot of tasks if the target LLC happened to have fewer
> runnable tasks at load balancing time.
>
> Conversely, sync wakeups could happen without taking LLC loads
> into account, if the waker would leave an idle CPU behind on
> the target LLC.
>
> Signed-off-by: Rik van Riel <[email protected]>
>
> ---
> include/linux/sched/topology.h | 3 ++-
> kernel/sched/fair.c | 56 +++++++++++++++++++++++++++++++++---------
> 2 files changed, 46 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> index d7b6dab956ec..0c295ff5049b 100644
> --- a/include/linux/sched/topology.h
> +++ b/include/linux/sched/topology.h
> @@ -77,7 +77,8 @@ struct sched_domain_shared {
> * used by wake_affine().
> */
> unsigned long nr_running;
> - unsigned long load;
> + unsigned long min_load;
> + unsigned long max_load;
> unsigned long capacity;
> };
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 86195add977f..7740c6776e08 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5239,6 +5239,23 @@ static unsigned long target_load(int cpu, int type)
> return max(rq->cpu_load[type-1], total);
> }
>
> +static void min_max_load(int cpu, unsigned long *min_load,
> + unsigned long *max_load)
> +{
> + struct rq *rq = cpu_rq(cpu);
> + unsigned long minl = ULONG_MAX;
> + unsigned long maxl = 0;
> + int i;
> +
> + for (i = 0; i < CPU_LOAD_IDX_MAX; i++) {
> + minl = min(minl, rq->cpu_load[i]);
> + maxl = max(maxl, rq->cpu_load[i]);
> + }
> +
> + *min_load = minl;
> + *max_load = maxl;
> +}
> +
> static unsigned long capacity_of(int cpu)
> {
> return cpu_rq(cpu)->cpu_capacity;
> @@ -5310,7 +5327,8 @@ static int wake_wide(struct task_struct *p)
>
> struct llc_stats {
> unsigned long nr_running;
> - unsigned long load;
> + unsigned long min_load;
> + unsigned long max_load;
> unsigned long capacity;
> int has_capacity;
> };
> @@ -5323,7 +5341,8 @@ static bool get_llc_stats(struct llc_stats *stats, int cpu)
> return false;
>
> stats->nr_running = READ_ONCE(sds->nr_running);
> - stats->load = READ_ONCE(sds->load);
> + stats->min_load = READ_ONCE(sds->min_load);
> + stats->max_load = READ_ONCE(sds->max_load);
> stats->capacity = READ_ONCE(sds->capacity);
> stats->has_capacity = stats->nr_running < per_cpu(sd_llc_size, cpu);
>
> @@ -5359,10 +5378,14 @@ wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
> unsigned long current_load = task_h_load(current);
>
> /* in this case load hits 0 and this LLC is considered 'idle' */
> - if (current_load > this_stats.load)
> + if (current_load > this_stats.max_load)
> + return true;
> +
> + /* allow if the CPU would go idle, regardless of LLC load */
> + if (current_load >= target_load(this_cpu, sd->wake_idx))
> return true;
>
> - this_stats.load -= current_load;
> + this_stats.max_load -= current_load;
> }
>
> /*
> @@ -5375,10 +5398,6 @@ wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
> if (prev_stats.has_capacity && prev_stats.nr_running < this_stats.nr_running+1)
> return false;
>
> - /* if this cache has capacity, come here */
> - if (this_stats.has_capacity && this_stats.nr_running+1 < prev_stats.nr_running)
> - return true;
> -
> /*
> * Check to see if we can move the load without causing too much
> * imbalance.
> @@ -5391,8 +5410,8 @@ wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
> prev_eff_load = 100 + (sd->imbalance_pct - 100) / 2;
> prev_eff_load *= this_stats.capacity;
>
> - this_eff_load *= this_stats.load + task_load;
> - prev_eff_load *= prev_stats.load - task_load;
> + this_eff_load *= this_stats.max_load + task_load;
> + prev_eff_load *= prev_stats.min_load - task_load;
>
> return this_eff_load <= prev_eff_load;
> }
> @@ -7033,6 +7052,8 @@ enum group_type {
> struct sg_lb_stats {
> unsigned long avg_load; /*Avg load across the CPUs of the group */
> unsigned long group_load; /* Total load over the CPUs of the group */
> + unsigned long min_load;
> + unsigned long max_load;
> unsigned long sum_weighted_load; /* Weighted load of group's tasks */
> unsigned long load_per_task;
> unsigned long group_capacity;
> @@ -7059,6 +7080,8 @@ struct sd_lb_stats {
> unsigned long total_load; /* Total load of all groups in sd */
> unsigned long total_capacity; /* Total capacity of all groups in sd */
> unsigned long avg_load; /* Average load across all groups in sd */
> + unsigned long min_load; /* Sum of lowest loadavg on CPUs */
> + unsigned long max_load; /* Sum of highest loadavg on CPUs */
>
> struct sg_lb_stats busiest_stat;/* Statistics of the busiest group */
> struct sg_lb_stats local_stat; /* Statistics of the local group */
> @@ -7077,6 +7100,8 @@ static inline void init_sd_lb_stats(struct sd_lb_stats *sds)
> .local = NULL,
> .total_running = 0UL,
> .total_load = 0UL,
> + .min_load = 0UL,
> + .max_load = 0UL,
> .total_capacity = 0UL,
> .busiest_stat = {
> .avg_load = 0UL,
> @@ -7358,7 +7383,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> int local_group, struct sg_lb_stats *sgs,
> bool *overload)
> {
> - unsigned long load;
> + unsigned long load, min_load, max_load;
> int i, nr_running;
>
> memset(sgs, 0, sizeof(*sgs));
> @@ -7372,7 +7397,11 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> else
> load = source_load(i, load_idx);
>
> + min_max_load(i, &min_load, &max_load);
> +
> sgs->group_load += load;
> + sgs->min_load += min_load;
> + sgs->max_load += max_load;
> sgs->group_util += cpu_util(i);
> sgs->sum_nr_running += rq->cfs.h_nr_running;
>
> @@ -7569,6 +7598,8 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
> /* Now, start updating sd_lb_stats */
> sds->total_running += sgs->sum_nr_running;
> sds->total_load += sgs->group_load;
> + sds->min_load += sgs->min_load;
> + sds->max_load += sgs->max_load;
> sds->total_capacity += sgs->group_capacity;
>
> sg = sg->next;
> @@ -7596,7 +7627,8 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
> * XXX fix that.
> */
> WRITE_ONCE(shared->nr_running, sds->total_running);
> - WRITE_ONCE(shared->load, sds->total_load);
> + WRITE_ONCE(shared->min_load, sds->min_load);
> + WRITE_ONCE(shared->max_load, sds->max_load);
> WRITE_ONCE(shared->capacity, sds->total_capacity);
> }
>
>
>
On Wed, Sep 27, 2017 at 01:58:20PM -0400, Rik van Riel wrote:
> I like the simplicity of your approach! I hope it does not break
> stuff like netperf...
So the old approach that looks at the weight of the two CPUs behaves
slightly better in the overloaded case. On the threads==nr_cpus load
points they match fairly evenly.
I seem to have misplaced my netperf scripts, but I'll have a play with
it.
On Wed, Sep 27, 2017 at 01:58:20PM -0400, Rik van Riel wrote:
> @@ -5359,10 +5378,14 @@ wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
> unsigned long current_load = task_h_load(current);
>
> /* in this case load hits 0 and this LLC is considered 'idle' */
> - if (current_load > this_stats.load)
> + if (current_load > this_stats.max_load)
> + return true;
> +
> + /* allow if the CPU would go idle, regardless of LLC load */
> + if (current_load >= target_load(this_cpu, sd->wake_idx))
> return true;
>
> - this_stats.load -= current_load;
> + this_stats.max_load -= current_load;
> }
>
> /*
> @@ -5375,10 +5398,6 @@ wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
> if (prev_stats.has_capacity && prev_stats.nr_running < this_stats.nr_running+1)
> return false;
>
> - /* if this cache has capacity, come here */
> - if (this_stats.has_capacity && this_stats.nr_running+1 < prev_stats.nr_running)
> - return true;
> -
> /*
> * Check to see if we can move the load without causing too much
> * imbalance.
> @@ -5391,8 +5410,8 @@ wake_affine_llc(struct sched_domain *sd, struct task_struct *p,
> prev_eff_load = 100 + (sd->imbalance_pct - 100) / 2;
> prev_eff_load *= this_stats.capacity;
>
> - this_eff_load *= this_stats.load + task_load;
> - prev_eff_load *= prev_stats.load - task_load;
> + this_eff_load *= this_stats.max_load + task_load;
> + prev_eff_load *= prev_stats.min_load - task_load;
>
> return this_eff_load <= prev_eff_load;
> }
So I would really like a workload that needs this LLC/NUMA stuff.
Because I much prefer the simpler: 'on which of these two CPUs can I run
soonest' approach.