Re: AMD Bulldozer topology regression since 4.6

[Brice Goglin]

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;
>> ---
>>