Message ID | 20230524155630.794584-6-mpe@ellerman.id.au (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [1/9] cpu/SMT: Move SMT prototypes into cpu_smt.h | expand |
On Thu, May 25 2023 at 01:56, Michael Ellerman wrote: > There is a hook which allows arch code to control how many threads per Can you please write out architecture in changelogs and comments? I know 'arch' is commonly used but while my brain parser tolerates 'arch_' prefixes it raises an exception on 'arch' in prose as 'arch' is a regular word with a completely different meaning. Changelogs and comments are not space constraint. > @@ -2505,20 +2505,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(); This switch case does not make sense anymore. The only situation which reaches this is when the control value goes from CPU_SMT_DISABLED to CPU_SMT_FORCE_DISABLED because that's not changing the number of threads. So something like this is completely sufficient: if (num_threads > orig_threads) ret = cpuhp_smt_enable(); else if (num_threads < orig_threads || ctrval == CPU_SMT_FORCE_DISABLED) ret = cpuhp_smt_disable(ctrlval); No? Thanks, tglx
On Sat, Jun 10 2023 at 22:09, Thomas Gleixner wrote: > On Thu, May 25 2023 at 01:56, Michael Ellerman wrote: >> There is a hook which allows arch code to control how many threads per > > Can you please write out architecture in changelogs and comments? > > I know 'arch' is commonly used but while my brain parser tolerates > 'arch_' prefixes it raises an exception on 'arch' in prose as 'arch' is > a regular word with a completely different meaning. Changelogs and > comments are not space constraint. > >> @@ -2505,20 +2505,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)) Why does this not simply check cpu_smt_max_threads? else if (num_threads > 1 && num_threads <= cpu_smt_max_threads) cpu_smt_max_threads should have been established already, no? Thanks, tglx
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 72b9a03a4fef..aca23c7b4547 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -2497,7 +2497,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; @@ -2505,20 +2505,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(); @@ -2556,6 +2574,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); }
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 <mpe@ellerman.id.au> --- .../ABI/testing/sysfs-devices-system-cpu | 1 + kernel/cpu.c | 39 ++++++++++++++++--- 2 files changed, 34 insertions(+), 6 deletions(-)