| Message ID | 20241114141127.23232-2-yangyicong@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support SMT control on arm64 | expand |
Hello Yicong, On 11/14/24 15:11, 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> > --- > arch/powerpc/include/asm/topology.h | 1 + > arch/x86/include/asm/topology.h | 2 +- > include/linux/topology.h | 20 ++++++++++++++++++++ > 3 files changed, 22 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 92f3664dd933..d84d9b6d8678 100644 > --- a/arch/x86/include/asm/topology.h > +++ b/arch/x86/include/asm/topology.h > @@ -219,11 +219,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..b8e860276518 100644 > --- a/include/linux/topology.h > +++ b/include/linux/topology.h > @@ -240,6 +240,26 @@ 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. As Thomas suggested, would it be possible to check it for other architectures ? For instance for loongarch at arch/loongarch/kernel/smp.c, clear_cpu_sibling_map() seems to completely clear &cpu_sibling_map[cpu] when a CPU is put offline. This would make topology_sibling_cpumask(cpu) to be empty and cpu_bootable() return false if the CPU never booted before. Personal note: cpu_bootable() is called from an already online CPU: cpu_bootable (kernel/cpu.c:678) cpu_up (kernel/cpu.c:1722 kernel/cpu.c:1702) bringup_nonboot_cpus (kernel/cpu.c:1793 kernel/cpu.c:1901) smp_init (./include/linux/bitmap.h:445 ./include/linux/nodemask.h:241 ./include/linux/nodemask.h:438 kernel/smp.c:1011) kernel_init_freeable (init/main.c:1573) kernel_init (init/main.c:1473) ret_from_fork (arch/arm64/kernel/entry.S:861) store_cpu_topology() for arm64 is called from the booting CPU: store_cpu_topology (drivers/base/arch_topology.c:921) secondary_start_kernel (arch/arm64/kernel/smp.c:251) __secondary_switched (arch/arm64/kernel/head.S:418) > + */ > + 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/11/15 17:42, Pierre Gondois wrote: > Hello Yicong, > > > On 11/14/24 15:11, 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> >> --- >> arch/powerpc/include/asm/topology.h | 1 + >> arch/x86/include/asm/topology.h | 2 +- >> include/linux/topology.h | 20 ++++++++++++++++++++ >> 3 files changed, 22 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 92f3664dd933..d84d9b6d8678 100644 >> --- a/arch/x86/include/asm/topology.h >> +++ b/arch/x86/include/asm/topology.h >> @@ -219,11 +219,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..b8e860276518 100644 >> --- a/include/linux/topology.h >> +++ b/include/linux/topology.h >> @@ -240,6 +240,26 @@ 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. > > As Thomas suggested, would it be possible to check it for other > architectures ? > For instance for loongarch at arch/loongarch/kernel/smp.c, > clear_cpu_sibling_map() seems to completely clear &cpu_sibling_map[cpu] > when a CPU is put offline. This would make topology_sibling_cpumask(cpu) > to be empty and cpu_bootable() return false if the CPU never booted before. > cpu_bootable() only affects architectures select HOTPLUG_SMT, otherwise it'll always return true. Since x86 and powerpc have their own illustration of primary thread and have an override version of this funciton, arm64 is the only user now by this patchset. We have this guarantee for arm64 and also for other architectures using arch_topology.c (see clear_cpu_topology()). So if loogarch has a different implementation, they should implement a topology_is_primary_thread() variant to support HOTPLUG_SMT. > Personal note: > cpu_bootable() is called from an already online CPU: > cpu_bootable (kernel/cpu.c:678) > cpu_up (kernel/cpu.c:1722 kernel/cpu.c:1702) > bringup_nonboot_cpus (kernel/cpu.c:1793 kernel/cpu.c:1901) > smp_init (./include/linux/bitmap.h:445 ./include/linux/nodemask.h:241 ./include/linux/nodemask.h:438 kernel/smp.c:1011) > kernel_init_freeable (init/main.c:1573) > kernel_init (init/main.c:1473) > ret_from_fork (arch/arm64/kernel/entry.S:861) > > store_cpu_topology() for arm64 is called from the booting CPU: > store_cpu_topology (drivers/base/arch_topology.c:921) > secondary_start_kernel (arch/arm64/kernel/smp.c:251) > __secondary_switched (arch/arm64/kernel/head.S:418) > >> + */ >> + 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 18/11/2024 11:50, Yicong Yang wrote: > On 2024/11/15 17:42, Pierre Gondois wrote: >> Hello Yicong, >> >> >> On 11/14/24 15:11, Yicong Yang wrote: >>> From: Yicong Yang <yangyicong@hisilicon.com> [...] >>> diff --git a/include/linux/topology.h b/include/linux/topology.h >>> index 52f5850730b3..b8e860276518 100644 >>> --- a/include/linux/topology.h >>> +++ b/include/linux/topology.h >>> @@ -240,6 +240,26 @@ 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. >> >> As Thomas suggested, would it be possible to check it for other >> architectures ? >> For instance for loongarch at arch/loongarch/kernel/smp.c, >> clear_cpu_sibling_map() seems to completely clear &cpu_sibling_map[cpu] >> when a CPU is put offline. This would make topology_sibling_cpumask(cpu) >> to be empty and cpu_bootable() return false if the CPU never booted before. >> > > cpu_bootable() only affects architectures select HOTPLUG_SMT, otherwise it'll always > return true. Since x86 and powerpc have their own illustration of primary thread and > have an override version of this funciton, arm64 is the only user now by this patchset. > We have this guarantee for arm64 and also for other architectures using arch_topology.c > (see clear_cpu_topology()). So if loogarch has a different implementation, they > should implement a topology_is_primary_thread() variant to support HOTPLUG_SMT. I also stumbled over this sentence. drivers/base/arch_topology.c: void clear_cpu_topology(int cpu) (2) ... cpumask_set_cpu(cpu, &cpu_topo->thread_sibling) (4) void __init reset_cpu_topology(void) (1) for_each_possible_cpu(cpu) ... clear_cpu_topology(cpu) (2) #if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) (3) void __init init_cpu_topology(void) reset_cpu_topology() (1) ... Does this mean the default implementation relies on (4), i.e. is only valid for arm64 and riscv? (3) Do all the other archs then have to overwrite the default implementation (like x86 and powerpc) if they want to implement CONFIG_HOTPLUG_SMT? [...]
On 2024/11/18 23:04, Dietmar Eggemann wrote: > On 18/11/2024 11:50, Yicong Yang wrote: >> On 2024/11/15 17:42, Pierre Gondois wrote: >>> Hello Yicong, >>> >>> >>> On 11/14/24 15:11, Yicong Yang wrote: >>>> From: Yicong Yang <yangyicong@hisilicon.com> > > [...] > >>>> diff --git a/include/linux/topology.h b/include/linux/topology.h >>>> index 52f5850730b3..b8e860276518 100644 >>>> --- a/include/linux/topology.h >>>> +++ b/include/linux/topology.h >>>> @@ -240,6 +240,26 @@ 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. >>> >>> As Thomas suggested, would it be possible to check it for other >>> architectures ? >>> For instance for loongarch at arch/loongarch/kernel/smp.c, >>> clear_cpu_sibling_map() seems to completely clear &cpu_sibling_map[cpu] >>> when a CPU is put offline. This would make topology_sibling_cpumask(cpu) >>> to be empty and cpu_bootable() return false if the CPU never booted before. >>> >> >> cpu_bootable() only affects architectures select HOTPLUG_SMT, otherwise it'll always >> return true. Since x86 and powerpc have their own illustration of primary thread and >> have an override version of this funciton, arm64 is the only user now by this patchset. >> We have this guarantee for arm64 and also for other architectures using arch_topology.c >> (see clear_cpu_topology()). So if loogarch has a different implementation, they >> should implement a topology_is_primary_thread() variant to support HOTPLUG_SMT. > > I also stumbled over this sentence. > > drivers/base/arch_topology.c: > > void clear_cpu_topology(int cpu) (2) > > ... > cpumask_set_cpu(cpu, &cpu_topo->thread_sibling) (4) > > void __init reset_cpu_topology(void) (1) > > for_each_possible_cpu(cpu) > > ... > clear_cpu_topology(cpu) (2) > > #if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) (3) > void __init init_cpu_topology(void) > > reset_cpu_topology() (1) > ... > > Does this mean the default implementation relies on (4), i.e. is only > valid for arm64 and riscv? (3) > Do all the other archs then have to overwrite the default implementation > (like x86 and powerpc) if they want to implement CONFIG_HOTPLUG_SMT? > I think yes if they have problems with the default implementation. That's what used to be to support HOTPLUG_SMT before this, each arthitecture provides their own variant of topology_is_primary_thread. The current approach may also work since cpu_bootable() will make all the CPUs boot at least once: static inline bool cpu_bootable(unsigned int cpu) { [...] if (topology_is_primary_thread(cpu)) return true; return !cpumask_test_cpu(cpu, &cpus_booted_once_mask); // True if not booted } The boot process will be like below. cpu_bootable() is checked twice: -> cpu_up() cpu_bootable() (1) [...] cpuhp_bringup_ap() [ archs usually update the sibling_mask in start_secondary() here ] bringup_wait_for_ap_online() if (!cpu_bootable(cpu)) (2) return -ECANCELED // roll back and offline this CPU So an empty mask in (1) won't block the CPU going online. And the default topology_is_primary_thread() should work if sibling mask updated before the checking in (2). I hacked x86 to use the default topology_is_primary_thread and tested on a qemu vm and it seems also work (just for test since the primary thread should not always be the 1st thread in the core on x86, not quite sure). Thanks.
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 92f3664dd933..d84d9b6d8678 100644 --- a/arch/x86/include/asm/topology.h +++ b/arch/x86/include/asm/topology.h @@ -219,11 +219,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..b8e860276518 100644 --- a/include/linux/topology.h +++ b/include/linux/topology.h @@ -240,6 +240,26 @@ 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. + */ + 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));