Messages in this thread | | | Date | Wed, 10 Feb 2021 09:05:14 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH] x86, sched: Allow NUMA nodes to share an LLC on Intel platforms |
| |
On Tue, Feb 09, 2021 at 02:39:43PM -0800, Alison Schofield wrote:
> Commit 1340ccfa9a9a ("x86,sched: Allow topologies where NUMA nodes > share an LLC") added a vendor and model specific check to skip the > topology_sane() check for Intel's Sky Lake Server CPUs where NUMA > nodes shared an LLC. > > This topology is no longer a quirk for Intel CPUs as Ice Lake and > Sapphire Rapids CPUs exhibit the same topology. Rather than maintain > the quirk list, define a synthetic flag that directs the scheduler > to allow this topology without warning for all Intel CPUs when NUMA > is configured.
Hurmph, I still think it's daft.
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c > index 816fdbec795a..027348261080 100644 > --- a/arch/x86/kernel/cpu/intel.c > +++ b/arch/x86/kernel/cpu/intel.c > @@ -719,6 +719,21 @@ static void init_intel(struct cpuinfo_x86 *c) > tsx_disable(); > > split_lock_init(); > + > + /* > + * Set X86_BUG_NUMA_SHARES_LLC to allow topologies where NUMA > + * nodes share an LLC. In Sub-NUMA Clustering mode Intel CPUs > + * may enumerate an LLC as shared by multiple NUMA nodes. The > + * LLC is shared for off-package data access but private to > + * the NUMA node for on-package access. This topology first > + * appeared in SKYLAKE_X. It was treated as a quirk and allowed. > + * This topology reappeared in ICELAKE_X and SAPPHIRERAPIDS_X. > + * Rather than maintain a list of quirk CPUS, allow this topology > + * on all Intel CPUs with NUMA configured. When this X86_BUG is > + * set, the scheduler accepts this topology without warning. > + */ > + 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.
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_ :-(
| |