Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch V4 02/41] cpu/SMT: Make SMT control more robust against enumeration failures | Date | Tue, 10 Oct 2023 14:18:47 +0200 |
| |
On Tue, Aug 15 2023 at 14:15, Dave Hansen wrote: > On 8/14/23 01:53, Thomas Gleixner wrote: >> -static inline bool cpu_smt_allowed(unsigned int cpu) >> +static inline bool cpu_bootable(unsigned int cpu) >> { >> if (cpu_smt_control == CPU_SMT_ENABLED) >> return true; >> >> + if (cpu_smt_control == CPU_SMT_NOT_SUPPORTED) >> + return true; > > I found this new pair of if()'s rather counterintuitive to read. > > The first one reads like: > > "If SMT is not supported, the CPU is always bootable" > > but "supported" could easily mean CONFIG_SMP==n (which is actually > covered in the next case). Would this be better named: > > CPU_SMT_NOT_ENUMERATED > or > CPU_SMT_NOT_DETECTED > > ?
Yes, no, maybe. I rather keep them as is because the strings which are exposed via sysfs cannot be changed and are matching.
> /* Every CPU is bootable on non-SMT systems: */ > if (cpu_smt_control == CPU_SMT_NOT_DETECTED) > return true; > > For the next one: > >> + if (cpu_smt_control == CPU_SMT_NOT_IMPLEMENTED) >> + return true; > > This reads a bit like "SMT is not implemented" rather than "SMT controls > are not implemented". Maybe a comment would help: > > /* All CPUs are bootable if controls are not implemented: */
Sure.
| |