| Message ID | 20220704101605.1318280-10-sudeep.holla@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arch_topology: Updates to add socket support and fix cluster ids | expand |
Hi Sudeep, On Mon, Jul 4, 2022 at 12:19 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > Currently ACPI populates just the minimum information about the last > level cache from PPTT in order to feed the same to build sched_domains. > Similar support for DT platforms is not present. > > In order to enable the same, the entire cache hierarchy information can > be built as part of CPU topoplogy parsing both on ACPI and DT platforms. > > Note that this change builds the cacheinfo early even on ACPI systems, > but the current mechanism of building llc_sibling mask remains unchanged. > > Tested-by: Ionela Voinescu <ionela.voinescu@arm.com> > Reviewed-by: Gavin Shan <gshan@redhat.com> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> Thanks for your patch! > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c > @@ -7,6 +7,7 @@ > */ > > #include <linux/acpi.h> > +#include <linux/cacheinfo.h> > #include <linux/cpu.h> > #include <linux/cpufreq.h> > #include <linux/device.h> > @@ -780,15 +781,28 @@ __weak int __init parse_acpi_topology(void) > #if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) > void __init init_cpu_topology(void) > { > + int ret, cpu; > + > reset_cpu_topology(); > + ret = parse_acpi_topology(); > + if (!ret) > + ret = of_have_populated_dt() && parse_dt_topology(); > > - /* > - * Discard anything that was parsed if we hit an error so we > - * don't use partial information. > - */ > - if (parse_acpi_topology()) > - reset_cpu_topology(); > - else if (of_have_populated_dt() && parse_dt_topology()) > + if (ret) { > + /* > + * Discard anything that was parsed if we hit an error so we > + * don't use partial information. > + */ > reset_cpu_topology(); > + return; > + } > + > + for_each_possible_cpu(cpu) { > + ret = detect_cache_attributes(cpu); > + if (ret) { > + pr_info("Early cacheinfo failed, ret = %d\n", ret); This is triggered Early cacheinfo failed, ret = -12 on all my RV64 platforms (K210, PolarFire, StarLight). -12 = -ENOMEM. The boot continues regardless, and the K210 even has enough spare RAM after boot to run "ls", unlike two weeks ago ;-) > + break; > + } > + } > } Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 19/07/2022 15:22, Geert Uytterhoeven wrote: > Hi Sudeep, > Hey Geert, > On Mon, Jul 4, 2022 at 12:19 PM Sudeep Holla <sudeep.holla@arm.com> wrote: >> Currently ACPI populates just the minimum information about the last >> level cache from PPTT in order to feed the same to build sched_domains. >> Similar support for DT platforms is not present. >> >> In order to enable the same, the entire cache hierarchy information can >> be built as part of CPU topoplogy parsing both on ACPI and DT platforms. >> >> Note that this change builds the cacheinfo early even on ACPI systems, >> but the current mechanism of building llc_sibling mask remains unchanged. >> >> Tested-by: Ionela Voinescu <ionela.voinescu@arm.com> >> Reviewed-by: Gavin Shan <gshan@redhat.com> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > Thanks for your patch! > >> --- a/drivers/base/arch_topology.c >> +++ b/drivers/base/arch_topology.c >> @@ -7,6 +7,7 @@ >> */ >> >> #include <linux/acpi.h> >> +#include <linux/cacheinfo.h> >> #include <linux/cpu.h> >> #include <linux/cpufreq.h> >> #include <linux/device.h> >> @@ -780,15 +781,28 @@ __weak int __init parse_acpi_topology(void) >> #if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) >> void __init init_cpu_topology(void) >> { >> + int ret, cpu; >> + >> reset_cpu_topology(); >> + ret = parse_acpi_topology(); >> + if (!ret) >> + ret = of_have_populated_dt() && parse_dt_topology(); >> >> - /* >> - * Discard anything that was parsed if we hit an error so we >> - * don't use partial information. >> - */ >> - if (parse_acpi_topology()) >> - reset_cpu_topology(); >> - else if (of_have_populated_dt() && parse_dt_topology()) >> + if (ret) { >> + /* >> + * Discard anything that was parsed if we hit an error so we >> + * don't use partial information. >> + */ >> reset_cpu_topology(); >> + return; >> + } >> + >> + for_each_possible_cpu(cpu) { >> + ret = detect_cache_attributes(cpu); >> + if (ret) { >> + pr_info("Early cacheinfo failed, ret = %d\n", ret); > > This is triggered > > Early cacheinfo failed, ret = -12 > > on all my RV64 platforms (K210, PolarFire, StarLight). This should be fixed by Sudeeps most recent patchset, at least it was when I tested it! https://lore.kernel.org/all/20220713133344.1201247-1-sudeep.holla@arm.com/ > -12 = -ENOMEM. > > The boot continues regardless, and the K210 even has enough spare > RAM after boot to run "ls", unlike two weeks ago ;-) > >> + break; >> + } >> + } >> } > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Jul 19, 2022 at 03:37:22PM +0100, Conor Dooley wrote: > On 19/07/2022 15:22, Geert Uytterhoeven wrote: > > Hi Sudeep, > > > > Hey Geert, > [...] > > > > This is triggered > > > > Early cacheinfo failed, ret = -12 > > > > on all my RV64 platforms (K210, PolarFire, StarLight). > > This should be fixed by Sudeeps most recent patchset, at least > it was when I tested it! > https://lore.kernel.org/all/20220713133344.1201247-1-sudeep.holla@arm.com/ > Conor you beat me in the response speed :). > > -12 = -ENOMEM. > > > > The boot continues regardless, and the K210 even has enough spare > > RAM after boot to run "ls", unlike two weeks ago ;-) > > Yes Conor initially reported this and I suspected something to do with per-cpu allocation as the early cacheinfo failed but succeeded in device initcall level. However when fixing some hotplug issue, I moved the detection of cache attributes on all cpus from boot cpu to individual CPUs in the secondary startup which seem to fix the issue as I assume the per-cpu allocation is ready to use at that stage. However we still have one pending issue[0] to address even after [1], but that doesn't affect DT platforms.
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index 579c851a2bd7..e2f7d9ea558e 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -7,6 +7,7 @@ */ #include <linux/acpi.h> +#include <linux/cacheinfo.h> #include <linux/cpu.h> #include <linux/cpufreq.h> #include <linux/device.h> @@ -780,15 +781,28 @@ __weak int __init parse_acpi_topology(void) #if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) void __init init_cpu_topology(void) { + int ret, cpu; + reset_cpu_topology(); + ret = parse_acpi_topology(); + if (!ret) + ret = of_have_populated_dt() && parse_dt_topology(); - /* - * Discard anything that was parsed if we hit an error so we - * don't use partial information. - */ - if (parse_acpi_topology()) - reset_cpu_topology(); - else if (of_have_populated_dt() && parse_dt_topology()) + if (ret) { + /* + * Discard anything that was parsed if we hit an error so we + * don't use partial information. + */ reset_cpu_topology(); + return; + } + + for_each_possible_cpu(cpu) { + ret = detect_cache_attributes(cpu); + if (ret) { + pr_info("Early cacheinfo failed, ret = %d\n", ret); + break; + } + } } #endif