[MODERATED] Re: [patch V2 01/12] sched/smt: Update sched_smt_present at runtime

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Jun 06, 2018 at 09:27:15PM +0200, speck for Thomas Gleixner wrote:
> Subject: [patch V2 01/12] sched/smt: Update sched_smt_present at runtime
> From: Peter Zijlstra <peterz@infradead.org>
> 
> The static key sched_smt_present is only updated at boot time when SMT
> siblings have been deteced. Booting with maxcpus=1 and bringing the
> siblings online after boot rebuilds the scheduling domains correctly but
> does not update the static key, so the SMT code is not enabled.
> 
> Let the key update in the scheduler CPU hotplug code to fix this.
> 
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

with one question below (not that matters much, but):
> ---
>  kernel/sched/core.c |   32 ++++++++++++++------------------
>  kernel/sched/fair.c |    1 +
>  2 files changed, 15 insertions(+), 18 deletions(-)
> 
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -5771,6 +5771,20 @@ int sched_cpu_activate(unsigned int cpu)
>  	struct rq *rq = cpu_rq(cpu);
>  	struct rq_flags rf;
>  
> +#ifdef CONFIG_SCHED_SMT
> +	/*
> +	 * We can't rely on the sched domains themselves to reliably inform us
> +	 * of SMT, but if we ever see a CPU with siblings pass by, enable the
> +	 * SMT code.
> +	 *
> +	 * The failure case for domains is when we boot with SMT disabled, then
> +	 * create partitions that split all cores and then hotplug the

Don't you nee to first hotplug the siblings, then create the partitions?

Or did you mean 'hotplug' == smp_callin?

> +	 * siblings. In that case we'll never build an SMT domain.
> +	 */
> +	if (cpumask_weight(cpu_smt_mask(cpu)) > 1)
> +		static_branch_enable_cpuslocked(&sched_smt_present);
> +#endif
> +
>  	set_cpu_active(cpu, true);
>  
>  	if (sched_smp_initialized) {
> @@ -5868,22 +5882,6 @@ int sched_cpu_dying(unsigned int cpu)
>  }
>  #endif
>  
> -#ifdef CONFIG_SCHED_SMT
> -DEFINE_STATIC_KEY_FALSE(sched_smt_present);
> -
> -static void sched_init_smt(void)
> -{
> -	/*
> -	 * We've enumerated all CPUs and will assume that if any CPU
> -	 * has SMT siblings, CPU0 will too.
> -	 */
> -	if (cpumask_weight(cpu_smt_mask(0)) > 1)
> -		static_branch_enable(&sched_smt_present);
> -}
> -#else
> -static inline void sched_init_smt(void) { }
> -#endif
> -
>  void __init sched_init_smp(void)
>  {
>  	sched_init_numa();
> @@ -5905,8 +5903,6 @@ void __init sched_init_smp(void)
>  	init_sched_rt_class();
>  	init_sched_dl_class();
>  
> -	sched_init_smt();
> -
>  	sched_smp_initialized = true;
>  }
>  
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6238,6 +6238,7 @@ static inline int find_idlest_cpu(struct
>  }
>  
>  #ifdef CONFIG_SCHED_SMT
> +DEFINE_STATIC_KEY_FALSE(sched_smt_present);
>  
>  static inline void set_idle_cores(int cpu, int val)
>  {
>