[PATCH v2 0/2] Skip offline cores when enabling SMT on PowerPC

[PATCH v2 0/2] Skip offline cores when enabling SMT on PowerPC

[Nysal Jan K.A.]

From: "Nysal Jan K.A" <nysal@linux.ibm.com>

After the addition of HOTPLUG_SMT support for PowerPC [1] there was a
regression reported [2] when enabling SMT. On a system with at least
one offline core, when enabling SMT, the expectation is that no CPUs
of offline cores are made online.

On a POWER9 system with 4 cores in SMT4 mode:
$ ppc64_cpu --info
Core   0:    0*    1*    2*    3*
Core   1:    4*    5*    6*    7*
Core   2:    8*    9*   10*   11*
Core   3:   12*   13*   14*   15*

Turn only one core on:
$ ppc64_cpu --cores-on=1
$ ppc64_cpu --info
Core   0:    0*    1*    2*    3*
Core   1:    4     5     6     7
Core   2:    8     9    10    11
Core   3:   12    13    14    15

Change the SMT level to 2:
$ ppc64_cpu --smt=2
$ ppc64_cpu --info
Core   0:    0*    1*    2     3
Core   1:    4     5     6     7
Core   2:    8     9    10    11
Core   3:   12    13    14    15

As expected we see only two CPUs of core 0 are online

Change the SMT level to 4:
$ ppc64_cpu --smt=4
$ ppc64_cpu --info
Core   0:    0*    1*    2*    3*
Core   1:    4*    5*    6*    7*
Core   2:    8*    9*   10*   11*
Core   3:   12*   13*   14*   15*

The CPUs of offline cores are made online. If a core is offline then
enabling SMT should not online CPUs of this core. An arch specific
function topology_is_core_online() is proposed to address this.

topology_sibling_cpumask() might not be available yet for the to be
brought up CPU, so it cannot be relied upon to detect an offline core.

v2:
 * Document PowerPC specific behaviour when enabling SMT

v1:
 https://lore.kernel.org/all/20240612185046.1826891-1-nysal@linux.ibm.com/

[1] https://lore.kernel.org/lkml/20230705145143.40545-1-ldufour@linux.ibm.com/
[2] https://groups.google.com/g/powerpc-utils-devel/c/wrwVzAAnRlI/m/5KJSoqP4BAAJ

Nysal Jan K.A (2):
  cpu/SMT: Enable SMT only if a core is online
  powerpc/topology: Check if a core is online

 Documentation/ABI/testing/sysfs-devices-system-cpu |  3 ++-
 arch/powerpc/include/asm/topology.h                | 13 +++++++++++++
 kernel/cpu.c                                       | 12 +++++++++++-
 3 files changed, 26 insertions(+), 2 deletions(-)


base-commit: 94ede2a3e9135764736221c080ac7c0ad993dc2d
-- 
2.35.3

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

[Nysal Jan K.A.]

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)
-- 
2.35.3

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

[Shrikanth Hegde]

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>

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

[Thomas Gleixner]

On Wed, Jul 31 2024 at 08:31, Nysal Jan K. A. wrote:
> 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>

Assuming this goes through the PPC tree:

  Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

Michael: If I should route it through my tree, let me know.

Thanks,

        tglx

[PATCH v2 2/2] powerpc/topology: Check if a core is online

[Nysal Jan K.A.]

From: "Nysal Jan K.A" <nysal@linux.ibm.com>

topology_is_core_online() checks if the core a CPU belongs to
is online. The core is online if at least one of the sibling
CPUs is online. The first CPU of an online core is also online
in the common case, so this should be fairly quick.

Signed-off-by: Nysal Jan K.A <nysal@linux.ibm.com>
---
 arch/powerpc/include/asm/topology.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index f4e6f2dd04b7..16bacfe8c7a2 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -145,6 +145,7 @@ static inline int cpu_to_coregroup_id(int cpu)
 
 #ifdef CONFIG_HOTPLUG_SMT
 #include <linux/cpu_smt.h>
+#include <linux/cpumask.h>
 #include <asm/cputhreads.h>
 
 static inline bool topology_is_primary_thread(unsigned int cpu)
@@ -156,6 +157,18 @@ static inline bool topology_smt_thread_allowed(unsigned int cpu)
 {
 	return cpu_thread_in_core(cpu) < cpu_smt_num_threads;
 }
+
+#define topology_is_core_online topology_is_core_online
+static inline bool topology_is_core_online(unsigned int cpu)
+{
+	int i, first_cpu = cpu_first_thread_sibling(cpu);
+
+	for (i = first_cpu; i < first_cpu + threads_per_core; ++i) {
+		if (cpu_online(i))
+			return true;
+	}
+	return false;
+}
 #endif
 
 #endif /* __KERNEL__ */
-- 
2.35.3

Re: [PATCH v2 2/2] powerpc/topology: Check if a core is online

[Shrikanth Hegde]

On 7/31/24 8:31 AM, Nysal Jan K.A. wrote:
> From: "Nysal Jan K.A" <nysal@linux.ibm.com>
> 
> topology_is_core_online() checks if the core a CPU belongs to
> is online. The core is online if at least one of the sibling
> CPUs is online. The first CPU of an online core is also online
> in the common case, so this should be fairly quick.
> 
> Signed-off-by: Nysal Jan K.A <nysal@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/topology.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
> index f4e6f2dd04b7..16bacfe8c7a2 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -145,6 +145,7 @@ static inline int cpu_to_coregroup_id(int cpu)
>  
>  #ifdef CONFIG_HOTPLUG_SMT
>  #include <linux/cpu_smt.h>
> +#include <linux/cpumask.h>
>  #include <asm/cputhreads.h>
>  
>  static inline bool topology_is_primary_thread(unsigned int cpu)
> @@ -156,6 +157,18 @@ static inline bool topology_smt_thread_allowed(unsigned int cpu)
>  {
>  	return cpu_thread_in_core(cpu) < cpu_smt_num_threads;
>  }
> +
> +#define topology_is_core_online topology_is_core_online
> +static inline bool topology_is_core_online(unsigned int cpu)
> +{
> +	int i, first_cpu = cpu_first_thread_sibling(cpu);
> +
> +	for (i = first_cpu; i < first_cpu + threads_per_core; ++i) {
> +		if (cpu_online(i))
> +			return true;
> +	}
> +	return false;
> +}
>  #endif
>  
>  #endif /* __KERNEL__ */

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

Re: [PATCH v2 0/2] Skip offline cores when enabling SMT on PowerPC

[Michael Ellerman]

On Wed, 31 Jul 2024 08:31:11 +0530, Nysal Jan K.A. wrote:
> After the addition of HOTPLUG_SMT support for PowerPC [1] there was a
> regression reported [2] when enabling SMT. On a system with at least
> one offline core, when enabling SMT, the expectation is that no CPUs
> of offline cores are made online.
> 
> On a POWER9 system with 4 cores in SMT4 mode:
> $ ppc64_cpu --info
> Core   0:    0*    1*    2*    3*
> Core   1:    4*    5*    6*    7*
> Core   2:    8*    9*   10*   11*
> Core   3:   12*   13*   14*   15*
> 
> [...]

Applied to powerpc/fixes.

[1/2] cpu/SMT: Enable SMT only if a core is online
      https://git.kernel.org/powerpc/c/6c17ea1f3eaa330d445ac14a9428402ce4e3055e
[2/2] powerpc/topology: Check if a core is online
      https://git.kernel.org/powerpc/c/227bbaabe64b6f9cd98aa051454c1d4a194a8c6a

cheers