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.
There is a hook which allows arch code to control how many threads per
core are supported. To retain the existing behaviour, the x86 hook only
supports 1 thread or all threads.
Signed-off-by: Michael Ellerman <[email protected]>
---
.../ABI/testing/sysfs-devices-system-cpu | 1 +
kernel/cpu.c | 39 ++++++++++++++++---
2 files changed, 34 insertions(+), 6 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 ae2fa26a5b63..248f0734098a 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2507,7 +2507,7 @@ 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;
if (cpu_smt_control == CPU_SMT_FORCE_DISABLED)
return -EPERM;
@@ -2515,20 +2515,38 @@ __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 (num_threads > 1 && topology_smt_threads_supported(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) {
+ orig_threads = cpu_smt_num_threads;
+ cpu_smt_num_threads = num_threads;
+
+ if (num_threads > orig_threads) {
+ ret = cpuhp_smt_enable();
+ } else if (num_threads < orig_threads) {
+ ret = cpuhp_smt_disable(ctrlval);
+ } else if (ctrlval != cpu_smt_control) {
switch (ctrlval) {
case CPU_SMT_ENABLED:
ret = cpuhp_smt_enable();
@@ -2566,6 +2584,15 @@ static ssize_t control_show(struct device *dev,
{
const char *state = smt_states[cpu_smt_control];
+ /*
+ * 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);
+
return snprintf(buf, PAGE_SIZE - 2, "%s\n", state);
}
--
2.41.0
On Thu, Jun 15 2023 at 17:46, Laurent Dufour wrote:
>
> - if (ctrlval != cpu_smt_control) {
> + orig_threads = cpu_smt_num_threads;
> + cpu_smt_num_threads = num_threads;
> +
> + if (num_threads > orig_threads) {
> + ret = cpuhp_smt_enable();
> + } else if (num_threads < orig_threads) {
> + ret = cpuhp_smt_disable(ctrlval);
> + } else if (ctrlval != cpu_smt_control) {
> switch (ctrlval) {
> case CPU_SMT_ENABLED:
> ret = cpuhp_smt_enable();
This switch() is still as pointless as in the previous version.
OFF -> ON, ON -> OFF, ON -> FORCE_OFF are covered by the num_threads
comparisons.
So the only case where (ctrlval != cpu_smt_control) is relevant is the
OFF -> FORCE_OFF transition because in that case the number of threads
is not changing.
force_off = ctrlval != cpu_smt_control && ctrval == 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);
Should just work, no?
Thanks,
tglx
Thomas Gleixner <[email protected]> writes:
> On Thu, Jun 15 2023 at 17:46, Laurent Dufour wrote:
>>
>> - if (ctrlval != cpu_smt_control) {
>> + orig_threads = cpu_smt_num_threads;
>> + cpu_smt_num_threads = num_threads;
>> +
>> + if (num_threads > orig_threads) {
>> + ret = cpuhp_smt_enable();
>> + } else if (num_threads < orig_threads) {
>> + ret = cpuhp_smt_disable(ctrlval);
>> + } else if (ctrlval != cpu_smt_control) {
>> switch (ctrlval) {
>> case CPU_SMT_ENABLED:
>> ret = cpuhp_smt_enable();
>
> This switch() is still as pointless as in the previous version.
>
> OFF -> ON, ON -> OFF, ON -> FORCE_OFF are covered by the num_threads
> comparisons.
>
> So the only case where (ctrlval != cpu_smt_control) is relevant is the
> OFF -> FORCE_OFF transition because in that case the number of threads
> is not changing.
>
> force_off = ctrlval != cpu_smt_control && ctrval == 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);
>
> Should just work, no?
Yes, I think so.
I'll fold that in and do a respin of this series for 6.6 in the next
week or two.
cheers