From: Conor Dooley <[email protected]>
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: [email protected]
Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
Reported-by: Brice Goglin <[email protected]>
Link: https://github.com/open-mpi/hwloc/issues/536
Signed-off-by: Conor Dooley <[email protected]>
---
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);
+}
base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
--
2.37.0
On Wed, Jul 6, 2022 at 11:46 AM Conor Dooley <[email protected]> wrote:
>
> From: Conor Dooley <[email protected]>
>
> 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: [email protected]
> Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
> Reported-by: Brice Goglin <[email protected]>
> Link: https://github.com/open-mpi/hwloc/issues/536
> Signed-off-by: Conor Dooley <[email protected]>
> ---
>
> 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 ?
> base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
> --
> 2.37.0
>
--
Regards,
Atish
Hi Conor,
I love your patch! Yet something to improve:
[auto build test ERROR on b6f1f2fa2bddd69ff46a190b8120bd440fd50563]
url: https://github.com/intel-lab-lkp/linux/commits/Conor-Dooley/riscv-arch-topology-fix-default-topology-reporting/20220707-024856
base: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
config: riscv-randconfig-r042-20220706 (https://download.01.org/0day-ci/archive/20220707/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project f553287b588916de09c66e3e32bf75e5060f967f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/824f4c3cb56ada865e6e3b14457c0582fa255cbf
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Conor-Dooley/riscv-arch-topology-fix-default-topology-reporting/20220707-024856
git checkout 824f4c3cb56ada865e6e3b14457c0582fa255cbf
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash arch/riscv/kernel/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> arch/riscv/kernel/topology.c:17:37: error: use of undeclared identifier 'cpu_topology'
struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
^
>> arch/riscv/kernel/topology.c:31:2: error: call to undeclared function 'update_siblings_masks'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
update_siblings_masks(cpuid);
^
2 errors generated.
vim +/cpu_topology +17 arch/riscv/kernel/topology.c
14
15 void store_cpu_topology(unsigned int cpuid)
16 {
> 17 struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
18
19 if (cpuid_topo->package_id != -1)
20 goto topology_populated;
21
22 cpuid_topo->thread_id = -1;
23 cpuid_topo->core_id = cpuid;
24 cpuid_topo->package_id = cpu_to_node(cpuid);
25
26 pr_debug("CPU%u: package %d core %d thread %d\n",
27 cpuid, cpuid_topo->package_id, cpuid_topo->core_id,
28 cpuid_topo->thread_id);
29
30 topology_populated:
> 31 update_siblings_masks(cpuid);
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Wed, Jul 06, 2022 at 02:38:01PM -0700, Atish Patra wrote:
> On Wed, Jul 6, 2022 at 11:46 AM Conor Dooley <[email protected]> wrote:
> >
> > From: Conor Dooley <[email protected]>
> >
> > 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: [email protected]
> > Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
> > Reported-by: Brice Goglin <[email protected]>
> > Link: https://github.com/open-mpi/hwloc/issues/536
> > Signed-off-by: Conor Dooley <[email protected]>
> > ---
> >
> > 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.
> > base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
> > --
> > 2.37.0
> >
>
>
> --
> Regards,
> Atish
--
Regards,
Sudeep
On Wed, Jul 06, 2022 at 07:45:59PM +0100, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> 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: [email protected]
> Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
> Reported-by: Brice Goglin <[email protected]>
> Link: https://github.com/open-mpi/hwloc/issues/536
> Signed-off-by: Conor Dooley <[email protected]>
> ---
>
> 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.
--
Regards,
Sudeep
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 <[email protected]> wrote:
>>>
>>> From: Conor Dooley <[email protected]>
>>>
>>> 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: [email protected]
>>> Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
>>> Reported-by: Brice Goglin <[email protected]>
>>> Link: https://github.com/open-mpi/hwloc/issues/536
>>> Signed-off-by: Conor Dooley <[email protected]>
>>> ---
>>>
>>> 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.
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 <[email protected]>
>>
>> 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: [email protected]
>> Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
>> Reported-by: Brice Goglin <[email protected]>
>> Link: https://github.com/open-mpi/hwloc/issues/536
>> Signed-off-by: Conor Dooley <[email protected]>
>> ---
>>
>> 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.