>> +                    /*
>> +                     * 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

Reply via email to