| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 30 May 2024 17:26:03 -0700
From: Tony Luck <tony.luck@...el.com>
To: Reinette Chatre <reinette.chatre@...el.com>
Cc: 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>, 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
On Thu, May 30, 2024 at 01:21:01PM -0700, Reinette Chatre wrote:
> 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,
The name get_cpu_cacheinfo() already exists and does something different
(returns a "struct cpu_cacheinfo *" rather than a "struct cacheinfo *").
How does this look for the change to <linux/cacheinfo.h> ... add a new
function get_cpu_cacheinfo_level() and then use it as a helper for the
existing get_cpu_cacheinfo_id()
-Tony
---
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
index 2cb15fe4fe12..301b0b24f446 100644
--- a/include/linux/cacheinfo.h
+++ b/include/linux/cacheinfo.h
@@ -113,10 +113,10 @@ int acpi_get_cache_info(unsigned int cpu,
const struct attribute_group *cache_get_priv_group(struct cacheinfo *this_leaf);
/*
- * Get the id of the cache associated with @cpu at level @level.
+ * Get the cacheinfo structure for cache associated with @cpu at level @level.
* cpuhp lock must be held.
*/
-static inline int get_cpu_cacheinfo_id(int cpu, int level)
+static inline struct cacheinfo *get_cpu_cacheinfo_level(int cpu, int level)
{
struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
int i;
@@ -124,12 +124,23 @@ static inline int get_cpu_cacheinfo_id(int cpu, int level)
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].id;
- return -1;
+ return &ci->info_list[i];
+ return NULL;
}
}
- return -1;
+ return NULL;
+}
+
+/*
+ * Get the id of the cache associated with @cpu at level @level.
+ * cpuhp lock must be held.
+ */
+static inline int get_cpu_cacheinfo_id(int cpu, int level)
+{
+ struct cacheinfo *ci = get_cpu_cacheinfo_level(cpu, level);
+
+ return ci ? ci->id : -1;
}
#ifdef CONFIG_ARM64
Powered by blists - more mailing lists