Hi,
Strictly speaking those two patches are independent, but I figured it would
make sense to send them together (since one led to the other).
Patch 1 adds a sanity check against EAS + SMT.
Patch 2 enables CONFIG_SCHED_SMT in the arm64 defconfig.
Cheers,
Valentin
Valentin Schneider (2):
sched/topology: Don't enable EAS on SMT systems
arm64: defconfig: enable CONFIG_SCHED_SMT
arch/arm64/configs/defconfig | 1 +
kernel/sched/topology.c | 4 ++++
2 files changed, 5 insertions(+)
--
2.24.0
EAS already requires asymmetric CPU capacities to be enabled, and mixing
this with SMT is an aberration, but better be safe than sorry.
Signed-off-by: Valentin Schneider <[email protected]>
---
kernel/sched/topology.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 00911884b7e7..76cd0a370b9a 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -360,6 +360,10 @@ static bool build_perf_domains(const struct cpumask *cpu_map)
goto free;
}
+ /* EAS definitely does *not* handle SMT */
+ if (sched_smt_active())
+ goto free;
+
for_each_cpu(i, cpu_map) {
/* Skip already covered CPUs. */
if (find_pd(pd, i))
--
2.24.0
The (CFS) scheduler has some extra logic catering to systems with SMT, but
that logic won't be compiled in unless the above config is set.
Note that the SMT-centric codepaths are gated by the sched_smt_present
static key, and the SMT sched_domains will only survive if the platform has
SMT. As such, the only impact on !SMT platforms should be a slightly
bigger kernel - no behavioural change.
Distro kernels already enable it, which makes sense since there already are
things like ThunderX2 out in the wild. Enable it for the defconfig.
Some deltas
===========
FWIW my ELF symbol table diff looks something like this:
NAME BEFORE AFTER DELTA
update_sd_lb_stats.constprop.135 0 1864 +1864
find_idlest_group.isra.115 0 1808 +1808
update_numa_stats.isra.121 0 628 +628
select_task_rq_fair 3236 3732 +496
compute_energy.isra.112 0 420 +420
score_nearby_nodes.part.120 0 380 +380
__update_idle_core 0 232 +232
nohz_balance_exit_idle.part.127 0 216 +216
sched_slice.isra.99 0 172 +172
update_load_avg.part.107 0 116 +116
wakeup_preempt_entity.isra.101 0 92 +92
sched_cpu_activate 340 396 +56
pick_next_task_idle 8 56 +48
sched_cpu_deactivate 252 292 +40
show_smt_active 44 80 +36
cpu_smt_mask 0 28 +28
set_next_task_idle 4 32 +28
task_numa_work 680 692 +12
cpu_smt_flags 0 8 +8
enqueue_task_fair 2608 2612 +4
wakeup_preempt_entity.isra.104 92 0 -92
update_load_avg 1028 932 -96
task_numa_migrate 1824 1728 -96
sched_slice.isra.102 172 0 -172
nohz_balance_exit_idle.part.130 216 0 -216
task_numa_find_cpu 2116 1868 -248
score_nearby_nodes.part.123 380 0 -380
compute_energy.isra.115 420 0 -420
update_numa_stats.isra.124 472 0 -472
find_idlest_group.isra.118 1808 0 -1808
update_sd_lb_stats.constprop.138 1864 0 -1864
------------------------------------------------------------------
DELTA SUM +820
As for the sched_domains, this is on a hikey960:
before:
$ cat /proc/sys/kernel/sched_domain/cpu*/domain*/name | sort | uniq
DIE
MC
after:
$ cat /proc/sys/kernel/sched_domain/cpu*/domain*/name | sort | uniq
DIE
MC
Signed-off-by: Valentin Schneider <[email protected]>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 905109f6814f..3e75007f6592 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -62,6 +62,7 @@ CONFIG_ARCH_ZX=y
CONFIG_ARCH_ZYNQMP=y
CONFIG_ARM64_VA_BITS_48=y
CONFIG_SCHED_MC=y
+CONFIG_SCHED_SMT=y
CONFIG_NUMA=y
CONFIG_SECCOMP=y
CONFIG_KEXEC=y
--
2.24.0
On Wednesday 26 Feb 2020 at 16:41:17 (+0000), Valentin Schneider wrote:
> EAS already requires asymmetric CPU capacities to be enabled, and mixing
> this with SMT is an aberration, but better be safe than sorry.
>
> Signed-off-by: Valentin Schneider <[email protected]>
Acked-by: Quentin Perret <[email protected]>
Thanks,
Quentin
> ---
> kernel/sched/topology.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 00911884b7e7..76cd0a370b9a 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -360,6 +360,10 @@ static bool build_perf_domains(const struct cpumask *cpu_map)
> goto free;
> }
>
> + /* EAS definitely does *not* handle SMT */
> + if (sched_smt_active())
> + goto free;
> +
> for_each_cpu(i, cpu_map) {
> /* Skip already covered CPUs. */
> if (find_pd(pd, i))
> --
> 2.24.0
>
On 27.02.20 13:00, Quentin Perret wrote:
> On Wednesday 26 Feb 2020 at 16:41:17 (+0000), Valentin Schneider wrote:
>> EAS already requires asymmetric CPU capacities to be enabled, and mixing
>> this with SMT is an aberration, but better be safe than sorry.
>>
>> Signed-off-by: Valentin Schneider <[email protected]>
>
> Acked-by: Quentin Perret <[email protected]>
>
> Thanks,
> Quentin
>
>> ---
>> kernel/sched/topology.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
>> index 00911884b7e7..76cd0a370b9a 100644
>> --- a/kernel/sched/topology.c
>> +++ b/kernel/sched/topology.c
>> @@ -360,6 +360,10 @@ static bool build_perf_domains(const struct cpumask *cpu_map)
>> goto free;
>> }
>>
>> + /* EAS definitely does *not* handle SMT */
>> + if (sched_smt_active())
Can you add a pr_warn() and use the current comment as the warning
message? Since we have one for !Asym CPU capacity and !schedutil.
>> + goto free;
>> +
[...]
There is this 'EAS can be used ...' list of currently 4 items in the
build_perf_domains() function header. You could include 'X. No SMT
support' there.
;-)
On 26.02.20 16:41, Valentin Schneider wrote:
> Hi,
>
> Strictly speaking those two patches are independent, but I figured it would
> make sense to send them together (since one led to the other).
>
> Patch 1 adds a sanity check against EAS + SMT.
> Patch 2 enables CONFIG_SCHED_SMT in the arm64 defconfig.
>
> Cheers,
> Valentin
With those small questions in 1/2 and 2/2:
Reviewed-by: "Dietmar Eggemann <[email protected]>"
On Thu, Feb 27 2020, Dietmar Eggemann wrote:
>>> + /* EAS definitely does *not* handle SMT */
>>> + if (sched_smt_active())
>
> Can you add a pr_warn() and use the current comment as the warning
> message? Since we have one for !Asym CPU capacity and !schedutil.
>
>>> + goto free;
>>> +
>
> [...]
>
> There is this 'EAS can be used ...' list of currently 4 items in the
> build_perf_domains() function header. You could include 'X. No SMT
> support' there.
> ;-)
Right, the rst doc says "EAS on SMT is not supported" but I think that can
be interpreted as "EAS on !asym SMT". I'll add the warning and update the
comment.