Re: [PATCH] x86/AMD: Fix LLC ID for AMD Fam17h systems

From: Yazen Ghannam
Date: Thu Oct 27 2016 - 10:14:47 EST


>> + /*
>> + * LLC is at the Core Complex level.
>> + * Core Complex Id is ApicId[3].
>> + */
>> + else if (c->x86 == 0x17)
>> + per_cpu(cpu_llc_id, cpu) = c->initial_apicid >> 3;
>
> This whole if/else block lacks curly braces. See:
>
> https://marc.info/?l=linux-kernel&m=147351236615103
>

Okay, understood.

>> - /*
>> - * Fix percpu cpu_llc_id here as LLC topology is different
>> - * for Fam17h systems.
>> - */
>> - if (c->x86 != 0x17 || !cpuid_edx(0x80000006))
>> - return;
>> -
>> - socket_id = (c->apicid >> bits) - 1;
>> - core_complex_id = (c->apicid & ((1 << bits) - 1)) >> 3;
>> -
>> - per_cpu(cpu_llc_id, cpu) = (socket_id << 3) | core_complex_id;
>
> So if I've read the patch correctly then the trivial fix for fam17h would
> have been:
>
>> + per_cpu(cpu_llc_id, cpu) = c->initial_apicid >> 3;
>
> Right?
>

Right.

> And this one liner wants to be a seperate patch with a proper
> explanation. And that simple hunk can be tagged for stable.
>

Okay, I'll make it a separate patch. It still wouldn't be a one-liner
because then socket_id and core_complex_id would be left unused and
should be removed.

> The rest of the patch is cleanup and improvement and want's to be seperated
> out and explained proper.
>

Okay, will do.

Thanks,
Yazen