Message ID | 20230615154635.13660-8-ldufour@linux.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Introduce SMT level and add PowerPC support | expand |
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 <tglx@linutronix.de> 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
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); }