On Wed, Mar 15, 2017 at 10:18 PM, Michael Ellerman <m...@ellerman.id.au> wrote:
> Oliver O'Halloran <ooh...@gmail.com> writes:
>
>> To determine which logical CPUs are on the same core the kernel uses the
>> ibm,chipid property from the device tree node associated with that cpu.
>> The lookup for this this information is currently open coded in both
>> traverse_siblings() and traverse_siblings_chip_id(). This patch replaces
>> these manual lookups with the existing cpu_to_chip_id() function.
>
> Some minor nits.
>
> cpu_to_chip_id() actually searches recursively up the parents until it
> finds a ibm,chip-id, so it's not a 1:1 replacement for the existing
> logic, but it's probably still an OK conversion. It's still worth
> mentioning in the change log thought.
fair enough
>> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
>> index 893bd7f79be6..dfe0e1d9cd06 100644
>> --- a/arch/powerpc/kernel/smp.c
>> +++ b/arch/powerpc/kernel/smp.c
>> @@ -664,23 +655,19 @@ static void traverse_core_siblings(int cpu, bool add)
>> {
>> struct device_node *l2_cache, *np;
>> const struct cpumask *mask;
>> - int i, chip, plen;
>> - const __be32 *prop;
>> + int chip_id;
>> + int i;
>>
>> - /* First see if we have ibm,chip-id properties in cpu nodes */
>> - np = of_get_cpu_node(cpu, NULL);
>> - if (np) {
>> - chip = -1;
>> - prop = of_get_property(np, "ibm,chip-id", &plen);
>> - if (prop && plen == sizeof(int))
>> - chip = of_read_number(prop, 1);
>> - of_node_put(np);
>> - if (chip >= 0) {
>> - traverse_siblings_chip_id(cpu, add, chip);
>> - return;
>> - }
>> + /* threads that share a chip-id are considered siblings (same die) */
>
> You might know it means the "same die", but AFAIK there's no actual
> definition for what the chip-id means, so let's not write comments that
> might be wrong in future. Just saying they're considered siblings is
> sufficient.
>
> Also "Threads" :)
The cpus masks are all built in terms of threads, so this is
technically correct even if it sounds stupid. Maybe "logical cpus"
would be better?
>
> cheers
