Re: [PATCH] riscv: arch-topology: fix default topology reporting

[Conor.Dooley]

On 07/07/2022 10:50, Sudeep Holla wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Wed, Jul 06, 2022 at 07:45:59PM +0100, Conor Dooley wrote:
>> From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
>>
>> RISC-V has no sane defaults to fall back on where there is no cpu-map
>> in the devicetree.
>> Without sane defaults, the package, core and thread IDs are all set to
>> -1. This causes user-visible inaccuracies for tools like hwloc/lstopo
>> which rely on the sysfs cpu topology files to detect a system's
>> topology.
>>
>> Add sane defaults in ~the exact same way as ARM64.
>>
>> CC: stable@xxxxxxxxxxxxxxx
>> Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
>> Reported-by: Brice Goglin <Brice.Goglin@xxxxxxxx>
>> Link: https://github.com/open-mpi/hwloc/issues/536
>> Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
>> ---
>>
>> Sudeep suggested that this be backported rather than the changes to
>> the devicetrees adding cpu-map since that property is optional.
>> That patchset is still valid in it's own right.
>>
>>   arch/riscv/include/asm/topology.h | 13 +++++++++++++
>>   arch/riscv/kernel/Makefile        |  1 +
>>   arch/riscv/kernel/smpboot.c       |  4 ++++
>>   arch/riscv/kernel/topology.c      | 32 +++++++++++++++++++++++++++++++
>>   4 files changed, 50 insertions(+)
>>   create mode 100644 arch/riscv/include/asm/topology.h
>>   create mode 100644 arch/riscv/kernel/topology.c
>>
> 
> [...]
> 
>> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
>> index f1e4948a4b52..d551c7f452d4 100644
>> --- a/arch/riscv/kernel/smpboot.c
>> +++ b/arch/riscv/kernel/smpboot.c
>> @@ -32,6 +32,7 @@
>>   #include <asm/sections.h>
>>   #include <asm/sbi.h>
>>   #include <asm/smp.h>
>> +#include <asm/topology.h>
>>
>>   #include "head.h"
>>
> 
> [...]
> 
>>   void __init smp_prepare_cpus(unsigned int max_cpus)
>> @@ -161,6 +164,7 @@ asmlinkage __visible void smp_callin(void)
>>        mmgrab(mm);
>>        current->active_mm = mm;
>>
>> +     store_cpu_topology(curr_cpuid);
>>        notify_cpu_starting(curr_cpuid);
>>        numa_add_cpu(curr_cpuid);
>>        update_siblings_masks(curr_cpuid);
> 
> If the above store_cpu_topology calls update_siblings_masks if required,
> probably you can drop this explicit call here.

Yeah, store_cpu_topology() does call update_siblings_masks().
I'll respin tonight with this (and the lkp error) fixed.

Thanks Sudeep,
Conor.