Re: AMD Bulldozer topology regression since 4.6

From: Brice Goglin
Date: Tue Jan 03 2017 - 05:52:21 EST




Le 29/11/2016 22:02, Brice Goglin a Ãcrit :
> Le 29/11/2016 20:39, Borislav Petkov a Ãcrit :
>> Does that fix it?
>>
>> Patch is against latest tip/master because we have some more changes in
>> that area.
> I tested the second patch on top of 4.8.11, it brings core_id back to
> where it was before 4.6, thanks.
>
> Reported-and-tested-by: Brice Goglin <Brice.Goglin@xxxxxxxx>

Hello Borislav
Are you pushing this fix to Linus soon?
We received several bug reports on our side about dual-core compute
units being exposed as a dual-threaded single-cores in sysfs.
Thanks
Brice




> However thread_siblings isn't back to where it was in 4.5. Now we have a
> single bit in each thread_siblings mask. That's correct with respect to
> the sysfs topology documentation. In 4.5, there were two bits (one for
> each core of the compute unit), which was wrong (cores with different
> core_ids shouldn't appear in each other thread_siblings). I assumed that
> these processors had to break the sysfs topology documentation to expose
> the concept of "dual-core compute-unit" which somehow sits between
> hyperthreading and dual-core.
>
> I personally do not care much about this regression, not sure about
> other user-space tools?
>
> Another minor related change: /proc/cpuinfo shows "cpu cores : 16"
> instead of "8".
>
> Brice
>
>
>> ---
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index 4daad1e39352..a070d7b0a133 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -305,15 +305,7 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
>>
>> /* get information required for multi-node processors */
>> if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
>> - u32 eax, ebx, ecx, edx;
>> -
>> - cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
>> - node_id = ecx & 7;
>> -
>> - /* get compute unit information */
>> - smp_num_siblings = ((ebx >> 8) & 3) + 1;
>> - c->x86_max_cores /= smp_num_siblings;
>> - c->cpu_core_id = ebx & 0xff;
>> + node_id = cpuid_ecx(0x8000001e) & 7;
>>
>> /*
>> * We may have multiple LLCs if L3 caches exist, so check if we
>> ---
>>
>> Just in case, I have one against Linus' master too:
>>
>> ---
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index 1e81a37c034e..19f50a9d6b42 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -305,15 +305,7 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
>>
>> /* get information required for multi-node processors */
>> if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
>> - u32 eax, ebx, ecx, edx;
>> -
>> - cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
>> - node_id = ecx & 7;
>> -
>> - /* get compute unit information */
>> - smp_num_siblings = ((ebx >> 8) & 3) + 1;
>> - c->x86_max_cores /= smp_num_siblings;
>> - c->cpu_core_id = ebx & 0xff;
>> + node_id = cpuid_ecx(0x8000001e) & 7;
>> } else if (cpu_has(c, X86_FEATURE_NODEID_MSR)) {
>> u64 value;
>> ---
>>