Messages in this thread | | | From | <> | Subject | Re: [PATCH] riscv: arch-topology: fix default topology reporting | Date | Thu, 7 Jul 2022 10:12:48 +0000 |
| |
On 07/07/2022 10:47, Sudeep Holla wrote: > On Wed, Jul 06, 2022 at 02:38:01PM -0700, Atish Patra wrote: >> On Wed, Jul 6, 2022 at 11:46 AM Conor Dooley <mail@conchuod.ie> 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/include/asm/topology.h b/arch/riscv/include/asm/topology.h >>> new file mode 100644 >>> index 000000000000..36bc6ecda898 >>> --- /dev/null >>> +++ b/arch/riscv/include/asm/topology.h >>> @@ -0,0 +1,13 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>> +/* >>> + * Copyright (c) 2022 Microchip Technology Inc. and its subsidiaries >>> + */ >>> + >>> +#ifndef _ASM_RISCV_TOPOLOGY_H >>> +#define _ASM_RISCV_TOPOLOGY_H >>> + >>> +#include <asm-generic/topology.h> >>> + >>> +void store_cpu_topology(unsigned int cpuid); >>> + >>> +#endif /* _ASM_RISCV_TOPOLOGY_H */ >>> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile >>> index c71d6591d539..9518882ba6f9 100644 >>> --- a/arch/riscv/kernel/Makefile >>> +++ b/arch/riscv/kernel/Makefile >>> @@ -50,6 +50,7 @@ obj-y += riscv_ksyms.o >>> obj-y += stacktrace.o >>> obj-y += cacheinfo.o >>> obj-y += patch.o >>> +obj-y += topology.o >>> obj-y += probes/ >>> obj-$(CONFIG_MMU) += vdso.o vdso/ >>> >>> 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" >>> >>> @@ -40,6 +41,8 @@ static DECLARE_COMPLETION(cpu_running); >>> void __init smp_prepare_boot_cpu(void) >>> { >>> init_cpu_topology(); >>> + >>> + store_cpu_topology(smp_processor_id()); >>> } >>> >>> 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); >>> diff --git a/arch/riscv/kernel/topology.c b/arch/riscv/kernel/topology.c >>> new file mode 100644 >>> index 000000000000..db72862bd5b5 >>> --- /dev/null >>> +++ b/arch/riscv/kernel/topology.c >>> @@ -0,0 +1,32 @@ >>> +// SPDX-License-Identifier: GPL-2.0-only >>> +/* >>> + * Copyright (c) 2022 Microchip Technology Inc. and its subsidiaries >>> + * >>> + * Based on the arm64 version, which was in turn based on arm32, which was >>> + * ultimately based on sh's. >>> + * The arm64 version was listed as: >>> + * Copyright (C) 2011,2013,2014 Linaro Limited. >>> + */ >>> + >>> +#include <linux/arch_topology.h> >>> +#include <linux/topology.h> >>> +#include <asm/topology.h> >>> + >>> +void store_cpu_topology(unsigned int cpuid) >>> +{ >>> + struct cpu_topology *cpuid_topo = &cpu_topology[cpuid]; >>> + >>> + if (cpuid_topo->package_id != -1) >>> + goto topology_populated; >>> + >>> + cpuid_topo->thread_id = -1; >>> + cpuid_topo->core_id = cpuid; >>> + cpuid_topo->package_id = cpu_to_node(cpuid); >>> + >>> + pr_debug("CPU%u: package %d core %d thread %d\n", >>> + cpuid, cpuid_topo->package_id, cpuid_topo->core_id, >>> + cpuid_topo->thread_id); >>> + >>> +topology_populated: >>> + update_siblings_masks(cpuid); >>> +} >>> >> >> This function is pretty much the same as the arm64 one except the >> UP/mpidr check. >> Can we move this to the common code as well ? >> > > While I completely agree with the idea, not sure if that makes backports > (if required) any difficult. If so, I would rather keep this way for a > release and then move both to the common place in arch_topology.
Yeah, that seems like a good idea. I'll let this patch just touch RISC-V for the sake of backporting & create a second patch to move to a common implementation.
Since I've not modified any real arch code before, I'd rather make that a separate patch/series too for the sake of getting this patch applied as a v5.19-rc(late) fix.
| |