Re: [PATCH v11 2/4] arch_topology: Support SMT control for OF based system
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
Date: 2025-02-28 11:11:23
Also in:
linuxppc-dev, lkml
On 18/02/2025 15:10, Yicong Yang wrote:
From: Yicong Yang <yangyicong@hisilicon.com> On building the topology from the devicetree, we've already gotten the SMT thread number of each core. Update the largest SMT thread number and enable the SMT control by the end of topology parsing. The core's SMT control provides two interface to the users [1]: 1) enable/disable SMT by writing on/off 2) enable/disable SMT by writing thread number 1/max_thread_number
1/max_thread_number stands for '1 or max_thread_number', right ? Aren't the two interfaces: (a) /sys/devices/system/cpu/smt/active (b) /sys/devices/system/cpu/smt/control and you write 1) or 2) (or 'forceoff') into (b)?
If a system have more than one SMT thread number the 2) may
s/have/has
not handle it well, since there're multiple thread numbers in the
multiple thread numbers other than 1, right?
system and 2) only accept 1/max_thread_number. So issue a warning to notify the users if such system detected.
This paragraph seems to be about heterogeneous systems. Maybe mention this? Heterogeneous system with SMT only on a subset of cores (like Intel Hybrid): This one works (N threads per core with N=1 and N=2) just fine. But on Arm64 (default) we would still see: [0.075782] Heterogeneous SMT topology is partly supported by SMT control
quoted hunk ↗ jump to hunk
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/testing/sysfs-devices-system-cpu#n542 Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- drivers/base/arch_topology.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index 3ebe77566788..23f425a9d77a 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c@@ -11,6 +11,7 @@ #include <linux/cleanup.h> #include <linux/cpu.h> #include <linux/cpufreq.h> +#include <linux/cpu_smt.h> #include <linux/device.h> #include <linux/of.h> #include <linux/slab.h>@@ -506,6 +507,10 @@ core_initcall(free_raw_capacity); #endif #if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) + +/* Maximum SMT thread number detected used to enable the SMT control */
maybe shorter ? /* used to enable SMT control */
quoted hunk ↗ jump to hunk
+static unsigned int max_smt_thread_num; + /* * This function returns the logic cpu number of the node. * There are basically three kinds of return values:@@ -565,6 +570,16 @@ static int __init parse_core(struct device_node *core, int package_id, i++; } while (1); + /* + * If max_smt_thread_num has been initialized and doesn't match + * the thread number of this entry, then the system has + * heterogeneous SMT topology. + */ + if (max_smt_thread_num && max_smt_thread_num != i) + pr_warn_once("Heterogeneous SMT topology is partly supported by SMT control\n"); + + max_smt_thread_num = max_t(unsigned int, max_smt_thread_num, i); + cpu = get_cpu_for_node(core); if (cpu >= 0) { if (!leaf) {@@ -677,6 +692,18 @@ static int __init parse_socket(struct device_node *socket) if (!has_socket) ret = parse_cluster(socket, 0, -1, 0); + /* + * Notify the CPU framework of the SMT support. Initialize the + * max_smt_thread_num to 1 if no SMT support detected or failed + * to parse the topology. A thread number of 1 can be handled by + * the framework so we don't need to check max_smt_thread_num to + * see we support SMT or not.
Not sure whether the last sentence is needed here? [...]