I'm taking over the series Michael sent previously [1] which is smartly
reviewing the initial series I sent [2]. This series is addressing the
comments sent by Thomas and me on the Michael's one.
Here is a short introduction to the issue this series is addressing:
When a new CPU is added, the kernel is activating all its threads. This
leads to weird, but functional, result when adding CPU on a SMT 4 system
for instance.
Here the newly added CPU 1 has 8 threads while the other one has 4 threads
active (system has been booted with the 'smt-enabled=4' kernel option):
ltcden3-lp12:~ # ppc64_cpu --info
Core 0: 0* 1* 2* 3* 4 5 6 7
Core 1: 8* 9* 10* 11* 12* 13* 14* 15*
This mixed SMT level may confused end users and/or some applications.
There is no SMT level recorded in the kernel (common code), neither in user
space, as far as I know. Such a level is helpful when adding new CPU or
when optimizing the energy efficiency (when reactivating CPUs).
When SMP and HOTPLUG_SMT are defined, this series is adding a new SMT level
(cpu_smt_num_threads) and few callbacks allowing the architecture code to
fine control this value, setting a max and a "at boot" level, and
controling whether a thread should be onlined or not.
v3:
Fix a build error in the patch 6/9
v2:
As Thomas suggested,
Reword some commit's description
Remove topology_smt_supported()
Remove topology_smt_threads_supported()
Introduce CONFIG_SMT_NUM_THREADS_DYNAMIC
Remove switch() in __store_smt_control()
Update kernel-parameters.txt
[1] https://lore.kernel.org/linuxppc-dev/[email protected]/
[2] https://lore.kernel.org/linuxppc-dev/[email protected]/
Laurent Dufour (1):
cpu/SMT: Remove topology_smt_supported()
Michael Ellerman (8):
cpu/SMT: Move SMT prototypes into cpu_smt.h
cpu/SMT: Move smt/control simple exit cases earlier
cpu/SMT: Store the current/max number of threads
cpu/SMT: Create topology_smt_thread_allowed()
cpu/SMT: Allow enabling partial SMT states via sysfs
powerpc/pseries: Initialise CPU hotplug callbacks earlier
powerpc: Add HOTPLUG_SMT support
powerpc/pseries: Honour current SMT state when DLPAR onlining CPUs
.../ABI/testing/sysfs-devices-system-cpu | 1 +
.../admin-guide/kernel-parameters.txt | 4 +-
arch/Kconfig | 3 +
arch/powerpc/Kconfig | 2 +
arch/powerpc/include/asm/topology.h | 15 +++
arch/powerpc/kernel/smp.c | 8 +-
arch/powerpc/platforms/pseries/hotplug-cpu.c | 30 +++--
arch/powerpc/platforms/pseries/pseries.h | 2 +
arch/powerpc/platforms/pseries/setup.c | 2 +
arch/x86/include/asm/topology.h | 4 +-
arch/x86/kernel/cpu/bugs.c | 3 +-
arch/x86/kernel/smpboot.c | 8 --
include/linux/cpu.h | 25 +---
include/linux/cpu_smt.h | 33 +++++
kernel/cpu.c | 118 ++++++++++++++----
15 files changed, 187 insertions(+), 71 deletions(-)
create mode 100644 include/linux/cpu_smt.h
--
2.41.0
From: Michael Ellerman <[email protected]>
Some architectures allows partial SMT states, ie. when not all SMT
threads are brought online.
To support that, add an architecture helper which checks whether a given
CPU is allowed to be brought online depending on how many SMT threads are
currently enabled. Since this is only applicable to architecture supporting
partial SMT, only these architectures should select the new configuration
variable CONFIG_SMT_NUM_THREADS_DYNAMIC. For the other architectures, not
supporting the partial SMT states, there is no need to define
topology_cpu_smt_allowed(), the generic code assumed that all the threads
are allowed or only the primary ones.
Call the helper from cpu_smt_enable(), and cpu_smt_allowed() when SMT is
enabled, to check if the particular thread should be onlined. Notably,
also call it from cpu_smt_disable() if CPU_SMT_ENABLED, to allow
offlining some threads to move from a higher to lower number of threads
online.
Signed-off-by: Michael Ellerman <[email protected]>
Suggested-by: Thomas Gleixner <[email protected]>
[ldufour: slightly reword the commit's description]
[ldufour: introduce CONFIG_SMT_NUM_THREADS_DYNAMIC]
Signed-off-by: Laurent Dufour <[email protected]>
---
arch/Kconfig | 3 +++
kernel/cpu.c | 24 +++++++++++++++++++++++-
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 205fd23e0cad..c69e9c662a87 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -34,6 +34,9 @@ config ARCH_HAS_SUBPAGE_FAULTS
config HOTPLUG_SMT
bool
+config SMT_NUM_THREADS_DYNAMIC
+ bool
+
config GENERIC_ENTRY
bool
diff --git a/kernel/cpu.c b/kernel/cpu.c
index e354af92b2b8..29bf310651c6 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -466,9 +466,23 @@ static int __init smt_cmdline_disable(char *str)
}
early_param("nosmt", smt_cmdline_disable);
+/*
+ * For Archicture supporting partial SMT states check if the thread is allowed.
+ * Otherwise this has already been checked through cpu_smt_max_threads when
+ * setting the SMT level.
+ */
+static inline bool cpu_smt_thread_allowed(unsigned int cpu)
+{
+#ifdef CONFIG_SMT_NUM_THREADS_DYNAMIC
+ return topology_smt_thread_allowed(cpu);
+#else
+ return true;
+#endif
+}
+
static inline bool cpu_smt_allowed(unsigned int cpu)
{
- if (cpu_smt_control == CPU_SMT_ENABLED)
+ if (cpu_smt_control == CPU_SMT_ENABLED && cpu_smt_thread_allowed(cpu))
return true;
if (topology_is_primary_thread(cpu))
@@ -2283,6 +2297,12 @@ int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval)
for_each_online_cpu(cpu) {
if (topology_is_primary_thread(cpu))
continue;
+ /*
+ * Disable can be called with CPU_SMT_ENABLED when changing
+ * from a higher to lower number of SMT threads per core.
+ */
+ if (ctrlval == CPU_SMT_ENABLED && cpu_smt_thread_allowed(cpu))
+ continue;
ret = cpu_down_maps_locked(cpu, CPUHP_OFFLINE);
if (ret)
break;
@@ -2317,6 +2337,8 @@ 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))
+ continue;
ret = _cpu_up(cpu, 0, CPUHP_ONLINE);
if (ret)
break;
--
2.41.0
From: Michael Ellerman <[email protected]>
Add support to the /sys/devices/system/cpu/smt/control interface for
enabling a specified number of SMT threads per core, including partial
SMT states where not all threads are brought online.
The current interface accepts "on" and "off", to enable either 1 or all
SMT threads per core.
This commit allows writing an integer, between 1 and the number of SMT
threads supported by the machine. Writing 1 is a synonym for "off", 2 or
more enables SMT with the specified number of threads.
When reading the file, if all threads are online "on" is returned, to
avoid changing behaviour for existing users. If some other number of
threads is online then the integer value is returned.
Architectures like x86 only supporting 1 thread or all threads, should not
define CONFIG_SMT_NUM_THREADS_DYNAMIC. Architecture supporting partial SMT
states, like PowerPC, should define it.
Signed-off-by: Michael Ellerman <[email protected]>
[ldufour: slightly reword the commit's description]
[ldufour: remove switch() in __store_smt_control()]
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
[ldufour: fix build issue in control_show()]
Signed-off-by: Laurent Dufour <[email protected]>
---
.../ABI/testing/sysfs-devices-system-cpu | 1 +
kernel/cpu.c | 60 ++++++++++++++-----
2 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index f54867cadb0f..3c4cfb59d495 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -555,6 +555,7 @@ Description: Control Symmetric Multi Threading (SMT)
================ =========================================
"on" SMT is enabled
"off" SMT is disabled
+ "<N>" SMT is enabled with N threads per core.
"forceoff" SMT is force disabled. Cannot be changed.
"notsupported" SMT is not supported by the CPU
"notimplemented" SMT runtime toggling is not
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 29bf310651c6..d63f633e34cd 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2517,11 +2517,19 @@ static const struct attribute_group cpuhp_cpu_root_attr_group = {
#ifdef CONFIG_HOTPLUG_SMT
+static bool cpu_smt_num_threads_valid(unsigned int threads)
+{
+ if (IS_ENABLED(CONFIG_SMT_NUM_THREADS_DYNAMIC))
+ return threads >= 1 && threads <= cpu_smt_max_threads;
+ return threads == 1 || threads == cpu_smt_max_threads;
+}
+
static ssize_t
__store_smt_control(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
- int ctrlval, ret;
+ int ctrlval, ret, num_threads, orig_threads;
+ bool force_off;
if (cpu_smt_control == CPU_SMT_FORCE_DISABLED)
return -EPERM;
@@ -2529,30 +2537,39 @@ __store_smt_control(struct device *dev, struct device_attribute *attr,
if (cpu_smt_control == CPU_SMT_NOT_SUPPORTED)
return -ENODEV;
- if (sysfs_streq(buf, "on"))
+ if (sysfs_streq(buf, "on")) {
ctrlval = CPU_SMT_ENABLED;
- else if (sysfs_streq(buf, "off"))
+ num_threads = cpu_smt_max_threads;
+ } else if (sysfs_streq(buf, "off")) {
ctrlval = CPU_SMT_DISABLED;
- else if (sysfs_streq(buf, "forceoff"))
+ num_threads = 1;
+ } else if (sysfs_streq(buf, "forceoff")) {
ctrlval = CPU_SMT_FORCE_DISABLED;
- else
+ num_threads = 1;
+ } else if (kstrtoint(buf, 10, &num_threads) == 0) {
+ if (num_threads == 1)
+ ctrlval = CPU_SMT_DISABLED;
+ else if (cpu_smt_num_threads_valid(num_threads))
+ ctrlval = CPU_SMT_ENABLED;
+ else
+ return -EINVAL;
+ } else {
return -EINVAL;
+ }
ret = lock_device_hotplug_sysfs();
if (ret)
return ret;
- if (ctrlval != cpu_smt_control) {
- switch (ctrlval) {
- case CPU_SMT_ENABLED:
- ret = cpuhp_smt_enable();
- break;
- case CPU_SMT_DISABLED:
- case CPU_SMT_FORCE_DISABLED:
- ret = cpuhp_smt_disable(ctrlval);
- break;
- }
- }
+ orig_threads = cpu_smt_num_threads;
+ cpu_smt_num_threads = num_threads;
+
+ force_off = ctrlval != cpu_smt_control && ctrlval == CPU_SMT_FORCE_DISABLED;
+
+ if (num_threads > orig_threads)
+ ret = cpuhp_smt_enable();
+ else if (num_threads < orig_threads || force_off)
+ ret = cpuhp_smt_disable(ctrlval);
unlock_device_hotplug();
return ret ? ret : count;
@@ -2580,6 +2597,17 @@ static ssize_t control_show(struct device *dev,
{
const char *state = smt_states[cpu_smt_control];
+#ifdef CONFIG_HOTPLUG_SMT
+ /*
+ * If SMT is enabled but not all threads are enabled then show the
+ * number of threads. If all threads are enabled show "on". Otherwise
+ * show the state name.
+ */
+ if (cpu_smt_control == CPU_SMT_ENABLED &&
+ cpu_smt_num_threads != cpu_smt_max_threads)
+ return sysfs_emit(buf, "%d\n", cpu_smt_num_threads);
+#endif
+
return snprintf(buf, PAGE_SIZE - 2, "%s\n", state);
}
--
2.41.0
From: Michael Ellerman <[email protected]>
As part of the generic HOTPLUG_SMT code, there is support for disabling
secondary SMT threads at boot time, by passing "nosmt" on the kernel
command line.
The way that is implemented is the secondary threads are brought partly
online, and then taken back offline again. That is done to support x86
CPUs needing certain initialisation done on all threads. However powerpc
has similar needs, see commit d70a54e2d085 ("powerpc/powernv: Ignore
smt-enabled on Power8 and later").
For that to work the powerpc CPU hotplug callbacks need to be registered
before secondary CPUs are brought online, otherwise __cpu_disable()
fails due to smp_ops->cpu_disable being NULL.
So split the basic initialisation into pseries_cpu_hotplug_init() which
can be called early from setup_arch(). The DLPAR related initialisation
can still be done later, because it needs to do allocations.
Signed-off-by: Michael Ellerman <[email protected]>
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 22 ++++++++++++--------
arch/powerpc/platforms/pseries/pseries.h | 2 ++
arch/powerpc/platforms/pseries/setup.c | 2 ++
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 1a3cb313976a..61fb7cb00880 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -845,15 +845,9 @@ static struct notifier_block pseries_smp_nb = {
.notifier_call = pseries_smp_notifier,
};
-static int __init pseries_cpu_hotplug_init(void)
+void __init pseries_cpu_hotplug_init(void)
{
int qcss_tok;
- unsigned int node;
-
-#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
- ppc_md.cpu_probe = dlpar_cpu_probe;
- ppc_md.cpu_release = dlpar_cpu_release;
-#endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
rtas_stop_self_token = rtas_function_token(RTAS_FN_STOP_SELF);
qcss_tok = rtas_function_token(RTAS_FN_QUERY_CPU_STOPPED_STATE);
@@ -862,12 +856,22 @@ static int __init pseries_cpu_hotplug_init(void)
qcss_tok == RTAS_UNKNOWN_SERVICE) {
printk(KERN_INFO "CPU Hotplug not supported by firmware "
"- disabling.\n");
- return 0;
+ return;
}
smp_ops->cpu_offline_self = pseries_cpu_offline_self;
smp_ops->cpu_disable = pseries_cpu_disable;
smp_ops->cpu_die = pseries_cpu_die;
+}
+
+static int __init pseries_dlpar_init(void)
+{
+ unsigned int node;
+
+#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
+ ppc_md.cpu_probe = dlpar_cpu_probe;
+ ppc_md.cpu_release = dlpar_cpu_release;
+#endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
/* Processors can be added/removed only on LPAR */
if (firmware_has_feature(FW_FEATURE_LPAR)) {
@@ -886,4 +890,4 @@ static int __init pseries_cpu_hotplug_init(void)
return 0;
}
-machine_arch_initcall(pseries, pseries_cpu_hotplug_init);
+machine_arch_initcall(pseries, pseries_dlpar_init);
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index f8bce40ebd0c..f8893ba46e83 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -75,11 +75,13 @@ static inline int dlpar_hp_pmem(struct pseries_hp_errorlog *hp_elog)
#ifdef CONFIG_HOTPLUG_CPU
int dlpar_cpu(struct pseries_hp_errorlog *hp_elog);
+void pseries_cpu_hotplug_init(void);
#else
static inline int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
{
return -EOPNOTSUPP;
}
+static inline void pseries_cpu_hotplug_init(void) { }
#endif
/* PCI root bridge prepare function override for pseries */
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index e2a57cfa6c83..41451b76c6e5 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -816,6 +816,8 @@ static void __init pSeries_setup_arch(void)
/* Discover PIC type and setup ppc_md accordingly */
smp_init_pseries();
+ // Setup CPU hotplug callbacks
+ pseries_cpu_hotplug_init();
if (radix_enabled() && !mmu_has_feature(MMU_FTR_GTSE))
if (!firmware_has_feature(FW_FEATURE_RPT_INVALIDATE))
--
2.41.0
From: Michael Ellerman <[email protected]>
Move the simple exit cases, ie. which don't depend on the value written,
earlier in the function. That makes it clearer that regardless of the
input those states can not be transitioned out of.
That does have a user-visible effect, in that the error returned will
now always be EPERM/ENODEV for those states, regardless of the value
written. Previously writing an invalid value would return EINVAL even
when in those states.
Signed-off-by: Michael Ellerman <[email protected]>
---
kernel/cpu.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 237394e0574a..c67049bb3fc8 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2482,6 +2482,12 @@ __store_smt_control(struct device *dev, struct device_attribute *attr,
{
int ctrlval, ret;
+ if (cpu_smt_control == CPU_SMT_FORCE_DISABLED)
+ return -EPERM;
+
+ if (cpu_smt_control == CPU_SMT_NOT_SUPPORTED)
+ return -ENODEV;
+
if (sysfs_streq(buf, "on"))
ctrlval = CPU_SMT_ENABLED;
else if (sysfs_streq(buf, "off"))
@@ -2491,12 +2497,6 @@ __store_smt_control(struct device *dev, struct device_attribute *attr,
else
return -EINVAL;
- if (cpu_smt_control == CPU_SMT_FORCE_DISABLED)
- return -EPERM;
-
- if (cpu_smt_control == CPU_SMT_NOT_SUPPORTED)
- return -ENODEV;
-
ret = lock_device_hotplug_sysfs();
if (ret)
return ret;
--
2.41.0
From: Michael Ellerman <[email protected]>
Some architectures allows partial SMT states at boot time, ie. when
not all SMT threads are brought online.
To support that the SMT code needs to know the maximum number of SMT
threads, and also the currently configured number.
The architecture code knows the max number of threads, so have the
architecture code pass that value to cpu_smt_set_num_threads(). Note that
although topology_max_smt_threads() exists, it is not configured early
enough to be used here. As architecture, like PowerPC, allows the threads
number to be set through the kernel command line, also pass that value.
Signed-off-by: Michael Ellerman <[email protected]>
[ldufour: slightly reword the commit message]
[ldufour: rename cpu_smt_check_topology and add a num_threads argument]
Signed-off-by: Laurent Dufour <[email protected]>
---
arch/x86/kernel/cpu/bugs.c | 3 ++-
include/linux/cpu_smt.h | 8 ++++++--
kernel/cpu.c | 21 ++++++++++++++++++++-
3 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 182af64387d0..ed71ad385ea7 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -34,6 +34,7 @@
#include <asm/hypervisor.h>
#include <asm/tlbflush.h>
#include <asm/cpu.h>
+#include <asm/smp.h>
#include "cpu.h"
@@ -133,7 +134,7 @@ void __init check_bugs(void)
* identify_boot_cpu() initialized SMT support information, let the
* core code know.
*/
- cpu_smt_check_topology();
+ cpu_smt_set_num_threads(smp_num_siblings, smp_num_siblings);
if (!IS_ENABLED(CONFIG_SMP)) {
pr_info("CPU: ");
diff --git a/include/linux/cpu_smt.h b/include/linux/cpu_smt.h
index 722c2e306fef..0c1664294b57 100644
--- a/include/linux/cpu_smt.h
+++ b/include/linux/cpu_smt.h
@@ -12,15 +12,19 @@ enum cpuhp_smt_control {
#if defined(CONFIG_SMP) && defined(CONFIG_HOTPLUG_SMT)
extern enum cpuhp_smt_control cpu_smt_control;
+extern unsigned int cpu_smt_num_threads;
extern void cpu_smt_disable(bool force);
-extern void cpu_smt_check_topology(void);
+extern void cpu_smt_set_num_threads(unsigned int num_threads,
+ unsigned int max_threads);
extern bool cpu_smt_possible(void);
extern int cpuhp_smt_enable(void);
extern int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval);
#else
# define cpu_smt_control (CPU_SMT_NOT_IMPLEMENTED)
+# define cpu_smt_num_threads 1
static inline void cpu_smt_disable(bool force) { }
-static inline void cpu_smt_check_topology(void) { }
+static inline void cpu_smt_set_num_threads(unsigned int num_threads,
+ unsigned int max_threads) { }
static inline bool cpu_smt_possible(void) { return false; }
static inline int cpuhp_smt_enable(void) { return 0; }
static inline int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval) { return 0; }
diff --git a/kernel/cpu.c b/kernel/cpu.c
index c67049bb3fc8..edca8b7bd400 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -415,6 +415,8 @@ void __weak arch_smt_update(void) { }
#ifdef CONFIG_HOTPLUG_SMT
enum cpuhp_smt_control cpu_smt_control __read_mostly = CPU_SMT_ENABLED;
+static unsigned int cpu_smt_max_threads __ro_after_init;
+unsigned int cpu_smt_num_threads __read_mostly = UINT_MAX;
void __init cpu_smt_disable(bool force)
{
@@ -428,16 +430,33 @@ void __init cpu_smt_disable(bool force)
pr_info("SMT: disabled\n");
cpu_smt_control = CPU_SMT_DISABLED;
}
+ cpu_smt_num_threads = 1;
}
/*
* The decision whether SMT is supported can only be done after the full
* CPU identification. Called from architecture code.
*/
-void __init cpu_smt_check_topology(void)
+void __init cpu_smt_set_num_threads(unsigned int num_threads,
+ unsigned int max_threads)
{
+ WARN_ON(!num_threads || (num_threads > max_threads));
+
if (!topology_smt_supported())
cpu_smt_control = CPU_SMT_NOT_SUPPORTED;
+
+ cpu_smt_max_threads = max_threads;
+
+ /*
+ * If SMT has been disabled via the kernel command line or SMT is
+ * not supported, set cpu_smt_num_threads to 1 for consistency.
+ * If enabled, take the architecture requested number of threads
+ * to bring up into account.
+ */
+ if (cpu_smt_control != CPU_SMT_ENABLED)
+ cpu_smt_num_threads = 1;
+ else if (num_threads < cpu_smt_num_threads)
+ cpu_smt_num_threads = num_threads;
}
static int __init smt_cmdline_disable(char *str)
--
2.41.0
From: Michael Ellerman <[email protected]>
Integrate with the generic SMT support, so that when a CPU is DLPAR
onlined it is brought up with the correct SMT mode.
Signed-off-by: Michael Ellerman <[email protected]>
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 61fb7cb00880..e62835a12d73 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -398,6 +398,14 @@ static int dlpar_online_cpu(struct device_node *dn)
for_each_present_cpu(cpu) {
if (get_hard_smp_processor_id(cpu) != thread)
continue;
+
+ if (!topology_is_primary_thread(cpu)) {
+ if (cpu_smt_control != CPU_SMT_ENABLED)
+ break;
+ if (!topology_smt_thread_allowed(cpu))
+ break;
+ }
+
cpu_maps_update_done();
find_and_update_cpu_nid(cpu);
rc = device_online(get_cpu_device(cpu));
--
2.41.0
> On 29-Jun-2023, at 8:01 PM, Laurent Dufour <[email protected]> wrote:
>
> I'm taking over the series Michael sent previously [1] which is smartly
> reviewing the initial series I sent [2]. This series is addressing the
> comments sent by Thomas and me on the Michael's one.
>
> Here is a short introduction to the issue this series is addressing:
>
> When a new CPU is added, the kernel is activating all its threads. This
> leads to weird, but functional, result when adding CPU on a SMT 4 system
> for instance.
>
> Here the newly added CPU 1 has 8 threads while the other one has 4 threads
> active (system has been booted with the 'smt-enabled=4' kernel option):
>
> ltcden3-lp12:~ # ppc64_cpu --info
> Core 0: 0* 1* 2* 3* 4 5 6 7
> Core 1: 8* 9* 10* 11* 12* 13* 14* 15*
>
> This mixed SMT level may confused end users and/or some applications.
>
> There is no SMT level recorded in the kernel (common code), neither in user
> space, as far as I know. Such a level is helpful when adding new CPU or
> when optimizing the energy efficiency (when reactivating CPUs).
>
> When SMP and HOTPLUG_SMT are defined, this series is adding a new SMT level
> (cpu_smt_num_threads) and few callbacks allowing the architecture code to
> fine control this value, setting a max and a "at boot" level, and
> controling whether a thread should be onlined or not.
>
> v3:
> Fix a build error in the patch 6/9
Successfully tested the V3 version on a Power10 LPAR. Add/remove of
processor core worked correctly, preserving the SMT level (on a kernel
booted with smt-enabled= parameter)
Laurent (Thanks!) also provided a patch to update the ppc64_cpu &
lparstat utility. With patched ppc64_cpu utility verified that SMT level
changed at runtime was preserved across processor core add (on
a kernel booted without smt-enabled= parameter)
Based on these test results
Tested-by: Sachin Sant <[email protected]>
- Sachin
Le 30/06/2023 à 15:32, Sachin Sant a écrit :
>
>
>> On 29-Jun-2023, at 8:01 PM, Laurent Dufour <[email protected]> wrote:
>>
>> I'm taking over the series Michael sent previously [1] which is smartly
>> reviewing the initial series I sent [2]. This series is addressing the
>> comments sent by Thomas and me on the Michael's one.
>>
>> Here is a short introduction to the issue this series is addressing:
>>
>> When a new CPU is added, the kernel is activating all its threads. This
>> leads to weird, but functional, result when adding CPU on a SMT 4 system
>> for instance.
>>
>> Here the newly added CPU 1 has 8 threads while the other one has 4 threads
>> active (system has been booted with the 'smt-enabled=4' kernel option):
>>
>> ltcden3-lp12:~ # ppc64_cpu --info
>> Core 0: 0* 1* 2* 3* 4 5 6 7
>> Core 1: 8* 9* 10* 11* 12* 13* 14* 15*
>>
>> This mixed SMT level may confused end users and/or some applications.
>>
>> There is no SMT level recorded in the kernel (common code), neither in user
>> space, as far as I know. Such a level is helpful when adding new CPU or
>> when optimizing the energy efficiency (when reactivating CPUs).
>>
>> When SMP and HOTPLUG_SMT are defined, this series is adding a new SMT level
>> (cpu_smt_num_threads) and few callbacks allowing the architecture code to
>> fine control this value, setting a max and a "at boot" level, and
>> controling whether a thread should be onlined or not.
>>
>> v3:
>> Fix a build error in the patch 6/9
>
> Successfully tested the V3 version on a Power10 LPAR. Add/remove of
> processor core worked correctly, preserving the SMT level (on a kernel
> booted with smt-enabled= parameter)
>
> Laurent (Thanks!) also provided a patch to update the ppc64_cpu &
> lparstat utility. With patched ppc64_cpu utility verified that SMT level
> changed at runtime was preserved across processor core add (on
> a kernel booted without smt-enabled= parameter)
>
> Based on these test results
>
> Tested-by: Sachin Sant <[email protected]>
Thanks a lot, Sachin!
Once this series is accepted, I'll send the series to update ppc64_cpu.
On Thu, 2023-06-29 at 16:31 +0200, Laurent Dufour wrote:
> From: Michael Ellerman <[email protected]>
>
> Some architectures allows partial SMT states at boot time,
s/allows/allow.
thanks,
rui
Hi, Laurent,
I want to test this patch set and found that it does not apply on top
of latest usptream git, because of some changes in this merge window,
so better rebase.
thanks,
rui
On Thu, 2023-06-29 at 16:31 +0200, Laurent Dufour wrote:
> I'm taking over the series Michael sent previously [1] which is
> smartly
> reviewing the initial series I sent [2]. This series is addressing
> the
> comments sent by Thomas and me on the Michael's one.
>
> Here is a short introduction to the issue this series is addressing:
>
> When a new CPU is added, the kernel is activating all its threads.
> This
> leads to weird, but functional, result when adding CPU on a SMT 4
> system
> for instance.
>
> Here the newly added CPU 1 has 8 threads while the other one has 4
> threads
> active (system has been booted with the 'smt-enabled=4' kernel
> option):
>
> ltcden3-lp12:~ # ppc64_cpu --info
> Core 0: 0* 1* 2* 3* 4 5 6 7
> Core 1: 8* 9* 10* 11* 12* 13* 14* 15*
>
> This mixed SMT level may confused end users and/or some applications.
>
> There is no SMT level recorded in the kernel (common code), neither
> in user
> space, as far as I know. Such a level is helpful when adding new CPU
> or
> when optimizing the energy efficiency (when reactivating CPUs).
>
> When SMP and HOTPLUG_SMT are defined, this series is adding a new SMT
> level
> (cpu_smt_num_threads) and few callbacks allowing the architecture
> code to
> fine control this value, setting a max and a "at boot" level, and
> controling whether a thread should be onlined or not.
>
> v3:
> Fix a build error in the patch 6/9
> v2:
> As Thomas suggested,
> Reword some commit's description
> Remove topology_smt_supported()
> Remove topology_smt_threads_supported()
> Introduce CONFIG_SMT_NUM_THREADS_DYNAMIC
> Remove switch() in __store_smt_control()
> Update kernel-parameters.txt
>
> [1]
> https://lore.kernel.org/linuxppc-dev/[email protected]/
> [2]
> https://lore.kernel.org/linuxppc-dev/[email protected]/
>
> Laurent Dufour (1):
> cpu/SMT: Remove topology_smt_supported()
>
> Michael Ellerman (8):
> cpu/SMT: Move SMT prototypes into cpu_smt.h
> cpu/SMT: Move smt/control simple exit cases earlier
> cpu/SMT: Store the current/max number of threads
> cpu/SMT: Create topology_smt_thread_allowed()
> cpu/SMT: Allow enabling partial SMT states via sysfs
> powerpc/pseries: Initialise CPU hotplug callbacks earlier
> powerpc: Add HOTPLUG_SMT support
> powerpc/pseries: Honour current SMT state when DLPAR onlining CPUs
>
> .../ABI/testing/sysfs-devices-system-cpu | 1 +
> .../admin-guide/kernel-parameters.txt | 4 +-
> arch/Kconfig | 3 +
> arch/powerpc/Kconfig | 2 +
> arch/powerpc/include/asm/topology.h | 15 +++
> arch/powerpc/kernel/smp.c | 8 +-
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 30 +++--
> arch/powerpc/platforms/pseries/pseries.h | 2 +
> arch/powerpc/platforms/pseries/setup.c | 2 +
> arch/x86/include/asm/topology.h | 4 +-
> arch/x86/kernel/cpu/bugs.c | 3 +-
> arch/x86/kernel/smpboot.c | 8 --
> include/linux/cpu.h | 25 +---
> include/linux/cpu_smt.h | 33 +++++
> kernel/cpu.c | 118 ++++++++++++++--
> --
> 15 files changed, 187 insertions(+), 71 deletions(-)
> create mode 100644 include/linux/cpu_smt.h
>
On Thu, 2023-06-29 at 16:31 +0200, Laurent Dufour wrote:
> @@ -2580,6 +2597,17 @@ static ssize_t control_show(struct device
> *dev,
> {
> const char *state = smt_states[cpu_smt_control];
>
> +#ifdef CONFIG_HOTPLUG_SMT
> + /*
> + * If SMT is enabled but not all threads are enabled then
> show the
> + * number of threads. If all threads are enabled show "on".
> Otherwise
> + * show the state name.
> + */
> + if (cpu_smt_control == CPU_SMT_ENABLED &&
> + cpu_smt_num_threads != cpu_smt_max_threads)
> + return sysfs_emit(buf, "%d\n", cpu_smt_num_threads);
> +#endif
> +
My understanding is that cpu_smt_control is always set to
CPU_SMT_NOT_IMPLEMENTED when CONFIG_HOTPLUG_SMT is not set, so this
ifdef is not necessary, right?
thanks,
rui
Le 05/07/2023 à 05:14, Zhang, Rui a écrit :
> On Thu, 2023-06-29 at 16:31 +0200, Laurent Dufour wrote:
>> @@ -2580,6 +2597,17 @@ static ssize_t control_show(struct device
>> *dev,
>> {
>> const char *state = smt_states[cpu_smt_control];
>>
>> +#ifdef CONFIG_HOTPLUG_SMT
>> + /*
>> + * If SMT is enabled but not all threads are enabled then
>> show the
>> + * number of threads. If all threads are enabled show "on".
>> Otherwise
>> + * show the state name.
>> + */
>> + if (cpu_smt_control == CPU_SMT_ENABLED &&
>> + cpu_smt_num_threads != cpu_smt_max_threads)
>> + return sysfs_emit(buf, "%d\n", cpu_smt_num_threads);
>> +#endif
>> +
>
> My understanding is that cpu_smt_control is always set to
> CPU_SMT_NOT_IMPLEMENTED when CONFIG_HOTPLUG_SMT is not set, so this
> ifdef is not necessary, right?
Hi Rui,
Indeed, cpu_smt_control, cpu_smt_num_threads and cpu_smt_max_threads are
only defined when CONFIG_HOTPLUG_SMT is set. This is the reason for this
#ifdef block.
This has been reported by the kernel test robot testing v2:
https://lore.kernel.org/oe-kbuild-all/[email protected]
Cheers,
Laurent.
Le 05/07/2023 à 05:04, Zhang, Rui a écrit :
> Hi, Laurent,
>
> I want to test this patch set and found that it does not apply on top
> of latest usptream git, because of some changes in this merge window,
> so better rebase.
Hi Rui,
Thanks for your interest for this series.
The latest Thomas's changes came into the PowerPC next branch.
I'm working on a rebase.
Cheers,
Laurent.
> thanks,
> rui
>
> On Thu, 2023-06-29 at 16:31 +0200, Laurent Dufour wrote:
>> I'm taking over the series Michael sent previously [1] which is
>> smartly
>> reviewing the initial series I sent [2]. This series is addressing
>> the
>> comments sent by Thomas and me on the Michael's one.
>>
>> Here is a short introduction to the issue this series is addressing:
>>
>> When a new CPU is added, the kernel is activating all its threads.
>> This
>> leads to weird, but functional, result when adding CPU on a SMT 4
>> system
>> for instance.
>>
>> Here the newly added CPU 1 has 8 threads while the other one has 4
>> threads
>> active (system has been booted with the 'smt-enabled=4' kernel
>> option):
>>
>> ltcden3-lp12:~ # ppc64_cpu --info
>> Core 0: 0* 1* 2* 3* 4 5 6 7
>> Core 1: 8* 9* 10* 11* 12* 13* 14* 15*
>>
>> This mixed SMT level may confused end users and/or some applications.
>>
>> There is no SMT level recorded in the kernel (common code), neither
>> in user
>> space, as far as I know. Such a level is helpful when adding new CPU
>> or
>> when optimizing the energy efficiency (when reactivating CPUs).
>>
>> When SMP and HOTPLUG_SMT are defined, this series is adding a new SMT
>> level
>> (cpu_smt_num_threads) and few callbacks allowing the architecture
>> code to
>> fine control this value, setting a max and a "at boot" level, and
>> controling whether a thread should be onlined or not.
>>
>> v3:
>> Fix a build error in the patch 6/9
>> v2:
>> As Thomas suggested,
>> Reword some commit's description
>> Remove topology_smt_supported()
>> Remove topology_smt_threads_supported()
>> Introduce CONFIG_SMT_NUM_THREADS_DYNAMIC
>> Remove switch() in __store_smt_control()
>> Update kernel-parameters.txt
>>
>> [1]
>> https://lore.kernel.org/linuxppc-dev/[email protected]/
>> [2]
>> https://lore.kernel.org/linuxppc-dev/[email protected]/
>>
>> Laurent Dufour (1):
>> cpu/SMT: Remove topology_smt_supported()
>>
>> Michael Ellerman (8):
>> cpu/SMT: Move SMT prototypes into cpu_smt.h
>> cpu/SMT: Move smt/control simple exit cases earlier
>> cpu/SMT: Store the current/max number of threads
>> cpu/SMT: Create topology_smt_thread_allowed()
>> cpu/SMT: Allow enabling partial SMT states via sysfs
>> powerpc/pseries: Initialise CPU hotplug callbacks earlier
>> powerpc: Add HOTPLUG_SMT support
>> powerpc/pseries: Honour current SMT state when DLPAR onlining CPUs
>>
>> .../ABI/testing/sysfs-devices-system-cpu | 1 +
>> .../admin-guide/kernel-parameters.txt | 4 +-
>> arch/Kconfig | 3 +
>> arch/powerpc/Kconfig | 2 +
>> arch/powerpc/include/asm/topology.h | 15 +++
>> arch/powerpc/kernel/smp.c | 8 +-
>> arch/powerpc/platforms/pseries/hotplug-cpu.c | 30 +++--
>> arch/powerpc/platforms/pseries/pseries.h | 2 +
>> arch/powerpc/platforms/pseries/setup.c | 2 +
>> arch/x86/include/asm/topology.h | 4 +-
>> arch/x86/kernel/cpu/bugs.c | 3 +-
>> arch/x86/kernel/smpboot.c | 8 --
>> include/linux/cpu.h | 25 +---
>> include/linux/cpu_smt.h | 33 +++++
>> kernel/cpu.c | 118 ++++++++++++++--
>> --
>> 15 files changed, 187 insertions(+), 71 deletions(-)
>> create mode 100644 include/linux/cpu_smt.h
>>
>
Le 05/07/2023 à 05:05, Zhang, Rui a écrit :
> On Thu, 2023-06-29 at 16:31 +0200, Laurent Dufour wrote:
>> From: Michael Ellerman <[email protected]>
>>
>> Some architectures allows partial SMT states at boot time,
>
> s/allows/allow.
Thanks Rui !