On 2024/10/31 21:33, Thomas Gleixner wrote:
> On Thu, Oct 31 2024 at 20:17, Yicong Yang wrote:
>> On 2024/10/30 22:55, Thomas Gleixner wrote:
>>>> +static inline bool topology_is_primary_thread(unsigned int cpu)
>>>> +{
>>>> + /*
>>>> + * On SMT hotplug the primary thread of the SMT won't be disabled.
>>>> + * Architectures do have a special primary thread (e.g. x86) need
>>>> + * to override this function. Otherwise just make the first thread
>>>> + * in the SMT as the primary thread.
>>>> + */
>>>> + return cpu == cpumask_first(topology_sibling_cpumask(cpu));
>>>
>>> How is that supposed to work? Assume both siblings are offline, then the
>>> sibling mask is empty and you can't boot the CPU anymore.
>>>
>>
>> For architectures' using arch_topology, topology_sibling_cpumask() will at
>> least
>> contain the tested CPU itself. This is initialized in
>> drivers/base/arch_topology.c:reset_cpu_topology(). So it won't be
>> empty here.
>
> Fair enough. Can you please expand the comment and say:
>
> The sibling cpumask of a offline CPU contains always the CPU
> itself.
>
Sure, will make it clear.
>> Besides we don't need to check topology_is_primary_thread() at boot time:
>> -> cpu_up(cpu)
>> cpu_bootable()
>> if (cpu_smt_control == CPU_SMT_ENABLED &&
>> cpu_smt_thread_allowed(cpu)) // will always return true if
>> !CONFIG_SMT_NUM_THREADS_DYNAMIC
>> return true; // we'll always return here and @cpu is always bootable
>
> cpu_smt_control is not guaranteed to have CPU_SMT_ENABLED state, so this
> argument is bogus.
>
sorry for didn't explain all the cases here.
For cpu_sm_control == {CPU_SMT_ENABLED, CPU_SMT_NOT_SUPPORTED,
CPU_SMT_NOT_IMPLEMENTED},
all the cpu's bootable and we won't check topology_is_primary_thread().
static inline bool cpu_bootable(unsigned int cpu)
{
if (cpu_smt_control == CPU_SMT_ENABLED && cpu_smt_thread_allowed(cpu))
return true;
/* All CPUs are bootable if controls are not configured */
if (cpu_smt_control == CPU_SMT_NOT_IMPLEMENTED)
return true;
/* All CPUs are bootable if CPU is not SMT capable */
if (cpu_smt_control == CPU_SMT_NOT_SUPPORTED)
return true;
if (topology_is_primary_thread(cpu)) // Will be true for all the CPUs
when thread sibling's not built
// Only true for primary thread if
thread sibling's updated
// thread sibling will be updated
once the CPU's bootup, for arm64
// in secondary_start_kernel()
return true;
return !cpumask_test_cpu(cpu, &cpus_booted_once_mask); // Also be
updated once the CPU's bootup, in
//
secondary_start_kernel() for arm64
// Will return
false in the second check of
//
cpu_bootable() in the call chain below
}
For cpu_smt_control == {CPUS_SMT_DISABLED, CPU_SMT_FORCE_DISABLED} if user
specified the
boot option "nosmt" or "nosmt=force", it'll be a bit more complex. For a
non-primary
thread CPU, cpu_bootable() will return true and it'll be boot. Then after
thread sibling's
built cpu_bootable() will be checked secondly it the cpuhp callbacks, since
it'll return
false then and we'll roll back and offline it.
// for a non-primary thread CPU, system boot with "nosmt" or "nosmt=force"
-> cpu_up()
cpu_bootable() -> true, since the thread sibling mask only coutains CPU
itself
[...]
cpuhp_bringup_ap()
bringup_wait_for_ap_online()
if (!cpu_bootable(cpu)) // target CPU has been bringup, thread sibling
mask's updated
// then this non-primay thread won't be
bootable in this case
return -ECANCELED // roll back and offline this CPU
Thanks.
