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

From: <Conor.Dooley@microchip.com>
To: <sudeep.holla@arm.com>
Cc: <paul.walmsley@sifive.com>, <palmer@dabbelt.com>,
	<palmer@rivosinc.com>, <aou@eecs.berkeley.edu>,
	<Daire.McNamara@microchip.com>, <niklas.cassel@wdc.com>,
	<damien.lemoal@opensource.wdc.com>, <geert@linux-m68k.org>,
	<zong.li@sifive.com>, <kernel@esmil.dk>, <hahnjo@hahnjo.de>,
	<guoren@kernel.org>, <anup@brainfault.org>,
	<atishp@atishpatra.org>, <changbin.du@intel.com>,
	<heiko@sntech.de>, <philipp.tomsich@vrull.eu>,
	<linux-riscv@lists.infradead.org>, <mail@conchuod.ie>,
	<linux-kernel@vger.kernel.org>, <Brice.Goglin@inria.fr>
Subject: Re: [PATCH] riscv: arch-topology: fix default topology reporting
Date: Thu, 7 Jul 2022 10:15:27 +0000	[thread overview]
Message-ID: <cf4e5296-03b1-f8d4-9742-4d176dcbef25@microchip.com> (raw)
In-Reply-To: <20220707095037.i2pjcc7zcf4pi3ht@bogus>

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@microchip.com>
>>
>> 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@vger.kernel.org
>> Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
>> Reported-by: Brice Goglin <Brice.Goglin@inria.fr>
>> Link: https://github.com/open-mpi/hwloc/issues/536
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>
>> 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.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv