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
If a system have more than one SMT thread number the 2) may
not handle it well, since there're multiple thread numbers in the
system and 2) only accept 1/max_thread_number. So issue a warning
to notify the users if such system detected.
[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 | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 3ebe77566788..9e81060144c7 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 */
+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,17 @@ 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");
+
+ if (max_smt_thread_num < i)
+ max_smt_thread_num = i;
+
cpu = get_cpu_for_node(core);
if (cpu >= 0) {
if (!leaf) {
@@ -677,6 +693,16 @@ 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. 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.
+ */
+ if (!max_smt_thread_num)
+ max_smt_thread_num = 1;
+
+ cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
return ret;
}
--
2.24.0On Fri, 20 Dec 2024 15:53:11 +0800
Yicong Yang <yangyicong@huawei.com> 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
>
> If a system have more than one SMT thread number the 2) may
> not handle it well, since there're multiple thread numbers in the
> system and 2) only accept 1/max_thread_number. So issue a warning
> to notify the users if such system detected.
>
> [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>
Hi Yicong,
Apologies that I'm late to the game on this one.
A few comments inline. Only important one is whether to bail out early
on error from parse_cluster()
Thanks,
Jonathan
> ---
> drivers/base/arch_topology.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 3ebe77566788..9e81060144c7 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 */
> +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,17 @@ 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");
> +
> + if (max_smt_thread_num < i)
> + max_smt_thread_num = i;
Maybe more self documenting if you use min()? I'm not sure...
max_smt_thread_num = min(max_smt_thread_num, i);
> +
> cpu = get_cpu_for_node(core);
> if (cpu >= 0) {
> if (!leaf) {
> @@ -677,6 +693,16 @@ static int __init parse_socket(struct device_node *socket)
> if (!has_socket)
> ret = parse_cluster(socket, 0, -1, 0);
Is it appropriate to check ret before setting num threads?
if (!has_socket) {
ret = parse_cluster(socket, 0, -1, 0);
if (ret)
return ret;
}
...
return 0;
>
> + /*
> + * Notify the CPU framework of the SMT support. Initialize the
> + * max_smt_thread_num to 1 if no SMT support detected. 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.
> + */
> + if (!max_smt_thread_num)
> + max_smt_thread_num = 1;
> +
> + cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
Trivial but I'd put a blank line here.
> return ret;
> }
>
On 2024/12/24 0:33, Jonathan Cameron wrote:
> On Fri, 20 Dec 2024 15:53:11 +0800
> Yicong Yang <yangyicong@huawei.com> 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
>>
>> If a system have more than one SMT thread number the 2) may
>> not handle it well, since there're multiple thread numbers in the
>> system and 2) only accept 1/max_thread_number. So issue a warning
>> to notify the users if such system detected.
>>
>> [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>
> Hi Yicong,
>
> Apologies that I'm late to the game on this one.
>
> A few comments inline. Only important one is whether to bail out early
> on error from parse_cluster()
>
thanks for the comments.
> Thanks,
>
> Jonathan
>
>
>> ---
>> drivers/base/arch_topology.c | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>> index 3ebe77566788..9e81060144c7 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 */
>> +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,17 @@ 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");
>> +
>> + if (max_smt_thread_num < i)
>> + max_smt_thread_num = i;
>
> Maybe more self documenting if you use min()? I'm not sure...
> max_smt_thread_num = min(max_smt_thread_num, i);
>
sure, will use max_t here (since we'd like to know the maximum thread number).
>
>> +
>> cpu = get_cpu_for_node(core);
>> if (cpu >= 0) {
>> if (!leaf) {
>> @@ -677,6 +693,16 @@ static int __init parse_socket(struct device_node *socket)
>> if (!has_socket)
>> ret = parse_cluster(socket, 0, -1, 0);
>
> Is it appropriate to check ret before setting num threads?
> if (!has_socket) {
> ret = parse_cluster(socket, 0, -1, 0);
> if (ret)
> return ret;
> }
> ...
>
> return 0;
>
thanks for catching this. since we always need to notify the framework of
the SMT thread number if HOTPLUG_SMT selected, we should make it to 1 if
topology parsing failed. Since on failure the topology information will
be reset, a thread_number of 1 will be handled as SMT not supported.
>>
>> + /*
>> + * Notify the CPU framework of the SMT support. Initialize the
>> + * max_smt_thread_num to 1 if no SMT support detected. 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.
>> + */
>> + if (!max_smt_thread_num)
>> + max_smt_thread_num = 1;
>> +
>> + cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
>
> Trivial but I'd put a blank line here.
>
ok.
Thanks.
© 2016 - 2026 Red Hat, Inc.