Re: [PATCH v2 1/2] cpu/SMT: Enable SMT only if a core is online

[Shrikanth Hegde <sshegde@linux.ibm.com>]

On 7/31/24 8:31 AM, Nysal Jan K.A. wrote:
> From: "Nysal Jan K.A" <nysal@linux.ibm.com>
> 
> If a core is offline then enabling SMT should not online CPUs of
> this core. By enabling SMT, what is intended is either changing the SMT
> value from "off" to "on" or setting the SMT level (threads per core) from a
> lower to higher value.
> 
> On PowerPC the ppc64_cpu utility can be used, among other things, to
> perform the following functions:
> 
> ppc64_cpu --cores-on                # Get the number of online cores
> ppc64_cpu --cores-on=X              # Put exactly X cores online
> ppc64_cpu --offline-cores=X[,Y,...] # Put specified cores offline
> ppc64_cpu --smt={on|off|value}      # Enable, disable or change SMT level
> 
> If the user has decided to offline certain cores, enabling SMT should
> not online CPUs in those cores. This patch fixes the issue and changes
> the behaviour as described, by introducing an arch specific function
> topology_is_core_online(). It is currently implemented only for PowerPC.
> 
> Fixes: 73c58e7e1412 ("powerpc: Add HOTPLUG_SMT support")
> Reported-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> Closes: https://groups.google.com/g/powerpc-utils-devel/c/wrwVzAAnRlI/m/5KJSoqP4BAAJ
> Signed-off-by: Nysal Jan K.A <nysal@linux.ibm.com>
> ---
>  Documentation/ABI/testing/sysfs-devices-system-cpu |  3 ++-
>  kernel/cpu.c                                       | 12 +++++++++++-
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
> index 325873385b71..de725ca3be82 100644
> --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
> +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
> @@ -562,7 +562,8 @@ Description:	Control Symmetric Multi Threading (SMT)
>  			 ================ =========================================
>  
>  			 If control status is "forceoff" or "notsupported" writes
> -			 are rejected.
> +			 are rejected. Note that enabling SMT on PowerPC skips
> +			 offline cores.
>  
>  What:		/sys/devices/system/cpu/cpuX/power/energy_perf_bias
>  Date:		March 2019
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 1209ddaec026..b1fd2a3db91a 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -2689,6 +2689,16 @@ int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval)
>  	return ret;
>  }
>  
> +/**
> + * Check if the core a CPU belongs to is online
> + */
> +#if !defined(topology_is_core_online)
> +static inline bool topology_is_core_online(unsigned int cpu)
> +{
> +	return true;
> +}
> +#endif
> +
>  int cpuhp_smt_enable(void)
>  {
>  	int cpu, ret = 0;
> @@ -2699,7 +2709,7 @@ int cpuhp_smt_enable(void)
>  		/* Skip online CPUs and CPUs on offline nodes */
>  		if (cpu_online(cpu) || !node_online(cpu_to_node(cpu)))
>  			continue;
> -		if (!cpu_smt_thread_allowed(cpu))
> +		if (!cpu_smt_thread_allowed(cpu) || !topology_is_core_online(cpu))
>  			continue;
>  		ret = _cpu_up(cpu, 0, CPUHP_ONLINE);
>  		if (ret)

Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>