From: Yicong Yang <yangyicong@hisilicon.com>

Currently if architectures want to support HOTPLUG_SMT they need to
provide a topology_is_primary_thread() telling the framework which
thread in the SMT cannot offline. However arm64 doesn't have a
restriction on which thread in the SMT cannot offline, a simplest
choice is that just make 1st thread as the "primary" thread. So
just make this as the default implementation in the framework and
let architectures like x86 that have special primary thread to
override this function (which they've already done).

There's no need to provide a stub function if !CONFIG_SMP or
!CONFIG_HOTPLUG_SMP. In such case the testing CPU is already
the 1st CPU in the SMT so it's always the primary thread.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
As questioned in v9 [1] whether this works on architectures not using
CONFIG_GENERIC_ARCH_TOPOLOGY, hacked on LoongArch VM and this also works.
Architectures should use this on their own situation.
[1] https://lore.kernel.org/linux-arm-kernel/427bd639-33c3-47e4-9e83-68c428eb1a7d@arm.com/

[root@localhost smt]# uname -m
loongarch64
[root@localhost smt]# pwd
/sys/devices/system/cpu/smt
[root@localhost smt]# cat ../possible 
0-3
[root@localhost smt]# cat ../online 
0-3
[root@localhost smt]# cat control 
on
[root@localhost smt]# echo off > control 
[root@localhost smt]# cat control 
off
[root@localhost smt]# cat ../online 
0,2
[root@localhost smt]# echo on > control 
[root@localhost smt]# cat control 
on
[root@localhost smt]# cat ../online 
0-3

 arch/powerpc/include/asm/topology.h |  1 +
 arch/x86/include/asm/topology.h     |  2 +-
 include/linux/topology.h            | 22 ++++++++++++++++++++++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index 16bacfe8c7a2..da15b5efe807 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -152,6 +152,7 @@ static inline bool topology_is_primary_thread(unsigned int cpu)
 {
 	return cpu == cpu_first_thread_sibling(cpu);
 }
+#define topology_is_primary_thread topology_is_primary_thread
 
 static inline bool topology_smt_thread_allowed(unsigned int cpu)
 {
diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index fd41103ad342..faa0d6334ea4 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -228,11 +228,11 @@ static inline bool topology_is_primary_thread(unsigned int cpu)
 {
 	return cpumask_test_cpu(cpu, cpu_primary_thread_mask);
 }
+#define topology_is_primary_thread topology_is_primary_thread
 
 #else /* CONFIG_SMP */
 static inline int topology_phys_to_logical_pkg(unsigned int pkg) { return 0; }
 static inline int topology_max_smt_threads(void) { return 1; }
-static inline bool topology_is_primary_thread(unsigned int cpu) { return true; }
 static inline unsigned int topology_amd_nodes_per_pkg(void) { return 1; }
 #endif /* !CONFIG_SMP */
 
diff --git a/include/linux/topology.h b/include/linux/topology.h
index 52f5850730b3..b3aba443c4eb 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -240,6 +240,28 @@ static inline const struct cpumask *cpu_smt_mask(int cpu)
 }
 #endif
 
+#ifndef topology_is_primary_thread
+
+#define topology_is_primary_thread topology_is_primary_thread
+
+static inline bool topology_is_primary_thread(unsigned int cpu)
+{
+	/*
+	 * On SMT hotplug the primary thread of the SMT won't be disabled.
+	 * Architectures do have a special primary thread (e.g. x86) need
+	 * to override this function. Otherwise just make the first thread
+	 * in the SMT as the primary thread.
+	 *
+	 * The sibling cpumask of an offline CPU contains always the CPU
+	 * itself for architectures using CONFIG_GENERIC_ARCH_TOPOLOGY.
+	 * Other architectures should use this depend on their own
+	 * situation.
+	 */
+	return cpu == cpumask_first(topology_sibling_cpumask(cpu));
+}
+
+#endif
+
 static inline const struct cpumask *cpu_cpu_mask(int cpu)
 {
 	return cpumask_of_node(cpu_to_node(cpu));
-- 
2.24.0

On 12/20/24 13:23, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> Currently if architectures want to support HOTPLUG_SMT they need to
> provide a topology_is_primary_thread() telling the framework which
> thread in the SMT cannot offline. However arm64 doesn't have a
> restriction on which thread in the SMT cannot offline, a simplest
> choice is that just make 1st thread as the "primary" thread. So
> just make this as the default implementation in the framework and
> let architectures like x86 that have special primary thread to
> override this function (which they've already done).
> 
> There's no need to provide a stub function if !CONFIG_SMP or
> !CONFIG_HOTPLUG_SMP. In such case the testing CPU is already

s/CONFIG_HOTPLUG_SMP/CONFIG_HOTPLUG_SMT

> the 1st CPU in the SMT so it's always the primary thread.
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
> As questioned in v9 [1] whether this works on architectures not using
> CONFIG_GENERIC_ARCH_TOPOLOGY, hacked on LoongArch VM and this also works.
> Architectures should use this on their own situation.
> [1] https://lore.kernel.org/linux-arm-kernel/427bd639-33c3-47e4-9e83-68c428eb1a7d@arm.com/
> 

sorry to ask this question this late in the series.

I am curious to know why not just add the arch specific 
topology_is_primary_thread? current infra would handle that right?

is there any other arch that is going to enable this soon? or the 
rationale is to add the generic function so that archs that use generic 
topology it is just a kconfig change in case they want to add the support?

> [root@localhost smt]# uname -m
> loongarch64
> [root@localhost smt]# pwd
> /sys/devices/system/cpu/smt
> [root@localhost smt]# cat ../possible
> 0-3
> [root@localhost smt]# cat ../online
> 0-3
> [root@localhost smt]# cat control
> on
> [root@localhost smt]# echo off > control
> [root@localhost smt]# cat control
> off
> [root@localhost smt]# cat ../online
> 0,2
> [root@localhost smt]# echo on > control
> [root@localhost smt]# cat control
> on
> [root@localhost smt]# cat ../online
> 0-3
> 
>   arch/powerpc/include/asm/topology.h |  1 +
>   arch/x86/include/asm/topology.h     |  2 +-
>   include/linux/topology.h            | 22 ++++++++++++++++++++++
>   3 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
> index 16bacfe8c7a2..da15b5efe807 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -152,6 +152,7 @@ static inline bool topology_is_primary_thread(unsigned int cpu)
>   {
>   	return cpu == cpu_first_thread_sibling(cpu);
>   }
> +#define topology_is_primary_thread topology_is_primary_thread
>   
>   static inline bool topology_smt_thread_allowed(unsigned int cpu)
>   {
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index fd41103ad342..faa0d6334ea4 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -228,11 +228,11 @@ static inline bool topology_is_primary_thread(unsigned int cpu)
>   {
>   	return cpumask_test_cpu(cpu, cpu_primary_thread_mask);
>   }
> +#define topology_is_primary_thread topology_is_primary_thread
>   
>   #else /* CONFIG_SMP */
>   static inline int topology_phys_to_logical_pkg(unsigned int pkg) { return 0; }
>   static inline int topology_max_smt_threads(void) { return 1; }
> -static inline bool topology_is_primary_thread(unsigned int cpu) { return true; }
>   static inline unsigned int topology_amd_nodes_per_pkg(void) { return 1; }
>   #endif /* !CONFIG_SMP */
>   
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index 52f5850730b3..b3aba443c4eb 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -240,6 +240,28 @@ static inline const struct cpumask *cpu_smt_mask(int cpu)
>   }
>   #endif
>   
> +#ifndef topology_is_primary_thread
> +
> +#define topology_is_primary_thread topology_is_primary_thread
> +
> +static inline bool topology_is_primary_thread(unsigned int cpu)
> +{
> +	/*
> +	 * On SMT hotplug the primary thread of the SMT won't be disabled.
> +	 * Architectures do have a special primary thread (e.g. x86) need
> +	 * to override this function. Otherwise just make the first thread
> +	 * in the SMT as the primary thread.
> +	 *
> +	 * The sibling cpumask of an offline CPU contains always the CPU
> +	 * itself for architectures using CONFIG_GENERIC_ARCH_TOPOLOGY.
> +	 * Other architectures should use this depend on their own
> +	 * situation.
> +	 */
> +	return cpu == cpumask_first(topology_sibling_cpumask(cpu));
> +}
> +
> +#endif
> +
>   static inline const struct cpumask *cpu_cpu_mask(int cpu)
>   {
>   	return cpumask_of_node(cpu_to_node(cpu));

On 2024/12/26 15:18, Shrikanth Hegde wrote:
> 
> 
> On 12/20/24 13:23, Yicong Yang wrote:
>> From: Yicong Yang <yangyicong@hisilicon.com>
>>
>> Currently if architectures want to support HOTPLUG_SMT they need to
>> provide a topology_is_primary_thread() telling the framework which
>> thread in the SMT cannot offline. However arm64 doesn't have a
>> restriction on which thread in the SMT cannot offline, a simplest
>> choice is that just make 1st thread as the "primary" thread. So
>> just make this as the default implementation in the framework and
>> let architectures like x86 that have special primary thread to
>> override this function (which they've already done).
>>
>> There's no need to provide a stub function if !CONFIG_SMP or
>> !CONFIG_HOTPLUG_SMP. In such case the testing CPU is already
> 
> s/CONFIG_HOTPLUG_SMP/CONFIG_HOTPLUG_SMT
> 
>> the 1st CPU in the SMT so it's always the primary thread.
>>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>> ---
>> As questioned in v9 [1] whether this works on architectures not using
>> CONFIG_GENERIC_ARCH_TOPOLOGY, hacked on LoongArch VM and this also works.
>> Architectures should use this on their own situation.
>> [1] https://lore.kernel.org/linux-arm-kernel/427bd639-33c3-47e4-9e83-68c428eb1a7d@arm.com/
>>
> 
> sorry to ask this question this late in the series.
> 
> I am curious to know why not just add the arch specific topology_is_primary_thread? current infra would handle that right?

Yes, this also works. It was implemented like you said before v4. It's thought trival and was suggested to provide
a default implementation for all and make archs that have special requirement to override it.[1]

[1] https://lore.kernel.org/linux-arm-kernel/fe03425c-6b9a-f0eb-0e8d-e0f47404a7cb@huawei.com/

> 
> is there any other arch that is going to enable this soon? or the rationale is to add the generic function so that archs that use generic topology it is just a kconfig change in case they want to add the support?
> 

It depends on the arch and currently it's only going to be supported on arm64. If they don't have special requirement
of the primary thread, they can use this function. It's still needed to call cpu_smt_set_num_threads() besides select
the kconfig to notify the framework of the thread number since it maybe detected by arthitectural way or ACPI which is
also from the arch codes (like arm64). If it's detected from device tree and using CONFIG_GENERIC_ARCH_TOPOLOGY,
should work to only select kconfig.

Thanks.

>> [root@localhost smt]# uname -m
>> loongarch64
>> [root@localhost smt]# pwd
>> /sys/devices/system/cpu/smt
>> [root@localhost smt]# cat ../possible
>> 0-3
>> [root@localhost smt]# cat ../online
>> 0-3
>> [root@localhost smt]# cat control
>> on
>> [root@localhost smt]# echo off > control
>> [root@localhost smt]# cat control
>> off
>> [root@localhost smt]# cat ../online
>> 0,2
>> [root@localhost smt]# echo on > control
>> [root@localhost smt]# cat control
>> on
>> [root@localhost smt]# cat ../online
>> 0-3
>>
>>   arch/powerpc/include/asm/topology.h |  1 +
>>   arch/x86/include/asm/topology.h     |  2 +-
>>   include/linux/topology.h            | 22 ++++++++++++++++++++++
>>   3 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
>> index 16bacfe8c7a2..da15b5efe807 100644
>> --- a/arch/powerpc/include/asm/topology.h
>> +++ b/arch/powerpc/include/asm/topology.h
>> @@ -152,6 +152,7 @@ static inline bool topology_is_primary_thread(unsigned int cpu)
>>   {
>>       return cpu == cpu_first_thread_sibling(cpu);
>>   }
>> +#define topology_is_primary_thread topology_is_primary_thread
>>     static inline bool topology_smt_thread_allowed(unsigned int cpu)
>>   {
>> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
>> index fd41103ad342..faa0d6334ea4 100644
>> --- a/arch/x86/include/asm/topology.h
>> +++ b/arch/x86/include/asm/topology.h
>> @@ -228,11 +228,11 @@ static inline bool topology_is_primary_thread(unsigned int cpu)
>>   {
>>       return cpumask_test_cpu(cpu, cpu_primary_thread_mask);
>>   }
>> +#define topology_is_primary_thread topology_is_primary_thread
>>     #else /* CONFIG_SMP */
>>   static inline int topology_phys_to_logical_pkg(unsigned int pkg) { return 0; }
>>   static inline int topology_max_smt_threads(void) { return 1; }
>> -static inline bool topology_is_primary_thread(unsigned int cpu) { return true; }
>>   static inline unsigned int topology_amd_nodes_per_pkg(void) { return 1; }
>>   #endif /* !CONFIG_SMP */
>>   diff --git a/include/linux/topology.h b/include/linux/topology.h
>> index 52f5850730b3..b3aba443c4eb 100644
>> --- a/include/linux/topology.h
>> +++ b/include/linux/topology.h
>> @@ -240,6 +240,28 @@ static inline const struct cpumask *cpu_smt_mask(int cpu)
>>   }
>>   #endif
>>   +#ifndef topology_is_primary_thread
>> +
>> +#define topology_is_primary_thread topology_is_primary_thread
>> +
>> +static inline bool topology_is_primary_thread(unsigned int cpu)
>> +{
>> +    /*
>> +     * On SMT hotplug the primary thread of the SMT won't be disabled.
>> +     * Architectures do have a special primary thread (e.g. x86) need
>> +     * to override this function. Otherwise just make the first thread
>> +     * in the SMT as the primary thread.
>> +     *
>> +     * The sibling cpumask of an offline CPU contains always the CPU
>> +     * itself for architectures using CONFIG_GENERIC_ARCH_TOPOLOGY.
>> +     * Other architectures should use this depend on their own
>> +     * situation.
>> +     */
>> +    return cpu == cpumask_first(topology_sibling_cpumask(cpu));
>> +}
>> +
>> +#endif
>> +
>>   static inline const struct cpumask *cpu_cpu_mask(int cpu)
>>   {
>>       return cpumask_of_node(cpu_to_node(cpu));
> 
> .

On 2024/12/20 15:53, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> Currently if architectures want to support HOTPLUG_SMT they need to
> provide a topology_is_primary_thread() telling the framework which
> thread in the SMT cannot offline. However arm64 doesn't have a
> restriction on which thread in the SMT cannot offline, a simplest
> choice is that just make 1st thread as the "primary" thread. So
> just make this as the default implementation in the framework and
> let architectures like x86 that have special primary thread to
> override this function (which they've already done).
> 
> There's no need to provide a stub function if !CONFIG_SMP or
> !CONFIG_HOTPLUG_SMP. In such case the testing CPU is already
> the 1st CPU in the SMT so it's always the primary thread.
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
> As questioned in v9 [1] whether this works on architectures not using
> CONFIG_GENERIC_ARCH_TOPOLOGY, hacked on LoongArch VM and this also works.
> Architectures should use this on their own situation.
> [1] https://lore.kernel.org/linux-arm-kernel/427bd639-33c3-47e4-9e83-68c428eb1a7d@arm.com/
> 
> [root@localhost smt]# uname -m
> loongarch64
> [root@localhost smt]# pwd
> /sys/devices/system/cpu/smt
> [root@localhost smt]# cat ../possible 
> 0-3
> [root@localhost smt]# cat ../online 
> 0-3
> [root@localhost smt]# cat control 
> on
> [root@localhost smt]# echo off > control 
> [root@localhost smt]# cat control 
> off
> [root@localhost smt]# cat ../online 
> 0,2
> [root@localhost smt]# echo on > control 
> [root@localhost smt]# cat control 
> on
> [root@localhost smt]# cat ../online 
> 0-3

Tested with below code using the topology_is_primary_thread() introduced
in this patch. Tested on an ACPI-based QEMU VM emulating SMT2.

Subject: [PATCH] LoongArch: Support HOTPLUG_SMT on ACPI-based system

Support HOTPLUG_SMT on ACPI-based system using generic
topology_is_primary_thread().

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 arch/loongarch/Kconfig       |  1 +
 arch/loongarch/kernel/acpi.c | 26 ++++++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index dae3a9104ca6..bed1b0640b97 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -172,6 +172,7 @@ config LOONGARCH
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_TIF_NOHZ
 	select HAVE_VIRT_CPU_ACCOUNTING_GEN if !SMP
+	select HOTPLUG_SMT if HOTPLUG_CPU
 	select IRQ_FORCED_THREADING
 	select IRQ_LOONGARCH_CPU
 	select LOCK_MM_AND_FIND_VMA
diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
index 382a09a7152c..e642b0de57e7 100644
--- a/arch/loongarch/kernel/acpi.c
+++ b/arch/loongarch/kernel/acpi.c
@@ -15,9 +15,11 @@
 #include <linux/memblock.h>
 #include <linux/of_fdt.h>
 #include <linux/serial_core.h>
+#include <linux/xarray.h>
 #include <asm/io.h>
 #include <asm/numa.h>
 #include <asm/loongson.h>
+#include <linux/cpu_smt.h>

 int acpi_disabled;
 EXPORT_SYMBOL(acpi_disabled);
@@ -175,8 +177,12 @@ int pptt_enabled;

 int __init parse_acpi_topology(void)
 {
+	int thread_num, max_smt_thread_num = 1;
+	struct xarray core_threads;
 	int cpu, topology_id;
+	void *entry;

+	xa_init(&core_threads);
 	for_each_possible_cpu(cpu) {
 		topology_id = find_acpi_cpu_topology(cpu, 0);
 		if (topology_id < 0) {
@@ -184,19 +190,35 @@ int __init parse_acpi_topology(void)
 			return -ENOENT;
 		}

-		if (acpi_pptt_cpu_is_thread(cpu) <= 0)
+		if (acpi_pptt_cpu_is_thread(cpu) <= 0) {
 			cpu_data[cpu].core = topology_id;
-		else {
+		} else {
 			topology_id = find_acpi_cpu_topology(cpu, 1);
 			if (topology_id < 0)
 				return -ENOENT;

 			cpu_data[cpu].core = topology_id;
+
+			entry = xa_load(&core_threads, topology_id);
+			if (!entry) {
+				xa_store(&core_threads, topology_id,
+					 xa_mk_value(1), GFP_KERNEL);
+			} else {
+				thread_num = xa_to_value(entry);
+				thread_num++;
+				xa_store(&core_threads, topology_id,
+					 xa_mk_value(thread_num), GFP_KERNEL);
+
+				if (thread_num > max_smt_thread_num)
+					max_smt_thread_num = thread_num;
+			}
 		}
 	}

 	pptt_enabled = 1;

+	cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
+	xa_destroy(&core_threads);
 	return 0;
 }

-- 
2.24.0

> 
>  arch/powerpc/include/asm/topology.h |  1 +
>  arch/x86/include/asm/topology.h     |  2 +-
>  include/linux/topology.h            | 22 ++++++++++++++++++++++
>  3 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
> index 16bacfe8c7a2..da15b5efe807 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -152,6 +152,7 @@ static inline bool topology_is_primary_thread(unsigned int cpu)
>  {
>  	return cpu == cpu_first_thread_sibling(cpu);
>  }
> +#define topology_is_primary_thread topology_is_primary_thread
>  
>  static inline bool topology_smt_thread_allowed(unsigned int cpu)
>  {
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index fd41103ad342..faa0d6334ea4 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -228,11 +228,11 @@ static inline bool topology_is_primary_thread(unsigned int cpu)
>  {
>  	return cpumask_test_cpu(cpu, cpu_primary_thread_mask);
>  }
> +#define topology_is_primary_thread topology_is_primary_thread
>  
>  #else /* CONFIG_SMP */
>  static inline int topology_phys_to_logical_pkg(unsigned int pkg) { return 0; }
>  static inline int topology_max_smt_threads(void) { return 1; }
> -static inline bool topology_is_primary_thread(unsigned int cpu) { return true; }
>  static inline unsigned int topology_amd_nodes_per_pkg(void) { return 1; }
>  #endif /* !CONFIG_SMP */
>  
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index 52f5850730b3..b3aba443c4eb 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -240,6 +240,28 @@ static inline const struct cpumask *cpu_smt_mask(int cpu)
>  }
>  #endif
>  
> +#ifndef topology_is_primary_thread
> +
> +#define topology_is_primary_thread topology_is_primary_thread
> +
> +static inline bool topology_is_primary_thread(unsigned int cpu)
> +{
> +	/*
> +	 * On SMT hotplug the primary thread of the SMT won't be disabled.
> +	 * Architectures do have a special primary thread (e.g. x86) need
> +	 * to override this function. Otherwise just make the first thread
> +	 * in the SMT as the primary thread.
> +	 *
> +	 * The sibling cpumask of an offline CPU contains always the CPU
> +	 * itself for architectures using CONFIG_GENERIC_ARCH_TOPOLOGY.
> +	 * Other architectures should use this depend on their own
> +	 * situation.
> +	 */
> +	return cpu == cpumask_first(topology_sibling_cpumask(cpu));
> +}
> +
> +#endif
> +
>  static inline const struct cpumask *cpu_cpu_mask(int cpu)
>  {
>  	return cpumask_of_node(cpu_to_node(cpu));
>

On Fri, 20 Dec 2024 15:59:27 +0800
Yicong Yang <yangyicong@huawei.com> wrote:

> On 2024/12/20 15:53, Yicong Yang wrote:
> > From: Yicong Yang <yangyicong@hisilicon.com>
> > 
> > Currently if architectures want to support HOTPLUG_SMT they need to
> > provide a topology_is_primary_thread() telling the framework which
> > thread in the SMT cannot offline. However arm64 doesn't have a
> > restriction on which thread in the SMT cannot offline, a simplest
> > choice is that just make 1st thread as the "primary" thread. So
> > just make this as the default implementation in the framework and
> > let architectures like x86 that have special primary thread to
> > override this function (which they've already done).
> > 
> > There's no need to provide a stub function if !CONFIG_SMP or
> > !CONFIG_HOTPLUG_SMP. In such case the testing CPU is already
> > the 1st CPU in the SMT so it's always the primary thread.
> > 
> > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> > ---
> > As questioned in v9 [1] whether this works on architectures not using
> > CONFIG_GENERIC_ARCH_TOPOLOGY, hacked on LoongArch VM and this also works.
> > Architectures should use this on their own situation.
> > [1] https://lore.kernel.org/linux-arm-kernel/427bd639-33c3-47e4-9e83-68c428eb1a7d@arm.com/
> > 
> > [root@localhost smt]# uname -m
> > loongarch64
> > [root@localhost smt]# pwd
> > /sys/devices/system/cpu/smt
> > [root@localhost smt]# cat ../possible 
> > 0-3
> > [root@localhost smt]# cat ../online 
> > 0-3
> > [root@localhost smt]# cat control 
> > on
> > [root@localhost smt]# echo off > control 
> > [root@localhost smt]# cat control 
> > off
> > [root@localhost smt]# cat ../online 
> > 0,2
> > [root@localhost smt]# echo on > control 
> > [root@localhost smt]# cat control 
> > on
> > [root@localhost smt]# cat ../online 
> > 0-3  
> 
> Tested with below code using the topology_is_primary_thread() introduced
> in this patch. Tested on an ACPI-based QEMU VM emulating SMT2.
Nice bit of testing.

Given it all seems fine. FWIW
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
(for original patch, not the longarch one!)

> 
> Subject: [PATCH] LoongArch: Support HOTPLUG_SMT on ACPI-based system
> 
> Support HOTPLUG_SMT on ACPI-based system using generic
> topology_is_primary_thread().
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
>  arch/loongarch/Kconfig       |  1 +
>  arch/loongarch/kernel/acpi.c | 26 ++++++++++++++++++++++++--
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index dae3a9104ca6..bed1b0640b97 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -172,6 +172,7 @@ config LOONGARCH
>  	select HAVE_SYSCALL_TRACEPOINTS
>  	select HAVE_TIF_NOHZ
>  	select HAVE_VIRT_CPU_ACCOUNTING_GEN if !SMP
> +	select HOTPLUG_SMT if HOTPLUG_CPU
>  	select IRQ_FORCED_THREADING
>  	select IRQ_LOONGARCH_CPU
>  	select LOCK_MM_AND_FIND_VMA
> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
> index 382a09a7152c..e642b0de57e7 100644
> --- a/arch/loongarch/kernel/acpi.c
> +++ b/arch/loongarch/kernel/acpi.c
> @@ -15,9 +15,11 @@
>  #include <linux/memblock.h>
>  #include <linux/of_fdt.h>
>  #include <linux/serial_core.h>
> +#include <linux/xarray.h>
>  #include <asm/io.h>
>  #include <asm/numa.h>
>  #include <asm/loongson.h>
> +#include <linux/cpu_smt.h>
> 
>  int acpi_disabled;
>  EXPORT_SYMBOL(acpi_disabled);
> @@ -175,8 +177,12 @@ int pptt_enabled;
> 
>  int __init parse_acpi_topology(void)
>  {
> +	int thread_num, max_smt_thread_num = 1;
> +	struct xarray core_threads;
>  	int cpu, topology_id;
> +	void *entry;
> 
> +	xa_init(&core_threads);
>  	for_each_possible_cpu(cpu) {
>  		topology_id = find_acpi_cpu_topology(cpu, 0);
>  		if (topology_id < 0) {
> @@ -184,19 +190,35 @@ int __init parse_acpi_topology(void)
>  			return -ENOENT;
>  		}
> 
> -		if (acpi_pptt_cpu_is_thread(cpu) <= 0)
> +		if (acpi_pptt_cpu_is_thread(cpu) <= 0) {
>  			cpu_data[cpu].core = topology_id;
> -		else {
> +		} else {
>  			topology_id = find_acpi_cpu_topology(cpu, 1);
>  			if (topology_id < 0)
>  				return -ENOENT;
> 
>  			cpu_data[cpu].core = topology_id;
> +
> +			entry = xa_load(&core_threads, topology_id);
> +			if (!entry) {
> +				xa_store(&core_threads, topology_id,
> +					 xa_mk_value(1), GFP_KERNEL);
> +			} else {
> +				thread_num = xa_to_value(entry);
> +				thread_num++;
> +				xa_store(&core_threads, topology_id,
> +					 xa_mk_value(thread_num), GFP_KERNEL);
> +
> +				if (thread_num > max_smt_thread_num)
> +					max_smt_thread_num = thread_num;
> +			}
>  		}
>  	}
> 
>  	pptt_enabled = 1;
> 
> +	cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
> +	xa_destroy(&core_threads);
>  	return 0;
>  }
>

On 2024/12/24 0:34, Jonathan Cameron wrote:
> On Fri, 20 Dec 2024 15:59:27 +0800
> Yicong Yang <yangyicong@huawei.com> wrote:
> 
>> On 2024/12/20 15:53, Yicong Yang wrote:
>>> From: Yicong Yang <yangyicong@hisilicon.com>
>>>
>>> Currently if architectures want to support HOTPLUG_SMT they need to
>>> provide a topology_is_primary_thread() telling the framework which
>>> thread in the SMT cannot offline. However arm64 doesn't have a
>>> restriction on which thread in the SMT cannot offline, a simplest
>>> choice is that just make 1st thread as the "primary" thread. So
>>> just make this as the default implementation in the framework and
>>> let architectures like x86 that have special primary thread to
>>> override this function (which they've already done).
>>>
>>> There's no need to provide a stub function if !CONFIG_SMP or
>>> !CONFIG_HOTPLUG_SMP. In such case the testing CPU is already
>>> the 1st CPU in the SMT so it's always the primary thread.
>>>
>>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>>> ---
>>> As questioned in v9 [1] whether this works on architectures not using
>>> CONFIG_GENERIC_ARCH_TOPOLOGY, hacked on LoongArch VM and this also works.
>>> Architectures should use this on their own situation.
>>> [1] https://lore.kernel.org/linux-arm-kernel/427bd639-33c3-47e4-9e83-68c428eb1a7d@arm.com/
>>>
>>> [root@localhost smt]# uname -m
>>> loongarch64
>>> [root@localhost smt]# pwd
>>> /sys/devices/system/cpu/smt
>>> [root@localhost smt]# cat ../possible 
>>> 0-3
>>> [root@localhost smt]# cat ../online 
>>> 0-3
>>> [root@localhost smt]# cat control 
>>> on
>>> [root@localhost smt]# echo off > control 
>>> [root@localhost smt]# cat control 
>>> off
>>> [root@localhost smt]# cat ../online 
>>> 0,2
>>> [root@localhost smt]# echo on > control 
>>> [root@localhost smt]# cat control 
>>> on
>>> [root@localhost smt]# cat ../online 
>>> 0-3  
>>
>> Tested with below code using the topology_is_primary_thread() introduced
>> in this patch. Tested on an ACPI-based QEMU VM emulating SMT2.
> Nice bit of testing.
> 
> Given it all seems fine. FWIW
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> (for original patch, not the longarch one!)

thanks. certainly :)

> 
>>
>> Subject: [PATCH] LoongArch: Support HOTPLUG_SMT on ACPI-based system
>>
>> Support HOTPLUG_SMT on ACPI-based system using generic
>> topology_is_primary_thread().
>>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>> ---
>>  arch/loongarch/Kconfig       |  1 +
>>  arch/loongarch/kernel/acpi.c | 26 ++++++++++++++++++++++++--
>>  2 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>> index dae3a9104ca6..bed1b0640b97 100644
>> --- a/arch/loongarch/Kconfig
>> +++ b/arch/loongarch/Kconfig
>> @@ -172,6 +172,7 @@ config LOONGARCH
>>  	select HAVE_SYSCALL_TRACEPOINTS
>>  	select HAVE_TIF_NOHZ
>>  	select HAVE_VIRT_CPU_ACCOUNTING_GEN if !SMP
>> +	select HOTPLUG_SMT if HOTPLUG_CPU
>>  	select IRQ_FORCED_THREADING
>>  	select IRQ_LOONGARCH_CPU
>>  	select LOCK_MM_AND_FIND_VMA
>> diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
>> index 382a09a7152c..e642b0de57e7 100644
>> --- a/arch/loongarch/kernel/acpi.c
>> +++ b/arch/loongarch/kernel/acpi.c
>> @@ -15,9 +15,11 @@
>>  #include <linux/memblock.h>
>>  #include <linux/of_fdt.h>
>>  #include <linux/serial_core.h>
>> +#include <linux/xarray.h>
>>  #include <asm/io.h>
>>  #include <asm/numa.h>
>>  #include <asm/loongson.h>
>> +#include <linux/cpu_smt.h>
>>
>>  int acpi_disabled;
>>  EXPORT_SYMBOL(acpi_disabled);
>> @@ -175,8 +177,12 @@ int pptt_enabled;
>>
>>  int __init parse_acpi_topology(void)
>>  {
>> +	int thread_num, max_smt_thread_num = 1;
>> +	struct xarray core_threads;
>>  	int cpu, topology_id;
>> +	void *entry;
>>
>> +	xa_init(&core_threads);
>>  	for_each_possible_cpu(cpu) {
>>  		topology_id = find_acpi_cpu_topology(cpu, 0);
>>  		if (topology_id < 0) {
>> @@ -184,19 +190,35 @@ int __init parse_acpi_topology(void)
>>  			return -ENOENT;
>>  		}
>>
>> -		if (acpi_pptt_cpu_is_thread(cpu) <= 0)
>> +		if (acpi_pptt_cpu_is_thread(cpu) <= 0) {
>>  			cpu_data[cpu].core = topology_id;
>> -		else {
>> +		} else {
>>  			topology_id = find_acpi_cpu_topology(cpu, 1);
>>  			if (topology_id < 0)
>>  				return -ENOENT;
>>
>>  			cpu_data[cpu].core = topology_id;
>> +
>> +			entry = xa_load(&core_threads, topology_id);
>> +			if (!entry) {
>> +				xa_store(&core_threads, topology_id,
>> +					 xa_mk_value(1), GFP_KERNEL);
>> +			} else {
>> +				thread_num = xa_to_value(entry);
>> +				thread_num++;
>> +				xa_store(&core_threads, topology_id,
>> +					 xa_mk_value(thread_num), GFP_KERNEL);
>> +
>> +				if (thread_num > max_smt_thread_num)
>> +					max_smt_thread_num = thread_num;
>> +			}
>>  		}
>>  	}
>>
>>  	pptt_enabled = 1;
>>
>> +	cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num);
>> +	xa_destroy(&core_threads);
>>  	return 0;
>>  }
>>
> 
> .
>