Re: [PATCH v2 1/2] arm64: topology: move store_cpu_topology() to shared code

[<Conor.Dooley@microchip.com>]

+CC Russel, Arnd

On 08/07/2022 21:45, Conor Dooley - M52691 wrote:
> On 08/07/2022 21:33, Conor Dooley wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> arm64's method of defining a default cpu topology requires only minimal
>> changes to apply to RISC-V also. The current arm64 implementation exits
>> early in a uniprocessor configuration by reading MPIDR & claiming that
>> uniprocessor can rely on the default values.
>>
>> This is appears to be a hangover from prior to '3102bc0e6ac7 ("arm64:
>> topology: Stop using MPIDR for topology information")', because the
>> current code just assigns default values for multiprocessor systems.
>>
>> With the MPIDR references removed, store_cpu_topolgy() can be moved to
>> the common arch_topology code.
>>
>> CC: stable@vger.kernel.org
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
---8<---
>>  #ifdef CONFIG_ACPI
>>  static bool __init acpi_cpu_is_threaded(int cpu)
>>  {
>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>> index 441e14ac33a4..07e84c6ac5c2 100644
>> --- a/drivers/base/arch_topology.c
>> +++ b/drivers/base/arch_topology.c
>> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>>  	}
>>  }
>>  
>> +void __weak store_cpu_topology(unsigned int cpuid)
> 
> Ahh crap, I forgot to remove the __weak.
> I won't immediately respin since it is minor. I've pushed it (without
> the __weak) to https://git.kernel.org/conor/h/arch-topo so it'll get
> the lkp coverage.

And build failure for arm32:

> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git arch-topo
> branch HEAD: df379c4b12f6b22fb8c07c2be16fd821a4fcbfc5  riscv: topology: fix default topology reporting
> 
> Error/Warning: (recently discovered and may have been fixed)
> 
> arch_topology.c:(.text+0xbac): multiple definition of `store_cpu_topology'; arch/arm/kernel/topology.o:topology.c:(.text+0x0): first defined here
> 
> Error/Warning ids grouped by kconfigs:
> 
> gcc_recent_errors
> `-- arm-defconfig
>     `-- multiple-definition-of-store_cpu_topology-arch-arm-kernel-topology.o:topology.c:(.text):first-defined-here
> 
> elapsed time: 721m

Looking at the arm32 implementation - it appears to be mostly the sort of MPIDR
stuff that was removed from the arm64 implementation in 3102bc0e6ac7 ("arm64:
topology: Stop using MPIDR for topology information"). Could arm32 benefit from
the same shared implemenation too, or is usage of MPIDR only invalid for arm64?

The other difference is a call to update_cpu_capacity() in the arm32
implementation. Could that be moved to smp_store_cpu_info() which is the only
callsite of store_cpu_topology()?

Either way, will respin a v3 that doesn't break the arm32 build when
CONFIG_GENERIC_ARCH_TOPOLOGY is enabled :)

Thanks,
Conor.