[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c6651270-25a3-4d9e-91f8-4aa9c8652b98@intel.com>
Date: Thu, 30 May 2024 13:28:47 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghua.yu@...el.com>,
"Maciej Wieczor-Retman" <maciej.wieczor-retman@...el.com>, Peter Newman
<peternewman@...gle.com>, James Morse <james.morse@....com>, Babu Moger
<babu.moger@....com>, Drew Fustini <dfustini@...libre.com>, Dave Martin
<Dave.Martin@....com>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
<patches@...ts.linux.dev>
Subject: Re: [PATCH v19 19/20] x86/resctrl: Sub-NUMA Cluster (SNC) detection
and enabling
Hi Tony,
should "and enabling" be dropped from shortlog?
On 5/28/24 3:20 PM, Tony Luck wrote:
> There isn't a simple hardware bit that indicates whether a CPU is
> running in Sub-NUMA Cluster (SNC) mode. Infer the state by comparing
> number CPUs sharing the L3 cache with CPU0 to the number of CPUs in
> the same NUMA node as CPU0.
>
> If SNC mode is detected, print a single informational message to the
> console.
>
> Add the missing definition of pr_fmt() to monitor.c. This wasn't
> noticed before as there are only "can't happen" console messages
> from this file.
>
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
> arch/x86/kernel/cpu/resctrl/monitor.c | 59 +++++++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index c7559735e33a..1c5162a68461 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -15,6 +15,8 @@
> * Software Developer Manual June 2016, volume 3, section 17.17.
> */
>
> +#define pr_fmt(fmt) "resctrl: " fmt
> +
> #include <linux/cpu.h>
> #include <linux/module.h>
> #include <linux/sizes.h>
> @@ -1095,6 +1097,61 @@ void arch_mon_domain_online(struct rdt_resource *r, struct rdt_mon_domain *d)
> wrmsrl(MSR_RMID_SNC_CONFIG, val);
> }
>
> +/* CPU models that support MSR_RMID_SNC_CONFIG */
> +static const struct x86_cpu_id snc_cpu_ids[] __initconst = {
> + X86_MATCH_VFM(INTEL_ICELAKE_X, 0),
> + X86_MATCH_VFM(INTEL_SAPPHIRERAPIDS_X, 0),
> + X86_MATCH_VFM(INTEL_EMERALDRAPIDS_X, 0),
> + X86_MATCH_VFM(INTEL_GRANITERAPIDS_X, 0),
> + X86_MATCH_VFM(INTEL_ATOM_CRESTMONT_X, 0),
> + {}
> +};
> +
> +/*
> + * There isn't a simple hardware bit that indicates whether a CPU is running
> + * in Sub-NUMA Cluster (SNC) mode. Infer the state by comparing the
> + * number CPUs sharing the L3 cache with CPU0 to the number of CPUs in
> + * the same NUMA node as CPU0.
> + * It is not possible to accurately determine SNC state if the system is
> + * booted with a maxcpus=N parameter. That distorts the ratio of SNC nodes
> + * to L3 caches. It will be OK if system is booted with hyperthreading
> + * disabled (since this doesn't affect the ratio).
> + */
> +static __init int snc_get_config(void)
> +{
> + struct cacheinfo *ci = get_cpu_cacheinfo_level(0, RESCTRL_L3_CACHE);
> + const cpumask_t *node0_cpumask;
Stray tab
> + int ret;
> +
> + if (!x86_match_cpu(snc_cpu_ids) || !ci)
> + return 1;
> +
> + cpus_read_lock();
> + if (num_online_cpus() != num_present_cpus())
> + pr_warn("Some CPUs offline, SNC detection may be incorrect\n");
> + cpus_read_unlock();
> +
> + node0_cpumask = cpumask_of_node(cpu_to_node(0));
> +
> + ret = cpumask_weight(&ci->shared_cpu_map) / cpumask_weight(node0_cpumask);
It is not obvious to the static checker I tried that cpumask_weight(node0_cpumask)
cannot be zero. Can you please insert a check to make static checkers happy?
> +
> + /* sanity check: Only valid results are 1, 2, 3, 4 */
> + switch (ret) {
> + case 1:
> + break;
> + case 2 ... 4:
> + pr_info("Sub-NUMA Cluster mode detected with %d nodes per L3 cache\n", ret);
> + rdt_resources_all[RDT_RESOURCE_L3].r_resctrl.mon_scope = RESCTRL_L3_NODE;
> + break;
> + default:
> + pr_warn("Ignore improbable SNC node count %d\n", ret);
> + ret = 1;
> + break;
> + }
> +
> + return ret;
> +}
> +
> int __init rdt_get_mon_l3_config(struct rdt_resource *r)
> {
> unsigned int mbm_offset = boot_cpu_data.x86_cache_mbm_width_offset;
> @@ -1102,6 +1159,8 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
> unsigned int threshold;
> int ret;
>
> + snc_nodes_per_l3_cache = snc_get_config();
> +
> resctrl_rmid_realloc_limit = boot_cpu_data.x86_cache_size * 1024;
> hw_res->mon_scale = boot_cpu_data.x86_cache_occ_scale / snc_nodes_per_l3_cache;
> r->num_rmid = (boot_cpu_data.x86_cache_max_rmid + 1) / snc_nodes_per_l3_cache;
Reinette
Powered by blists - more mailing lists