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)
{
