[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fe9a4cd5-f74d-4c89-8667-8b5b1841b84e@intel.com>
Date: Thu, 30 May 2024 13:21:01 -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 08/20] x86/resctrl: Prepare for new Sub-NUMA Cluster
(SNC) monitor files
Hi Tony,
On 5/28/24 3:19 PM, Tony Luck wrote:
> When SNC is enabled monitoring data is collected at the SNC node
> granularity, but must be reported at L3-cache granularity for
> backwards compatibility in addition to reporting at the node
> level.
>
> Add a "ci" field to the rdt_mon_domain structure to save the
> cache information about the enclosing L3 cache for the domain.
> This provides:
>
> 1) The cache id which is needed to compose the name of the legacy
> monitoring directory, and to determine which domains should be
> summed to provide L3-scoped data.
>
> 2) The shared_cpu_map which is needed to determine which CPUs can
> be used to read the RMID counters with the MSR interface.
>
> This is the first step to an eventual goal of monitor reporting files
> like this (for a system with two SNC nodes per L3):
>
> $ cd /sys/fs/resctrl/mon_data
> $ tree mon_L3_00
> mon_L3_00 <- 00 here is L3 cache id
> ├── llc_occupancy \ These files provide legacy support
> ├── mbm_local_bytes > for non-SNC aware monitor apps
> ├── mbm_total_bytes / that expect data at L3 cache level
> ├── mon_sub_L3_00 <- 00 here is SNC node id
> │ ├── llc_occupancy \ These files are finer grained
> │ ├── mbm_local_bytes > data from each SNC node
> │ └── mbm_total_bytes /
> └── mon_sub_L3_01
> ├── llc_occupancy \
> ├── mbm_local_bytes > As above, but for node 1.
> └── mbm_total_bytes /
>
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
> include/linux/resctrl.h | 2 ++
> arch/x86/kernel/cpu/resctrl/internal.h | 21 +++++++++++++++++++++
> arch/x86/kernel/cpu/resctrl/core.c | 7 ++++++-
> arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 1 -
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 1 -
> 5 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 64b6ad1b22a1..d733e1f6485d 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -96,6 +96,7 @@ struct rdt_ctrl_domain {
> /**
> * struct rdt_mon_domain - group of CPUs sharing a resctrl monitor resource
> * @hdr: common header for different domain types
> + * @ci: cache info for this domain
> * @rmid_busy_llc: bitmap of which limbo RMIDs are above threshold
> * @mbm_total: saved state for MBM total bandwidth
> * @mbm_local: saved state for MBM local bandwidth
> @@ -106,6 +107,7 @@ struct rdt_ctrl_domain {
> */
> struct rdt_mon_domain {
> struct rdt_domain_hdr hdr;
> + struct cacheinfo *ci;
> unsigned long *rmid_busy_llc;
> struct mbm_state *mbm_total;
> struct mbm_state *mbm_local;
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 135190e0711c..eb70d3136ced 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -2,6 +2,7 @@
> #ifndef _ASM_X86_RESCTRL_INTERNAL_H
> #define _ASM_X86_RESCTRL_INTERNAL_H
>
> +#include <linux/cacheinfo.h>
> #include <linux/resctrl.h>
> #include <linux/sched.h>
> #include <linux/kernfs.h>
> @@ -509,6 +510,26 @@ static inline bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level l)
>
> int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable);
>
> +/*
> + * Get the cacheinfo structure of the cache associated with @cpu at level @level.
> + * cpuhp lock must be held.
> + */
> +static inline struct cacheinfo *get_cpu_cacheinfo_level(int cpu, int level)
> +{
> + struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
> + int i;
> +
> + for (i = 0; i < ci->num_leaves; i++) {
> + if (ci->info_list[i].level == level) {
> + if (ci->info_list[i].attributes & CACHE_ID)
> + return &ci->info_list[i];
> + break;
> + }
> + }
> +
> + return NULL;
> +}
> +
This does not belong in resctrl. It really looks to partner well with existing
cache helpers in include/linux/cacheinfo.h that already contains get_cpu_cacheinfo_id().
Considering the existing naming get_cpu_cacheinfo() may be more appropriate.
Reinette
Powered by blists - more mailing lists