Messages in this thread Patch in this message | | | Date | Wed, 10 Feb 2021 20:38:42 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH] x86, sched: Allow NUMA nodes to share an LLC on Intel platforms |
| |
On Wed, Feb 10, 2021 at 07:22:03AM -0800, Dave Hansen wrote: > On 2/10/21 12:05 AM, Peter Zijlstra wrote: > >> + if (IS_ENABLED(CONFIG_NUMA)) > >> + set_cpu_bug(c, X86_BUG_NUMA_SHARES_LLC); > >> } > > This seens wrong too, it shouldn't be allowed pre SKX. And ideally only > > be allowed when SNC is enabled. > > Originally, this just added a few more models to the list of CPUs with > SNC. I was hoping for something a bit more durable that we wouldn't > have to go back and poke at every year or two.
It's not like we don't have to update a gazillion FMS tables for each new instance anyway :-(
> > Please make this more specific than: all Intel CPUs. Ofcourse, since you > > all knew this was an issue, you could've made it discoverable > > _somewhere_ :-( > > You're totally right, of course. The hardware could enumerate SNC as a > feature explicitly somewhere. But, that's a little silly because all of > the information that it's enumerating about the CPU caches and NUMA > nodes present and correct is *correct*. The secondary information would > only be for the CPU to say, "yeah, I'm really sure about that other stuff". > > I think this sanity check has outlived its usefulness.
Maybe BIOS monkeys got better, but I'm not sure I trust it all.
So SNC is all on-package, do all those nodes have the same pkg id? That is, I'm trying to find something to restrict topological madness.
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index 88cd0064d1f8..de1010dd0bba 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -458,6 +458,26 @@ static bool match_smt(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) return false; } +static bool match_die(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) +{ + if ((c->phys_proc_id == o->phys_proc_id) && + (c->cpu_die_id == o->cpu_die_id)) + return true; + return false; +} + +/* + * Unlike the other levels, we do not enforce keeping a + * multicore group inside a NUMA node. If this happens, we will + * discard the MC level of the topology later. + */ +static bool match_pkg(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) +{ + if (c->phys_proc_id == o->phys_proc_id) + return true; + return false; +} + /* * Define snc_cpu[] for SNC (Sub-NUMA Cluster) CPUs. * @@ -495,33 +515,12 @@ static bool match_llc(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) * means 'c' does not share the LLC of 'o'. This will be * reflected to userspace. */ - if (!topology_same_node(c, o) && x86_match_cpu(snc_cpu)) + if (!topology_same_node(c, o) && x86_match_cpu(snc_cpu) && match_pkg(c, o)) return false; return topology_sane(c, o, "llc"); } -/* - * Unlike the other levels, we do not enforce keeping a - * multicore group inside a NUMA node. If this happens, we will - * discard the MC level of the topology later. - */ -static bool match_pkg(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) -{ - if (c->phys_proc_id == o->phys_proc_id) - return true; - return false; -} - -static bool match_die(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) -{ - if ((c->phys_proc_id == o->phys_proc_id) && - (c->cpu_die_id == o->cpu_die_id)) - return true; - return false; -} - - #if defined(CONFIG_SCHED_SMT) || defined(CONFIG_SCHED_MC) static inline int x86_sched_itmt_flags(void) {
| |