* [PATCH] riscv: arch-topology: fix default topology reporting
@ 2022-07-06 18:45 ` Conor Dooley
0 siblings, 0 replies; 14+ messages in thread
From: Conor Dooley @ 2022-07-06 18:45 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Palmer Dabbelt, Albert Ou, Sudeep Holla
Cc: Daire McNamara, Conor Dooley, Niklas Cassel, Damien Le Moal,
Geert Uytterhoeven, Zong Li, Emil Renner Berthing,
Jonas Hahnfeld, Guo Ren, Anup Patel, Atish Patra, Changbin Du,
Heiko Stuebner, Philipp Tomsich, linux-riscv, linux-kernel,
Brice Goglin
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);
+}
base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
--
2.37.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] riscv: arch-topology: fix default topology reporting
2022-07-06 18:45 ` Conor Dooley
@ 2022-07-06 21:38 ` Atish Patra
-1 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2022-07-06 21:38 UTC (permalink / raw)
To: Conor Dooley
Cc: Paul Walmsley, Palmer Dabbelt, Palmer Dabbelt, Albert Ou,
Sudeep Holla, Daire McNamara, Conor Dooley, Niklas Cassel,
Damien Le Moal, Geert Uytterhoeven, Zong Li,
Emil Renner Berthing, Jonas Hahnfeld, Guo Ren, Anup Patel,
Changbin Du, Heiko Stuebner, Philipp Tomsich, linux-riscv,
linux-kernel@vger.kernel.org List, Brice Goglin
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 ?
> base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
> --
> 2.37.0
>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] riscv: arch-topology: fix default topology reporting
@ 2022-07-06 21:38 ` Atish Patra
0 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2022-07-06 21:38 UTC (permalink / raw)
To: Conor Dooley
Cc: Paul Walmsley, Palmer Dabbelt, Palmer Dabbelt, Albert Ou,
Sudeep Holla, Daire McNamara, Conor Dooley, Niklas Cassel,
Damien Le Moal, Geert Uytterhoeven, Zong Li,
Emil Renner Berthing, Jonas Hahnfeld, Guo Ren, Anup Patel,
Changbin Du, Heiko Stuebner, Philipp Tomsich, linux-riscv,
linux-kernel@vger.kernel.org List, Brice Goglin
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 ?
> base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
> --
> 2.37.0
>
--
Regards,
Atish
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] riscv: arch-topology: fix default topology reporting
2022-07-06 21:38 ` Atish Patra
@ 2022-07-07 9:47 ` Sudeep Holla
-1 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2022-07-07 9:47 UTC (permalink / raw)
To: Atish Patra
Cc: Conor Dooley, Paul Walmsley, Palmer Dabbelt, Palmer Dabbelt,
Albert Ou, Daire McNamara, Conor Dooley, Niklas Cassel,
Damien Le Moal, Geert Uytterhoeven, Zong Li,
Emil Renner Berthing, Jonas Hahnfeld, Guo Ren, Anup Patel,
Changbin Du, Heiko Stuebner, Philipp Tomsich, linux-riscv,
linux-kernel@vger.kernel.org List, Brice Goglin
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.
> > base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
> > --
> > 2.37.0
> >
>
>
> --
> Regards,
> Atish
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] riscv: arch-topology: fix default topology reporting
@ 2022-07-07 9:47 ` Sudeep Holla
0 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2022-07-07 9:47 UTC (permalink / raw)
To: Atish Patra
Cc: Conor Dooley, Paul Walmsley, Palmer Dabbelt, Palmer Dabbelt,
Albert Ou, Daire McNamara, Conor Dooley, Niklas Cassel,
Damien Le Moal, Geert Uytterhoeven, Zong Li,
Emil Renner Berthing, Jonas Hahnfeld, Guo Ren, Anup Patel,
Changbin Du, Heiko Stuebner, Philipp Tomsich, linux-riscv,
linux-kernel@vger.kernel.org List, Brice Goglin
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.
> > base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
> > --
> > 2.37.0
> >
>
>
> --
> Regards,
> Atish
--
Regards,
Sudeep
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] riscv: arch-topology: fix default topology reporting
2022-07-07 9:47 ` Sudeep Holla
@ 2022-07-07 10:12 ` Conor.Dooley
-1 siblings, 0 replies; 14+ messages in thread
From: Conor.Dooley @ 2022-07-07 10:12 UTC (permalink / raw)
To: sudeep.holla, atishp
Cc: paul.walmsley, palmer, palmer, aou, Daire.McNamara, Conor.Dooley,
niklas.cassel, damien.lemoal, geert, zong.li, kernel, hahnjo,
guoren, anup, changbin.du, heiko, philipp.tomsich, linux-riscv,
linux-kernel, Brice.Goglin
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.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] riscv: arch-topology: fix default topology reporting
@ 2022-07-07 10:12 ` Conor.Dooley
0 siblings, 0 replies; 14+ messages in thread
From: Conor.Dooley @ 2022-07-07 10:12 UTC (permalink / raw)
To: sudeep.holla, atishp
Cc: paul.walmsley, palmer, palmer, aou, Daire.McNamara, Conor.Dooley,
niklas.cassel, damien.lemoal, geert, zong.li, kernel, hahnjo,
guoren, anup, changbin.du, heiko, philipp.tomsich, linux-riscv,
linux-kernel, Brice.Goglin
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.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] riscv: arch-topology: fix default topology reporting
2022-07-06 18:45 ` Conor Dooley
@ 2022-07-06 23:38 ` kernel test robot
-1 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2022-07-06 23:38 UTC (permalink / raw)
To: Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou, Sudeep Holla
Cc: llvm, kbuild-all, Daire McNamara, Conor Dooley, Niklas Cassel,
Damien Le Moal, Geert Uytterhoeven, Zong Li,
Emil Renner Berthing, Jonas Hahnfeld, Guo Ren, Anup Patel,
Atish Patra, Changbin Du, Heiko Stuebner, Philipp Tomsich,
linux-riscv, linux-kernel, Brice Goglin
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/202207070728.MiQn5fv0-lkp@intel.com/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 <lkp@intel.com>
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] riscv: arch-topology: fix default topology reporting
@ 2022-07-06 23:38 ` kernel test robot
0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2022-07-06 23:38 UTC (permalink / raw)
To: Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou, Sudeep Holla
Cc: llvm, kbuild-all, Daire McNamara, Conor Dooley, Niklas Cassel,
Damien Le Moal, Geert Uytterhoeven, Zong Li,
Emil Renner Berthing, Jonas Hahnfeld, Guo Ren, Anup Patel,
Atish Patra, Changbin Du, Heiko Stuebner, Philipp Tomsich,
linux-riscv, linux-kernel, Brice Goglin
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/202207070728.MiQn5fv0-lkp@intel.com/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 <lkp@intel.com>
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] riscv: arch-topology: fix default topology reporting
2022-07-06 18:45 ` Conor Dooley
@ 2022-07-07 9:50 ` Sudeep Holla
-1 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2022-07-07 9:50 UTC (permalink / raw)
To: Conor Dooley
Cc: Paul Walmsley, Palmer Dabbelt, Palmer Dabbelt, Albert Ou,
Daire McNamara, Sudeep Holla, Conor Dooley, Niklas Cassel,
Damien Le Moal, Geert Uytterhoeven, Zong Li,
Emil Renner Berthing, Jonas Hahnfeld, Guo Ren, Anup Patel,
Atish Patra, Changbin Du, Heiko Stuebner, Philipp Tomsich,
linux-riscv, linux-kernel, Brice Goglin
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.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] riscv: arch-topology: fix default topology reporting
@ 2022-07-07 9:50 ` Sudeep Holla
0 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2022-07-07 9:50 UTC (permalink / raw)
To: Conor Dooley
Cc: Paul Walmsley, Palmer Dabbelt, Palmer Dabbelt, Albert Ou,
Daire McNamara, Sudeep Holla, Conor Dooley, Niklas Cassel,
Damien Le Moal, Geert Uytterhoeven, Zong Li,
Emil Renner Berthing, Jonas Hahnfeld, Guo Ren, Anup Patel,
Atish Patra, Changbin Du, Heiko Stuebner, Philipp Tomsich,
linux-riscv, linux-kernel, Brice Goglin
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.
--
Regards,
Sudeep
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] riscv: arch-topology: fix default topology reporting
2022-07-07 9:50 ` Sudeep Holla
@ 2022-07-07 10:15 ` Conor.Dooley
-1 siblings, 0 replies; 14+ messages in thread
From: Conor.Dooley @ 2022-07-07 10:15 UTC (permalink / raw)
To: sudeep.holla
Cc: paul.walmsley, palmer, palmer, aou, Daire.McNamara,
niklas.cassel, damien.lemoal, geert, zong.li, kernel, hahnjo,
guoren, anup, atishp, changbin.du, heiko, philipp.tomsich,
linux-riscv, mail, linux-kernel, Brice.Goglin
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.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] riscv: arch-topology: fix default topology reporting
@ 2022-07-07 10:15 ` Conor.Dooley
0 siblings, 0 replies; 14+ messages in thread
From: Conor.Dooley @ 2022-07-07 10:15 UTC (permalink / raw)
To: sudeep.holla
Cc: paul.walmsley, palmer, palmer, aou, Daire.McNamara,
niklas.cassel, damien.lemoal, geert, zong.li, kernel, hahnjo,
guoren, anup, atishp, changbin.du, heiko, philipp.tomsich,
linux-riscv, mail, linux-kernel, Brice.Goglin
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
^ permalink raw reply [flat|nested] 14+ messages in thread