[PATCH 0/2] sched, arm64: enable CONFIG_SCHED_SMT for arm64

[PATCH 0/2] sched, arm64: enable CONFIG_SCHED_SMT for arm64

[Valentin Schneider]

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

[PATCH 1/2] sched/topology: Don't enable EAS on SMT systems

[Valentin Schneider]

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 <valentin.schneider@arm.com>
---
 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

[PATCH 2/2] arm64: defconfig: enable CONFIG_SCHED_SMT

[Valentin Schneider]

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 <valentin.schneider@arm.com>
---
 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

Re: [PATCH 1/2] sched/topology: Don't enable EAS on SMT systems

[Quentin Perret]

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 <valentin.schneider@arm.com>

Acked-by: Quentin Perret <qperret@google.com>

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
> 

Re: [PATCH 1/2] sched/topology: Don't enable EAS on SMT systems

[Dietmar Eggemann]

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 <valentin.schneider@arm.com>
> 
> Acked-by: Quentin Perret <qperret@google.com>
> 
> 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.
 ;-)

Re: [PATCH 0/2] sched, arm64: enable CONFIG_SCHED_SMT for arm64

[Dietmar Eggemann]

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 <dietmar.eggemann@arm.com>"

Re: [PATCH 1/2] sched/topology: Don't enable EAS on SMT systems

[Valentin Schneider]

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.