* [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains
@ 2026-08-31 23:54 Reinette Chatre
2026-09-02 16:10 ` Ben Horgan
0 siblings, 1 reply; 10+ messages in thread
From: Reinette Chatre @ 2026-08-31 23:54 UTC (permalink / raw)
To: ben.horgan, james.morse, Dave.Martin, fenghuay
Cc: tony.luck, babu.moger, yu.c.chen, linux-arm-kernel, linux-kernel,
patches, reinette.chatre
A single struct mpam_resctrl_dom instance represents both an allocation and
monitor domain. When a system supports allocation and monitoring a single
struct mpam_resctrl_dom instance is created and then added to both the
monitor and control domain lists via
mpam_resctrl_dom::rdt_ctrl_domain::rdt_domain_hdr::list_head
and mpam_resctrl_dom::rdt_l3_mon_domain::rdt_domain_hdr::list_head
respectively.
Representing both allocation and monitoring domains with a single
architecture domain requires that allocation and monitoring be done at the
same scope and also requires a resource to have 1:1 support for an allocation
control and monitoring. The latter means that when a resource supports
multiple controls for allocation of a resource, for example a "min" bandwidth
control as well as a "max" bandwidth control for memory bandwidth allocation,
then each domain supporting each allocation control needs a matching
monitoring domain. This does not accurately represent the resource
capabilities.
Split MPAM allocation and monitoring domain management to enable a resource
to support multiple allocation controls. As an initial and knowingly
inefficient approach, duplicate struct mpam_resctrl_dom between the
monitoring and control lists while only initializing the relevant member
structs.
The goal of this conservative approach is to use something that can only
be compile tested by me to learn from MPAM folks on what the best approach
should be for MPAM to support multiple allocation controls.
This patch is extracted from the "Generic schema description" PoC [1] that,
among its various goals, aim to add support for MPAM's multiple controls
to resctrl.
This has only been compile tested by me but Fenghua's work to enable
MPAM's CPU-less NUMA nodes [2] is using it as a baseline.
I did consider a new layout as below but there seems to be a contradiction
in mpam_resctrl_alloc_domain() where domain addition for allocation as well
as monitoring domains unconditionally fails if there is no "ctrl_comp" while
a comment when adding a monitoring domain states "there may be no ctrl_comp
for the L3".
struct mpam_resctrl_mon_dom {
struct mpam_component *mon_comp[QOS_NUM_EVENTS];
struct rdt_l3_mon_domain resctrl_mon_dom;
}
struct mpam_resctrl_ctrl_dom {
struct mpam_component *ctrl_comp;
struct rdt_ctrl_domain resctrl_ctrl_dom;
};
Any feedback is appreciated.
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Link: https://lore.kernel.org/lkml/d258a32f-12d5-464d-abe9-4720fb3e44b3@intel.com/ # [1]
Link: https://lore.kernel.org/lkml/20260831172245.42253-1-fenghuay@nvidia.com/ # [2]
---
- Extracted from "generic schema description"
https://lore.kernel.org/lkml/d258a32f-12d5-464d-abe9-4720fb3e44b3@intel.com/
Changes since extract from PoC:
- Rebased on v7.3-rc1
- Re-wrote changelog
- Return NULL instead of ERR_PTR() when searching for monitoring domain
to ensure the if (!dom) check works.
---
drivers/resctrl/mpam_resctrl.c | 207 +++++++++++++++++++--------------
1 file changed, 121 insertions(+), 86 deletions(-)
diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
index 9d223057953a..c888773f3d03 100644
--- a/drivers/resctrl/mpam_resctrl.c
+++ b/drivers/resctrl/mpam_resctrl.c
@@ -1616,11 +1616,10 @@ static struct mpam_component *find_component(struct mpam_class *class, int cpu)
}
static struct mpam_resctrl_dom *
-mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res)
+mpam_resctrl_alloc_ctrl_domain(unsigned int cpu, struct mpam_resctrl_res *res)
{
int err;
struct mpam_resctrl_dom *dom;
- struct rdt_l3_mon_domain *mon_d;
struct rdt_ctrl_domain *ctrl_d;
struct mpam_class *class = res->class;
struct mpam_component *comp_iter, *ctrl_comp;
@@ -1628,6 +1627,9 @@ mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res)
lockdep_assert_held(&domain_list_lock);
+ if (!r->alloc_capable)
+ return ERR_PTR(-EINVAL);
+
ctrl_comp = NULL;
guard(srcu)(&mpam_srcu);
list_for_each_entry_srcu(comp_iter, &class->components, class_list,
@@ -1646,70 +1648,97 @@ mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res)
if (!dom)
return ERR_PTR(-ENOMEM);
- if (r->alloc_capable) {
- dom->ctrl_comp = ctrl_comp;
+ dom->ctrl_comp = ctrl_comp;
- ctrl_d = &dom->resctrl_ctrl_dom;
- mpam_resctrl_domain_hdr_init(cpu, ctrl_comp, r->rid, &ctrl_d->hdr);
- ctrl_d->hdr.type = RESCTRL_CTRL_DOMAIN;
- err = resctrl_online_ctrl_domain(r, ctrl_d);
- if (err)
- goto free_domain;
+ ctrl_d = &dom->resctrl_ctrl_dom;
+ mpam_resctrl_domain_hdr_init(cpu, ctrl_comp, r->rid, &ctrl_d->hdr);
+ ctrl_d->hdr.type = RESCTRL_CTRL_DOMAIN;
+ err = resctrl_online_ctrl_domain(r, ctrl_d);
+ if (err)
+ goto free_domain;
- mpam_resctrl_domain_insert(&r->ctrl_domains, &ctrl_d->hdr);
- } else {
- pr_debug("Skipped control domain online - no controls\n");
- }
+ mpam_resctrl_domain_insert(&r->ctrl_domains, &ctrl_d->hdr);
- if (r->mon_capable) {
- struct mpam_component *any_mon_comp = NULL;
- struct mpam_resctrl_mon *mon;
- enum resctrl_event_id eventid;
+ return dom;
- /*
- * Even if the monitor domain is backed by a different
- * component, the L3 component IDs need to be used... only
- * there may be no ctrl_comp for the L3.
- * Search each event's class list for a component with
- * overlapping CPUs and set up the dom->mon_comp array.
- */
+free_domain:
+ kfree(dom);
+ dom = ERR_PTR(err);
+
+ return dom;
+}
+
+static struct mpam_resctrl_dom *
+mpam_resctrl_alloc_mon_domain(unsigned int cpu, struct mpam_resctrl_res *res)
+{
+ int err;
+ struct mpam_resctrl_dom *dom;
+ struct rdt_l3_mon_domain *mon_d;
+ struct mpam_class *class = res->class;
+ struct mpam_component *comp_iter, *ctrl_comp;
+ struct rdt_resource *r = &res->resctrl_res;
+ struct mpam_component *any_mon_comp = NULL;
+ struct mpam_resctrl_mon *mon;
+ enum resctrl_event_id eventid;
- for_each_mpam_resctrl_mon(mon, eventid) {
- struct mpam_component *mon_comp;
+ lockdep_assert_held(&domain_list_lock);
- if (!mon->class)
- continue; // dummy resource
+ if (!r->mon_capable)
+ return ERR_PTR(-EINVAL);
- mon_comp = find_component(mon->class, cpu);
- dom->mon_comp[eventid] = mon_comp;
- if (mon_comp)
- any_mon_comp = mon_comp;
- }
- if (!any_mon_comp) {
- WARN_ON_ONCE(0);
- err = -EFAULT;
- goto offline_ctrl_domain;
+ ctrl_comp = NULL;
+ guard(srcu)(&mpam_srcu);
+ list_for_each_entry_srcu(comp_iter, &class->components, class_list,
+ srcu_read_lock_held(&mpam_srcu)) {
+ if (cpumask_test_cpu(cpu, &comp_iter->affinity)) {
+ ctrl_comp = comp_iter;
+ break;
}
+ }
- mon_d = &dom->resctrl_mon_dom;
- mpam_resctrl_domain_hdr_init(cpu, any_mon_comp, r->rid, &mon_d->hdr);
- mon_d->hdr.type = RESCTRL_MON_DOMAIN;
- err = resctrl_online_mon_domain(r, &mon_d->hdr);
- if (err)
- goto offline_ctrl_domain;
+ /* class has no component for this CPU */
+ if (WARN_ON_ONCE(!ctrl_comp))
+ return ERR_PTR(-EINVAL);
- mpam_resctrl_domain_insert(&r->mon_domains, &mon_d->hdr);
- } else {
- pr_debug("Skipped monitor domain online - no monitors\n");
+ dom = kzalloc_node(sizeof(*dom), GFP_KERNEL, cpu_to_node(cpu));
+ if (!dom)
+ return ERR_PTR(-ENOMEM);
+
+ /*
+ * Even if the monitor domain is backed by a different
+ * component, the L3 component IDs need to be used... only
+ * there may be no ctrl_comp for the L3.
+ * Search each event's class list for a component with
+ * overlapping CPUs and set up the dom->mon_comp array.
+ */
+ for_each_mpam_resctrl_mon(mon, eventid) {
+ struct mpam_component *mon_comp;
+
+ if (!mon->class)
+ continue; // dummy resource
+
+ mon_comp = find_component(mon->class, cpu);
+ dom->mon_comp[eventid] = mon_comp;
+ if (mon_comp)
+ any_mon_comp = mon_comp;
}
+ if (!any_mon_comp) {
+ WARN_ON_ONCE(0);
+ err = -EFAULT;
+ goto free_domain;
+ }
+
+ mon_d = &dom->resctrl_mon_dom;
+ mpam_resctrl_domain_hdr_init(cpu, any_mon_comp, r->rid, &mon_d->hdr);
+ mon_d->hdr.type = RESCTRL_MON_DOMAIN;
+ err = resctrl_online_mon_domain(r, &mon_d->hdr);
+ if (err)
+ goto free_domain;
+
+ mpam_resctrl_domain_insert(&r->mon_domains, &mon_d->hdr);
return dom;
-offline_ctrl_domain:
- if (r->alloc_capable) {
- mpam_resctrl_offline_domain_hdr(cpu, &ctrl_d->hdr);
- resctrl_offline_ctrl_domain(r, ctrl_d);
- }
free_domain:
kfree(dom);
dom = ERR_PTR(err);
@@ -1724,21 +1753,25 @@ mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res)
* This relies on mpam_resctrl_pick_domain_id() using the L3 cache-id
* for anything that is not a cache.
*/
-static struct mpam_resctrl_dom *mpam_resctrl_get_mon_domain_from_cpu(int cpu)
+static struct mpam_resctrl_dom *
+mpam_resctrl_get_mon_domain_from_cpu(int cpu, struct mpam_resctrl_res *res)
{
int cache_id;
struct mpam_resctrl_dom *dom;
- struct mpam_resctrl_res *l3 = &mpam_resctrl_controls[RDT_RESOURCE_L3];
+ struct rdt_resource *r = &res->resctrl_res;
lockdep_assert_cpus_held();
- if (!l3->class)
+ if (r->rid != RDT_RESOURCE_L3)
+ return NULL;
+
+ if (!res->class)
return NULL;
cache_id = get_cpu_cacheinfo_id(cpu, 3);
if (cache_id < 0)
return NULL;
- list_for_each_entry_rcu(dom, &l3->resctrl_res.mon_domains, resctrl_mon_dom.hdr.list) {
+ list_for_each_entry_rcu(dom, &res->resctrl_res.mon_domains, resctrl_mon_dom.hdr.list) {
if (dom->resctrl_mon_dom.hdr.id == cache_id)
return dom;
}
@@ -1747,7 +1780,7 @@ static struct mpam_resctrl_dom *mpam_resctrl_get_mon_domain_from_cpu(int cpu)
}
static struct mpam_resctrl_dom *
-mpam_resctrl_get_domain_from_cpu(int cpu, struct mpam_resctrl_res *res)
+mpam_resctrl_get_ctrl_domain_from_cpu(int cpu, struct mpam_resctrl_res *res)
{
struct mpam_resctrl_dom *dom;
struct rdt_resource *r = &res->resctrl_res;
@@ -1759,11 +1792,7 @@ mpam_resctrl_get_domain_from_cpu(int cpu, struct mpam_resctrl_res *res)
return dom;
}
- if (r->rid != RDT_RESOURCE_L3)
- return NULL;
-
- /* Search the mon domain list too - needed on monitor only platforms. */
- return mpam_resctrl_get_mon_domain_from_cpu(cpu);
+ return NULL;
}
int mpam_resctrl_online_cpu(unsigned int cpu)
@@ -1779,18 +1808,25 @@ int mpam_resctrl_online_cpu(unsigned int cpu)
if (!res->class)
continue; // dummy_resource;
- dom = mpam_resctrl_get_domain_from_cpu(cpu, res);
- if (!dom) {
- dom = mpam_resctrl_alloc_domain(cpu, res);
- if (IS_ERR(dom))
- return PTR_ERR(dom);
- } else {
- if (r->alloc_capable) {
+ if (r->alloc_capable) {
+ dom = mpam_resctrl_get_ctrl_domain_from_cpu(cpu, res);
+ if (!dom) {
+ dom = mpam_resctrl_alloc_ctrl_domain(cpu, res);
+ if (IS_ERR(dom))
+ return PTR_ERR(dom);
+ } else {
struct rdt_ctrl_domain *ctrl_d = &dom->resctrl_ctrl_dom;
mpam_resctrl_online_domain_hdr(cpu, &ctrl_d->hdr);
}
- if (r->mon_capable) {
+ }
+ if (r->mon_capable) {
+ dom = mpam_resctrl_get_mon_domain_from_cpu(cpu, res);
+ if (!dom) {
+ dom = mpam_resctrl_alloc_mon_domain(cpu, res);
+ if (IS_ERR(dom))
+ return PTR_ERR(dom);
+ } else {
struct rdt_l3_mon_domain *mon_d = &dom->resctrl_mon_dom;
mpam_resctrl_online_domain_hdr(cpu, &mon_d->hdr);
@@ -1815,36 +1851,35 @@ void mpam_resctrl_offline_cpu(unsigned int cpu)
struct mpam_resctrl_dom *dom;
struct rdt_l3_mon_domain *mon_d;
struct rdt_ctrl_domain *ctrl_d;
- bool ctrl_dom_empty, mon_dom_empty;
+ bool dom_empty;
struct rdt_resource *r = &res->resctrl_res;
if (!res->class)
continue; // dummy resource
- dom = mpam_resctrl_get_domain_from_cpu(cpu, res);
- if (WARN_ON_ONCE(!dom))
- continue;
-
if (r->alloc_capable) {
+ dom = mpam_resctrl_get_ctrl_domain_from_cpu(cpu, res);
+ if (WARN_ON_ONCE(!dom))
+ continue;
ctrl_d = &dom->resctrl_ctrl_dom;
- ctrl_dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &ctrl_d->hdr);
- if (ctrl_dom_empty)
+ dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &ctrl_d->hdr);
+ if (dom_empty) {
resctrl_offline_ctrl_domain(&res->resctrl_res, ctrl_d);
- } else {
- ctrl_dom_empty = true;
+ kfree(dom);
+ }
}
if (r->mon_capable) {
+ dom = mpam_resctrl_get_mon_domain_from_cpu(cpu, res);
+ if (WARN_ON_ONCE(!dom))
+ continue;
mon_d = &dom->resctrl_mon_dom;
- mon_dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &mon_d->hdr);
- if (mon_dom_empty)
+ dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &mon_d->hdr);
+ if (dom_empty) {
resctrl_offline_mon_domain(&res->resctrl_res, &mon_d->hdr);
- } else {
- mon_dom_empty = true;
+ kfree(dom);
+ }
}
-
- if (ctrl_dom_empty && mon_dom_empty)
- kfree(dom);
}
}
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains 2026-08-31 23:54 [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains Reinette Chatre @ 2026-09-02 16:10 ` Ben Horgan 2026-09-03 15:29 ` Reinette Chatre 0 siblings, 1 reply; 10+ messages in thread From: Ben Horgan @ 2026-09-02 16:10 UTC (permalink / raw) To: Reinette Chatre, james.morse, Dave.Martin, fenghuay Cc: tony.luck, babu.moger, yu.c.chen, linux-arm-kernel, linux-kernel, patches Hi Reinette, On 01/09/2026 00:54, Reinette Chatre wrote: > A single struct mpam_resctrl_dom instance represents both an allocation and > monitor domain. When a system supports allocation and monitoring a single > struct mpam_resctrl_dom instance is created and then added to both the > monitor and control domain lists via > mpam_resctrl_dom::rdt_ctrl_domain::rdt_domain_hdr::list_head > and mpam_resctrl_dom::rdt_l3_mon_domain::rdt_domain_hdr::list_head > respectively. > > Representing both allocation and monitoring domains with a single > architecture domain requires that allocation and monitoring be done at the > same scope and also requires a resource to have 1:1 support for an allocation > control and monitoring. The latter means that when a resource supports > multiple controls for allocation of a resource, for example a "min" bandwidth > control as well as a "max" bandwidth control for memory bandwidth allocation, > then each domain supporting each allocation control needs a matching > monitoring domain. This does not accurately represent the resource > capabilities. For MPAM, the scope of a control is determined by where the MSC are in a system. For the MSC in the L3 we collect the MSC in a cache instance and call that a component and collect those components and call them a class. Similarly for other caches. The scope of a control is based on the component it is in. Hence, we are unable to use all the flexibility that resctrl will offer. For MSC at the memory we currently only consider them for resctrl if there is only one memory instance (a single NUMA node) and that this corresponds to the topology of the L3 in the system (i.e only one L3 instance). This will change when NUMA scope is introduced to resctrl but for now the domain associated with the MPAM component for MSC at the memory covers all the cpus and in such a system there is only one L3 domain. The idea being that memory bandwidth controls at the memory are effectively the same as those at L3 if only if there is only one of each and no other caches in between. The initial MPAM driver put up for upstream review was more lenient than this and so the tigtening up ended up leaving some complexity around that is no longer needed. I hope to simplify this in the future and hopefully allow MPAM component to be used more directly to resctrl domain. > > Split MPAM allocation and monitoring domain management to enable a resource > to support multiple allocation controls. As an initial and knowingly > inefficient approach, duplicate struct mpam_resctrl_dom between the > monitoring and control lists while only initializing the relevant member > structs. > > The goal of this conservative approach is to use something that can only > be compile tested by me to learn from MPAM folks on what the best approach > should be for MPAM to support multiple allocation controls. > > This patch is extracted from the "Generic schema description" PoC [1] that, > among its various goals, aim to add support for MPAM's multiple controls > to resctrl. > > This has only been compile tested by me but Fenghua's work to enable > MPAM's CPU-less NUMA nodes [2] is using it as a baseline. > > I did consider a new layout as below but there seems to be a contradiction > in mpam_resctrl_alloc_domain() where domain addition for allocation as well > as monitoring domains unconditionally fails if there is no "ctrl_comp" while > a comment when adding a monitoring domain states "there may be no ctrl_comp > for the L3". Hmmm, confusing, but there is no real contradiction. Just some bad naming. Quoting the problem area: ctrl_comp = NULL; guard(srcu)(&mpam_srcu); list_for_each_entry_srcu(comp_iter, &class->components, class_list, srcu_read_lock_held(&mpam_srcu)) { if (cpumask_test_cpu(cpu, &comp_iter->affinity)) { ctrl_comp = comp_iter; break; } } /* class has no component for this CPU */ if (WARN_ON_ONCE(!ctrl_comp)) return ERR_PTR(-EINVAL); In mpam_resctrl_alloc_domain() this initial 'ctrl_comp' check ensures that there is for the class associated with the given resource, res, a component which includes the given cpu in its affinity mask. The class is all the L3 MSC or an equivalent of the same scope, see mpam_resctrl_monitor_init(). dom = kzalloc_node(sizeof(*dom), GFP_KERNEL, cpu_to_node(cpu)); if (!dom) return ERR_PTR(-ENOMEM); if (r->alloc_capable) { dom->ctrl_comp = ctrl_comp; If the resource is alloc capable this component is used as the domain ctrl_comp. ctrl_d = &dom->resctrl_ctrl_dom; mpam_resctrl_domain_hdr_init(cpu, ctrl_comp, r->rid, &ctrl_d->hdr); ctrl_d->hdr.type = RESCTRL_CTRL_DOMAIN; err = resctrl_online_ctrl_domain(r, ctrl_d); if (err) goto free_domain; mpam_resctrl_domain_insert(&r->ctrl_domains, &ctrl_d->hdr); } else { pr_debug("Skipped control domain online - no controls\n"); } if (r->mon_capable) { struct mpam_component *any_mon_comp = NULL; struct mpam_resctrl_mon *mon; enum resctrl_event_id eventid; /* * Even if the monitor domain is backed by a different * component, the L3 component IDs need to be used... only * there may be no ctrl_comp for the L3. * Search each event's class list for a component with * overlapping CPUs and set up the dom->mon_comp array. */ The MSC at the L3 may only have monitors and so no control component. > struct mpam_resctrl_mon_dom { > struct mpam_component *mon_comp[QOS_NUM_EVENTS]; > struct rdt_l3_mon_domain resctrl_mon_dom; > } > > struct mpam_resctrl_ctrl_dom { > struct mpam_component *ctrl_comp; > struct rdt_ctrl_domain resctrl_ctrl_dom; > }; What you have looks to work for me, with some local cmax, mbw_min, mbw_max additions but with the new layout also works. I gave it a go with this mechanical patch which uses the new layout. Thanks, Ben commit c67c624149474b96e07ccedda11a11ce968e5599 Author: Ben Horgan <ben.horgan@arm.com> Date: Tue Sep 1 17:36:29 2026 +0100 arm_mpam: resctrl: Separate monitor and control domain structure diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h index 3304ef64fcae..855f06657554 100644 --- a/drivers/resctrl/mpam_internal.h +++ b/drivers/resctrl/mpam_internal.h @@ -393,18 +393,14 @@ struct mpam_resctrl_ctrl { struct resctrl_ctrl r_ctrl; }; -struct mpam_resctrl_dom { - struct mpam_component *ctrl_comp; - - /* - * There is no single mon_comp because different events may be backed - * by different class/components. mon_comp is indexed by the event - * number. - */ +struct mpam_resctrl_mon_dom { struct mpam_component *mon_comp[QOS_NUM_EVENTS]; + struct rdt_l3_mon_domain resctrl_mon_dom; +}; +struct mpam_resctrl_ctrl_dom { + struct mpam_component *ctrl_comp; struct rdt_ctrl_domain resctrl_ctrl_dom; - struct rdt_l3_mon_domain resctrl_mon_dom; }; struct mpam_resctrl_res { diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c index faaa390ac5bf..9a198f71bac8 100644 --- a/drivers/resctrl/mpam_resctrl.c +++ b/drivers/resctrl/mpam_resctrl.c @@ -516,7 +516,7 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr, u32 closid, u32 rmid, enum resctrl_event_id eventid, void *arch_priv, u64 *val, void *arch_mon_ctx) { - struct mpam_resctrl_dom *l3_dom; + struct mpam_resctrl_mon_dom *l3_dom; struct mpam_component *mon_comp; u32 mon_idx = *(u32 *)arch_mon_ctx; enum mpam_device_features mon_type; @@ -530,7 +530,7 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr, if (eventid >= QOS_NUM_EVENTS || !mon->class) return -EINVAL; - l3_dom = container_of(hdr, struct mpam_resctrl_dom, resctrl_mon_dom.hdr); + l3_dom = container_of(hdr, struct mpam_resctrl_mon_dom, resctrl_mon_dom.hdr); mon_comp = l3_dom->mon_comp[eventid]; if (eventid != QOS_L3_OCCUP_EVENT_ID) @@ -1316,7 +1316,7 @@ u32 resctrl_arch_get_config(struct rdt_resource *r, struct resctrl_ctrl *ctrl, struct mpam_config *cfg; struct mpam_props *cprops; struct mpam_resctrl_res *res; - struct mpam_resctrl_dom *dom; + struct mpam_resctrl_ctrl_dom *dom; enum mpam_device_features configured_by; lockdep_assert_cpus_held(); @@ -1325,7 +1325,7 @@ u32 resctrl_arch_get_config(struct rdt_resource *r, struct resctrl_ctrl *ctrl, return resctrl_get_default_ctrlval(ctrl); res = container_of(r, struct mpam_resctrl_res, resctrl_res); - dom = container_of(d, struct mpam_resctrl_dom, resctrl_ctrl_dom); + dom = container_of(d, struct mpam_resctrl_ctrl_dom, resctrl_ctrl_dom); cprops = &res->class->props; /* @@ -1399,7 +1399,7 @@ int resctrl_arch_update_one(struct rdt_resource *r, struct resctrl_ctrl *ctrl, struct mpam_config cfg; struct mpam_props *cprops; struct mpam_resctrl_res *res; - struct mpam_resctrl_dom *dom; + struct mpam_resctrl_ctrl_dom *dom; lockdep_assert_cpus_held(); lockdep_assert_irqs_enabled(); @@ -1412,7 +1412,7 @@ int resctrl_arch_update_one(struct rdt_resource *r, struct resctrl_ctrl *ctrl, * resctrl_arch_update_domains() relies on this. */ res = container_of(r, struct mpam_resctrl_res, resctrl_res); - dom = container_of(d, struct mpam_resctrl_dom, resctrl_ctrl_dom); + dom = container_of(d, struct mpam_resctrl_ctrl_dom, resctrl_ctrl_dom); cprops = &res->class->props; if (mpam_resctrl_hide_cdp(r)) @@ -1626,12 +1626,12 @@ static struct mpam_component *find_component(struct mpam_class *class, int cpu) return NULL; } -static struct mpam_resctrl_dom * +static struct mpam_resctrl_ctrl_dom * mpam_resctrl_alloc_ctrl_domain(unsigned int cpu, struct mpam_resctrl_res *res, struct resctrl_ctrl *ctrl) { int err; - struct mpam_resctrl_dom *dom; + struct mpam_resctrl_ctrl_dom *dom; struct rdt_ctrl_domain *ctrl_d; struct mpam_class *class = res->class; struct mpam_component *comp_iter, *ctrl_comp; @@ -1680,11 +1680,11 @@ mpam_resctrl_alloc_ctrl_domain(unsigned int cpu, struct mpam_resctrl_res *res, return dom; } -static struct mpam_resctrl_dom * +static struct mpam_resctrl_mon_dom * mpam_resctrl_alloc_mon_domain(unsigned int cpu, struct mpam_resctrl_res *res) { int err; - struct mpam_resctrl_dom *dom; + struct mpam_resctrl_mon_dom *dom; struct rdt_l3_mon_domain *mon_d; struct mpam_class *class = res->class; struct mpam_component *comp_iter, *ctrl_comp; @@ -1766,11 +1766,11 @@ mpam_resctrl_alloc_mon_domain(unsigned int cpu, struct mpam_resctrl_res *res) * This relies on mpam_resctrl_pick_domain_id() using the L3 cache-id * for anything that is not a cache. */ -static struct mpam_resctrl_dom * +static struct mpam_resctrl_mon_dom * mpam_resctrl_get_mon_domain_from_cpu(int cpu, struct mpam_resctrl_res *res) { int cache_id; - struct mpam_resctrl_dom *dom; + struct mpam_resctrl_mon_dom *dom; struct rdt_resource *r = &res->resctrl_res; lockdep_assert_cpus_held(); @@ -1792,10 +1792,10 @@ mpam_resctrl_get_mon_domain_from_cpu(int cpu, struct mpam_resctrl_res *res) return NULL; } -static struct mpam_resctrl_dom * +static struct mpam_resctrl_ctrl_dom * mpam_resctrl_get_ctrl_domain_from_cpu(int cpu, struct resctrl_ctrl *ctrl) { - struct mpam_resctrl_dom *dom; + struct mpam_resctrl_ctrl_dom *dom; lockdep_assert_cpus_held(); @@ -1814,7 +1814,6 @@ int mpam_resctrl_online_cpu(unsigned int cpu) guard(mutex)(&domain_list_lock); for_each_mpam_resctrl_control(res, rid) { - struct mpam_resctrl_dom *dom; struct rdt_resource *r = &res->resctrl_res; struct resctrl_ctrl *ctrl; @@ -1822,27 +1821,32 @@ int mpam_resctrl_online_cpu(unsigned int cpu) continue; // dummy_resource; if (r->alloc_capable) { + struct mpam_resctrl_ctrl_dom *ctrl_dom; + for_each_resource_ctrl(ctrl, r) { - dom = mpam_resctrl_get_ctrl_domain_from_cpu(cpu, ctrl); - if (!dom) { - dom = mpam_resctrl_alloc_ctrl_domain(cpu, res, ctrl); - if (IS_ERR(dom)) - return PTR_ERR(dom); + ctrl_dom = mpam_resctrl_get_ctrl_domain_from_cpu(cpu, ctrl); + if (!ctrl_dom) { + ctrl_dom = mpam_resctrl_alloc_ctrl_domain(cpu, res, ctrl); + if (IS_ERR(ctrl_dom)) + return PTR_ERR(ctrl_dom); } else { - struct rdt_ctrl_domain *ctrl_d = &dom->resctrl_ctrl_dom; + struct rdt_ctrl_domain *ctrl_d; + ctrl_d = &ctrl_dom->resctrl_ctrl_dom; mpam_resctrl_online_domain_hdr(cpu, &ctrl_d->hdr); } } } if (r->mon_capable) { - dom = mpam_resctrl_get_mon_domain_from_cpu(cpu, res); - if (!dom) { - dom = mpam_resctrl_alloc_mon_domain(cpu, res); - if (IS_ERR(dom)) - return PTR_ERR(dom); + struct mpam_resctrl_mon_dom *mon_dom; + + mon_dom = mpam_resctrl_get_mon_domain_from_cpu(cpu, res); + if (!mon_dom) { + mon_dom = mpam_resctrl_alloc_mon_domain(cpu, res); + if (IS_ERR(mon_dom)) + return PTR_ERR(mon_dom); } else { - struct rdt_l3_mon_domain *mon_d = &dom->resctrl_mon_dom; + struct rdt_l3_mon_domain *mon_d = &mon_dom->resctrl_mon_dom; mpam_resctrl_online_domain_hdr(cpu, &mon_d->hdr); } @@ -1863,7 +1867,6 @@ void mpam_resctrl_offline_cpu(unsigned int cpu) guard(mutex)(&domain_list_lock); for_each_mpam_resctrl_control(res, rid) { - struct mpam_resctrl_dom *dom; struct rdt_l3_mon_domain *mon_d; struct rdt_ctrl_domain *ctrl_d; struct resctrl_ctrl *ctrl; @@ -1874,28 +1877,32 @@ void mpam_resctrl_offline_cpu(unsigned int cpu) continue; // dummy resource if (r->alloc_capable) { + struct mpam_resctrl_ctrl_dom *ctrl_dom; + for_each_resource_ctrl(ctrl, r) { - dom = mpam_resctrl_get_ctrl_domain_from_cpu(cpu, ctrl); - if (WARN_ON_ONCE(!dom)) + ctrl_dom = mpam_resctrl_get_ctrl_domain_from_cpu(cpu, ctrl); + if (WARN_ON_ONCE(!ctrl_dom)) continue; - ctrl_d = &dom->resctrl_ctrl_dom; + ctrl_d = &ctrl_dom->resctrl_ctrl_dom; dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &ctrl_d->hdr); if (dom_empty) { resctrl_offline_ctrl_domain(&res->resctrl_res, ctrl, ctrl_d); - kfree(dom); + kfree(ctrl_dom); } } } if (r->mon_capable) { - dom = mpam_resctrl_get_mon_domain_from_cpu(cpu, res); - if (WARN_ON_ONCE(!dom)) + struct mpam_resctrl_mon_dom *mon_dom; + + mon_dom = mpam_resctrl_get_mon_domain_from_cpu(cpu, res); + if (WARN_ON_ONCE(!mon_dom)) continue; - mon_d = &dom->resctrl_mon_dom; + mon_d = &mon_dom->resctrl_mon_dom; dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &mon_d->hdr); if (dom_empty) { resctrl_offline_mon_domain(&res->resctrl_res, &mon_d->hdr); - kfree(dom); + kfree(mon_dom); } } } > > Any feedback is appreciated. > > Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> > Link: https://lore.kernel.org/lkml/d258a32f-12d5-464d-abe9-4720fb3e44b3@intel.com/ # [1] > Link: https://lore.kernel.org/lkml/20260831172245.42253-1-fenghuay@nvidia.com/ # [2] > --- > - Extracted from "generic schema description" > https://lore.kernel.org/lkml/d258a32f-12d5-464d-abe9-4720fb3e44b3@intel.com/ > > Changes since extract from PoC: > - Rebased on v7.3-rc1 > - Re-wrote changelog > - Return NULL instead of ERR_PTR() when searching for monitoring domain > to ensure the if (!dom) check works. > --- > drivers/resctrl/mpam_resctrl.c | 207 +++++++++++++++++++-------------- > 1 file changed, 121 insertions(+), 86 deletions(-) > > diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c > index 9d223057953a..c888773f3d03 100644 > --- a/drivers/resctrl/mpam_resctrl.c > +++ b/drivers/resctrl/mpam_resctrl.c > @@ -1616,11 +1616,10 @@ static struct mpam_component *find_component(struct mpam_class *class, int cpu) > } > > static struct mpam_resctrl_dom * > -mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res) > +mpam_resctrl_alloc_ctrl_domain(unsigned int cpu, struct mpam_resctrl_res *res) > { > int err; > struct mpam_resctrl_dom *dom; > - struct rdt_l3_mon_domain *mon_d; > struct rdt_ctrl_domain *ctrl_d; > struct mpam_class *class = res->class; > struct mpam_component *comp_iter, *ctrl_comp; > @@ -1628,6 +1627,9 @@ mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res) > > lockdep_assert_held(&domain_list_lock); > > + if (!r->alloc_capable) > + return ERR_PTR(-EINVAL); > + > ctrl_comp = NULL; > guard(srcu)(&mpam_srcu); > list_for_each_entry_srcu(comp_iter, &class->components, class_list, > @@ -1646,70 +1648,97 @@ mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res) > if (!dom) > return ERR_PTR(-ENOMEM); > > - if (r->alloc_capable) { > - dom->ctrl_comp = ctrl_comp; > + dom->ctrl_comp = ctrl_comp; > > - ctrl_d = &dom->resctrl_ctrl_dom; > - mpam_resctrl_domain_hdr_init(cpu, ctrl_comp, r->rid, &ctrl_d->hdr); > - ctrl_d->hdr.type = RESCTRL_CTRL_DOMAIN; > - err = resctrl_online_ctrl_domain(r, ctrl_d); > - if (err) > - goto free_domain; > + ctrl_d = &dom->resctrl_ctrl_dom; > + mpam_resctrl_domain_hdr_init(cpu, ctrl_comp, r->rid, &ctrl_d->hdr); > + ctrl_d->hdr.type = RESCTRL_CTRL_DOMAIN; > + err = resctrl_online_ctrl_domain(r, ctrl_d); > + if (err) > + goto free_domain; > > - mpam_resctrl_domain_insert(&r->ctrl_domains, &ctrl_d->hdr); > - } else { > - pr_debug("Skipped control domain online - no controls\n"); > - } > + mpam_resctrl_domain_insert(&r->ctrl_domains, &ctrl_d->hdr); > > - if (r->mon_capable) { > - struct mpam_component *any_mon_comp = NULL; > - struct mpam_resctrl_mon *mon; > - enum resctrl_event_id eventid; > + return dom; > > - /* > - * Even if the monitor domain is backed by a different > - * component, the L3 component IDs need to be used... only > - * there may be no ctrl_comp for the L3. > - * Search each event's class list for a component with > - * overlapping CPUs and set up the dom->mon_comp array. > - */ > +free_domain: > + kfree(dom); > + dom = ERR_PTR(err); > + > + return dom; > +} > + > +static struct mpam_resctrl_dom * > +mpam_resctrl_alloc_mon_domain(unsigned int cpu, struct mpam_resctrl_res *res) > +{ > + int err; > + struct mpam_resctrl_dom *dom; > + struct rdt_l3_mon_domain *mon_d; > + struct mpam_class *class = res->class; > + struct mpam_component *comp_iter, *ctrl_comp; > + struct rdt_resource *r = &res->resctrl_res; > + struct mpam_component *any_mon_comp = NULL; > + struct mpam_resctrl_mon *mon; > + enum resctrl_event_id eventid; > > - for_each_mpam_resctrl_mon(mon, eventid) { > - struct mpam_component *mon_comp; > + lockdep_assert_held(&domain_list_lock); > > - if (!mon->class) > - continue; // dummy resource > + if (!r->mon_capable) > + return ERR_PTR(-EINVAL); > > - mon_comp = find_component(mon->class, cpu); > - dom->mon_comp[eventid] = mon_comp; > - if (mon_comp) > - any_mon_comp = mon_comp; > - } > - if (!any_mon_comp) { > - WARN_ON_ONCE(0); > - err = -EFAULT; > - goto offline_ctrl_domain; > + ctrl_comp = NULL; > + guard(srcu)(&mpam_srcu); > + list_for_each_entry_srcu(comp_iter, &class->components, class_list, > + srcu_read_lock_held(&mpam_srcu)) { > + if (cpumask_test_cpu(cpu, &comp_iter->affinity)) { > + ctrl_comp = comp_iter; > + break; > } > + } > > - mon_d = &dom->resctrl_mon_dom; > - mpam_resctrl_domain_hdr_init(cpu, any_mon_comp, r->rid, &mon_d->hdr); > - mon_d->hdr.type = RESCTRL_MON_DOMAIN; > - err = resctrl_online_mon_domain(r, &mon_d->hdr); > - if (err) > - goto offline_ctrl_domain; > + /* class has no component for this CPU */ > + if (WARN_ON_ONCE(!ctrl_comp)) > + return ERR_PTR(-EINVAL); > > - mpam_resctrl_domain_insert(&r->mon_domains, &mon_d->hdr); > - } else { > - pr_debug("Skipped monitor domain online - no monitors\n"); > + dom = kzalloc_node(sizeof(*dom), GFP_KERNEL, cpu_to_node(cpu)); > + if (!dom) > + return ERR_PTR(-ENOMEM); > + > + /* > + * Even if the monitor domain is backed by a different > + * component, the L3 component IDs need to be used... only > + * there may be no ctrl_comp for the L3. > + * Search each event's class list for a component with > + * overlapping CPUs and set up the dom->mon_comp array. > + */ > + for_each_mpam_resctrl_mon(mon, eventid) { > + struct mpam_component *mon_comp; > + > + if (!mon->class) > + continue; // dummy resource > + > + mon_comp = find_component(mon->class, cpu); > + dom->mon_comp[eventid] = mon_comp; > + if (mon_comp) > + any_mon_comp = mon_comp; > } > + if (!any_mon_comp) { > + WARN_ON_ONCE(0); > + err = -EFAULT; > + goto free_domain; > + } > + > + mon_d = &dom->resctrl_mon_dom; > + mpam_resctrl_domain_hdr_init(cpu, any_mon_comp, r->rid, &mon_d->hdr); > + mon_d->hdr.type = RESCTRL_MON_DOMAIN; > + err = resctrl_online_mon_domain(r, &mon_d->hdr); > + if (err) > + goto free_domain; > + > + mpam_resctrl_domain_insert(&r->mon_domains, &mon_d->hdr); > > return dom; > > -offline_ctrl_domain: > - if (r->alloc_capable) { > - mpam_resctrl_offline_domain_hdr(cpu, &ctrl_d->hdr); > - resctrl_offline_ctrl_domain(r, ctrl_d); > - } > free_domain: > kfree(dom); > dom = ERR_PTR(err); > @@ -1724,21 +1753,25 @@ mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res) > * This relies on mpam_resctrl_pick_domain_id() using the L3 cache-id > * for anything that is not a cache. > */ > -static struct mpam_resctrl_dom *mpam_resctrl_get_mon_domain_from_cpu(int cpu) > +static struct mpam_resctrl_dom * > +mpam_resctrl_get_mon_domain_from_cpu(int cpu, struct mpam_resctrl_res *res) > { > int cache_id; > struct mpam_resctrl_dom *dom; > - struct mpam_resctrl_res *l3 = &mpam_resctrl_controls[RDT_RESOURCE_L3]; > + struct rdt_resource *r = &res->resctrl_res; > > lockdep_assert_cpus_held(); > > - if (!l3->class) > + if (r->rid != RDT_RESOURCE_L3) > + return NULL; > + > + if (!res->class) > return NULL; > cache_id = get_cpu_cacheinfo_id(cpu, 3); > if (cache_id < 0) > return NULL; > > - list_for_each_entry_rcu(dom, &l3->resctrl_res.mon_domains, resctrl_mon_dom.hdr.list) { > + list_for_each_entry_rcu(dom, &res->resctrl_res.mon_domains, resctrl_mon_dom.hdr.list) { > if (dom->resctrl_mon_dom.hdr.id == cache_id) > return dom; > } > @@ -1747,7 +1780,7 @@ static struct mpam_resctrl_dom *mpam_resctrl_get_mon_domain_from_cpu(int cpu) > } > > static struct mpam_resctrl_dom * > -mpam_resctrl_get_domain_from_cpu(int cpu, struct mpam_resctrl_res *res) > +mpam_resctrl_get_ctrl_domain_from_cpu(int cpu, struct mpam_resctrl_res *res) > { > struct mpam_resctrl_dom *dom; > struct rdt_resource *r = &res->resctrl_res; > @@ -1759,11 +1792,7 @@ mpam_resctrl_get_domain_from_cpu(int cpu, struct mpam_resctrl_res *res) > return dom; > } > > - if (r->rid != RDT_RESOURCE_L3) > - return NULL; > - > - /* Search the mon domain list too - needed on monitor only platforms. */ > - return mpam_resctrl_get_mon_domain_from_cpu(cpu); > + return NULL; > } > > int mpam_resctrl_online_cpu(unsigned int cpu) > @@ -1779,18 +1808,25 @@ int mpam_resctrl_online_cpu(unsigned int cpu) > if (!res->class) > continue; // dummy_resource; > > - dom = mpam_resctrl_get_domain_from_cpu(cpu, res); > - if (!dom) { > - dom = mpam_resctrl_alloc_domain(cpu, res); > - if (IS_ERR(dom)) > - return PTR_ERR(dom); > - } else { > - if (r->alloc_capable) { > + if (r->alloc_capable) { > + dom = mpam_resctrl_get_ctrl_domain_from_cpu(cpu, res); > + if (!dom) { > + dom = mpam_resctrl_alloc_ctrl_domain(cpu, res); > + if (IS_ERR(dom)) > + return PTR_ERR(dom); > + } else { > struct rdt_ctrl_domain *ctrl_d = &dom->resctrl_ctrl_dom; > > mpam_resctrl_online_domain_hdr(cpu, &ctrl_d->hdr); > } > - if (r->mon_capable) { > + } > + if (r->mon_capable) { > + dom = mpam_resctrl_get_mon_domain_from_cpu(cpu, res); > + if (!dom) { > + dom = mpam_resctrl_alloc_mon_domain(cpu, res); > + if (IS_ERR(dom)) > + return PTR_ERR(dom); > + } else { > struct rdt_l3_mon_domain *mon_d = &dom->resctrl_mon_dom; > > mpam_resctrl_online_domain_hdr(cpu, &mon_d->hdr); > @@ -1815,36 +1851,35 @@ void mpam_resctrl_offline_cpu(unsigned int cpu) > struct mpam_resctrl_dom *dom; > struct rdt_l3_mon_domain *mon_d; > struct rdt_ctrl_domain *ctrl_d; > - bool ctrl_dom_empty, mon_dom_empty; > + bool dom_empty; > struct rdt_resource *r = &res->resctrl_res; > > if (!res->class) > continue; // dummy resource > > - dom = mpam_resctrl_get_domain_from_cpu(cpu, res); > - if (WARN_ON_ONCE(!dom)) > - continue; > - > if (r->alloc_capable) { > + dom = mpam_resctrl_get_ctrl_domain_from_cpu(cpu, res); > + if (WARN_ON_ONCE(!dom)) > + continue; > ctrl_d = &dom->resctrl_ctrl_dom; > - ctrl_dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &ctrl_d->hdr); > - if (ctrl_dom_empty) > + dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &ctrl_d->hdr); > + if (dom_empty) { > resctrl_offline_ctrl_domain(&res->resctrl_res, ctrl_d); > - } else { > - ctrl_dom_empty = true; > + kfree(dom); > + } > } > > if (r->mon_capable) { > + dom = mpam_resctrl_get_mon_domain_from_cpu(cpu, res); > + if (WARN_ON_ONCE(!dom)) > + continue; > mon_d = &dom->resctrl_mon_dom; > - mon_dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &mon_d->hdr); > - if (mon_dom_empty) > + dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &mon_d->hdr); > + if (dom_empty) { > resctrl_offline_mon_domain(&res->resctrl_res, &mon_d->hdr); > - } else { > - mon_dom_empty = true; > + kfree(dom); > + } > } > - > - if (ctrl_dom_empty && mon_dom_empty) > - kfree(dom); > } > } > ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains 2026-09-02 16:10 ` Ben Horgan @ 2026-09-03 15:29 ` Reinette Chatre 2026-09-07 17:01 ` Ben Horgan 0 siblings, 1 reply; 10+ messages in thread From: Reinette Chatre @ 2026-09-03 15:29 UTC (permalink / raw) To: Ben Horgan, james.morse, Dave.Martin, fenghuay Cc: tony.luck, babu.moger, yu.c.chen, linux-arm-kernel, linux-kernel, patches Hi Ben, On 9/2/26 9:10 AM, Ben Horgan wrote: > Hi Reinette, > > On 01/09/2026 00:54, Reinette Chatre wrote: >> A single struct mpam_resctrl_dom instance represents both an allocation and >> monitor domain. When a system supports allocation and monitoring a single >> struct mpam_resctrl_dom instance is created and then added to both the >> monitor and control domain lists via >> mpam_resctrl_dom::rdt_ctrl_domain::rdt_domain_hdr::list_head >> and mpam_resctrl_dom::rdt_l3_mon_domain::rdt_domain_hdr::list_head >> respectively. >> >> Representing both allocation and monitoring domains with a single >> architecture domain requires that allocation and monitoring be done at the >> same scope and also requires a resource to have 1:1 support for an allocation >> control and monitoring. The latter means that when a resource supports >> multiple controls for allocation of a resource, for example a "min" bandwidth >> control as well as a "max" bandwidth control for memory bandwidth allocation, >> then each domain supporting each allocation control needs a matching >> monitoring domain. This does not accurately represent the resource >> capabilities. > > For MPAM, the scope of a control is determined by where the MSC are in a system. For the MSC in the > L3 we collect the MSC in a cache instance and call that a component and collect those components and > call them a class. Similarly for other caches. The scope of a control is based on the component it > is in. Hence, we are unable to use all the flexibility that resctrl will offer. Thank you very much for this insight. Although, to me the final sentence is an unexpected jump from the description since in my view resctrl is catching up to what MPAM is capable of :) > > For MSC at the memory we currently only consider them for resctrl if there is only one memory > instance (a single NUMA node) and that this corresponds to the topology of the L3 in the system (i.e > only one L3 instance). This will change when NUMA scope is introduced to resctrl but for now the > domain associated with the MPAM component for MSC at the memory covers all the cpus and in such a > system there is only one L3 domain. The idea being that memory bandwidth controls at the memory are > effectively the same as those at L3 if only if there is only one of each and no other caches in > between. The initial MPAM driver put up for upstream review was more lenient than this and so the > tigtening up ended up leaving some complexity around that is no longer needed. I hope to simplify > this in the future and hopefully allow MPAM component to be used more directly to resctrl domain. For this I think this change is helpful since it makes it clear that a resctrl domain is associated with an MPAM component. > >> >> Split MPAM allocation and monitoring domain management to enable a resource >> to support multiple allocation controls. As an initial and knowingly >> inefficient approach, duplicate struct mpam_resctrl_dom between the >> monitoring and control lists while only initializing the relevant member >> structs. >> >> The goal of this conservative approach is to use something that can only >> be compile tested by me to learn from MPAM folks on what the best approach >> should be for MPAM to support multiple allocation controls. >> >> This patch is extracted from the "Generic schema description" PoC [1] that, >> among its various goals, aim to add support for MPAM's multiple controls >> to resctrl. >> >> This has only been compile tested by me but Fenghua's work to enable >> MPAM's CPU-less NUMA nodes [2] is using it as a baseline. >> >> I did consider a new layout as below but there seems to be a contradiction >> in mpam_resctrl_alloc_domain() where domain addition for allocation as well >> as monitoring domains unconditionally fails if there is no "ctrl_comp" while >> a comment when adding a monitoring domain states "there may be no ctrl_comp >> for the L3". > > Hmmm, confusing, but there is no real contradiction. Just some bad naming. > > Quoting the problem area: > > ctrl_comp = NULL; > guard(srcu)(&mpam_srcu); > list_for_each_entry_srcu(comp_iter, &class->components, class_list, > srcu_read_lock_held(&mpam_srcu)) { > if (cpumask_test_cpu(cpu, &comp_iter->affinity)) { > ctrl_comp = comp_iter; > break; > } > } Staring at this more above looks like an open-code of mpam_resctrl.c:find_component()? > > /* class has no component for this CPU */ > if (WARN_ON_ONCE(!ctrl_comp)) > return ERR_PTR(-EINVAL); > > > In mpam_resctrl_alloc_domain() this initial 'ctrl_comp' check ensures that there is for the class > associated with the given resource, res, a component which includes the given cpu in its affinity Why does the monitoring require that there is a component associated with the resource as opposed to only relying on the component associated with the the monitoring class, specifically the mpam_resctrl_mon::class? (more below) > mask. The class is all the L3 MSC or an equivalent of the same scope, see mpam_resctrl_monitor_init(). > > dom = kzalloc_node(sizeof(*dom), GFP_KERNEL, cpu_to_node(cpu)); > if (!dom) > return ERR_PTR(-ENOMEM); > > if (r->alloc_capable) { > dom->ctrl_comp = ctrl_comp; > > If the resource is alloc capable this component is used as the domain ctrl_comp. > > ctrl_d = &dom->resctrl_ctrl_dom; > mpam_resctrl_domain_hdr_init(cpu, ctrl_comp, r->rid, &ctrl_d->hdr); > ctrl_d->hdr.type = RESCTRL_CTRL_DOMAIN; > err = resctrl_online_ctrl_domain(r, ctrl_d); > if (err) > goto free_domain; > > mpam_resctrl_domain_insert(&r->ctrl_domains, &ctrl_d->hdr); > } else { > pr_debug("Skipped control domain online - no controls\n"); > } > > if (r->mon_capable) { > struct mpam_component *any_mon_comp = NULL; > struct mpam_resctrl_mon *mon; > enum resctrl_event_id eventid; > > /* > * Even if the monitor domain is backed by a different > * component, the L3 component IDs need to be used... only > * there may be no ctrl_comp for the L3. > * Search each event's class list for a component with > * overlapping CPUs and set up the dom->mon_comp array. > */ > The MSC at the L3 may only have monitors and so no control component. The code that follows is: for_each_mpam_resctrl_mon(mon, eventid) { struct mpam_component *mon_comp; if (!mon->class) continue; // dummy resource mon_comp = find_component(mon->class, cpu); Is this find_component() perhaps sufficient by itself (without the earlier "ctrl_comp" check) to determine if there is a valid component associated with this CPU to support this monitoring feature? Although, as written it seems that it is ok for mon_comp to be NULL? It is not clear to me if a mon_comp of NULL is able to handle all scenarios since it looks like mpam_resctrl_get_mon_domain_from_cpu() and mpam_resctrl_online_domain_hdr() does not consider the component at all. Would that not cause monitoring features to depend on which CPU of a domain comes online first? Could mon_comp perhaps be required to be !NULL here as a replacement for the earlier "ctrl_comp" check to ensure there is a component with the CPU in its affinity mask? dom->mon_comp[eventid] = mon_comp; if (mon_comp) any_mon_comp = mon_comp; } > > > > > >> struct mpam_resctrl_mon_dom { >> struct mpam_component *mon_comp[QOS_NUM_EVENTS]; >> struct rdt_l3_mon_domain resctrl_mon_dom; >> } >> >> struct mpam_resctrl_ctrl_dom { >> struct mpam_component *ctrl_comp; >> struct rdt_ctrl_domain resctrl_ctrl_dom; >> }; > > What you have looks to work for me, with some local cmax, mbw_min, mbw_max additions but with the > new layout also works. I gave it a go with this mechanical patch which uses the new layout. > > Thanks, > > Ben > > commit c67c624149474b96e07ccedda11a11ce968e5599 > Author: Ben Horgan <ben.horgan@arm.com> > Date: Tue Sep 1 17:36:29 2026 +0100 > > arm_mpam: resctrl: Separate monitor and control domain structure > > diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h > index 3304ef64fcae..855f06657554 100644 > --- a/drivers/resctrl/mpam_internal.h > +++ b/drivers/resctrl/mpam_internal.h > @@ -393,18 +393,14 @@ struct mpam_resctrl_ctrl { > struct resctrl_ctrl r_ctrl; > }; > > -struct mpam_resctrl_dom { > - struct mpam_component *ctrl_comp; > - > - /* > - * There is no single mon_comp because different events may be backed > - * by different class/components. mon_comp is indexed by the event > - * number. > - */ > +struct mpam_resctrl_mon_dom { > struct mpam_component *mon_comp[QOS_NUM_EVENTS]; > + struct rdt_l3_mon_domain resctrl_mon_dom; > +}; > > +struct mpam_resctrl_ctrl_dom { > + struct mpam_component *ctrl_comp; > struct rdt_ctrl_domain resctrl_ctrl_dom; > - struct rdt_l3_mon_domain resctrl_mon_dom; > }; > Thank you very much for trying this out. I find this layout better since the architecture domain structure only contains those members related to the domain. I see your snippet is based on the PoC, would you prefer I incorporate it into a new version of the PoC to get some more testing or to create a new version based on current upstream so that we can work on its upstream inclusion for the multiple controller support to build on? Thank you Reinette ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains 2026-09-03 15:29 ` Reinette Chatre @ 2026-09-07 17:01 ` Ben Horgan 2026-09-08 21:12 ` Reinette Chatre 0 siblings, 1 reply; 10+ messages in thread From: Ben Horgan @ 2026-09-07 17:01 UTC (permalink / raw) To: Reinette Chatre, james.morse, Dave.Martin, fenghuay Cc: tony.luck, babu.moger, yu.c.chen, linux-arm-kernel, linux-kernel, patches Hi Reinette, On 03/09/2026 16:29, Reinette Chatre wrote: > Hi Ben, > > On 9/2/26 9:10 AM, Ben Horgan wrote: >> Hi Reinette, >> >> On 01/09/2026 00:54, Reinette Chatre wrote: >>> A single struct mpam_resctrl_dom instance represents both an allocation and >>> monitor domain. When a system supports allocation and monitoring a single >>> struct mpam_resctrl_dom instance is created and then added to both the >>> monitor and control domain lists via >>> mpam_resctrl_dom::rdt_ctrl_domain::rdt_domain_hdr::list_head >>> and mpam_resctrl_dom::rdt_l3_mon_domain::rdt_domain_hdr::list_head >>> respectively. >>> >>> Representing both allocation and monitoring domains with a single >>> architecture domain requires that allocation and monitoring be done at the >>> same scope and also requires a resource to have 1:1 support for an allocation >>> control and monitoring. The latter means that when a resource supports >>> multiple controls for allocation of a resource, for example a "min" bandwidth >>> control as well as a "max" bandwidth control for memory bandwidth allocation, >>> then each domain supporting each allocation control needs a matching >>> monitoring domain. This does not accurately represent the resource >>> capabilities. >> >> For MPAM, the scope of a control is determined by where the MSC are in a system. For the MSC in the >> L3 we collect the MSC in a cache instance and call that a component and collect those components and >> call them a class. Similarly for other caches. The scope of a control is based on the component it >> is in. Hence, we are unable to use all the flexibility that resctrl will offer. > > Thank you very much for this insight. Although, to me the final sentence is an > unexpected jump from the description since in my view resctrl is catching up to > what MPAM is capable of :) > >> >> For MSC at the memory we currently only consider them for resctrl if there is only one memory >> instance (a single NUMA node) and that this corresponds to the topology of the L3 in the system (i.e >> only one L3 instance). This will change when NUMA scope is introduced to resctrl but for now the >> domain associated with the MPAM component for MSC at the memory covers all the cpus and in such a >> system there is only one L3 domain. The idea being that memory bandwidth controls at the memory are >> effectively the same as those at L3 if only if there is only one of each and no other caches in >> between. The initial MPAM driver put up for upstream review was more lenient than this and so the >> tigtening up ended up leaving some complexity around that is no longer needed. I hope to simplify >> this in the future and hopefully allow MPAM component to be used more directly to resctrl domain. > > For this I think this change is helpful since it makes it clear that a resctrl > domain is associated with an MPAM component. > >> >>> >>> Split MPAM allocation and monitoring domain management to enable a resource >>> to support multiple allocation controls. As an initial and knowingly >>> inefficient approach, duplicate struct mpam_resctrl_dom between the >>> monitoring and control lists while only initializing the relevant member >>> structs. >>> >>> The goal of this conservative approach is to use something that can only >>> be compile tested by me to learn from MPAM folks on what the best approach >>> should be for MPAM to support multiple allocation controls. >>> >>> This patch is extracted from the "Generic schema description" PoC [1] that, >>> among its various goals, aim to add support for MPAM's multiple controls >>> to resctrl. >>> >>> This has only been compile tested by me but Fenghua's work to enable >>> MPAM's CPU-less NUMA nodes [2] is using it as a baseline. >>> >>> I did consider a new layout as below but there seems to be a contradiction >>> in mpam_resctrl_alloc_domain() where domain addition for allocation as well >>> as monitoring domains unconditionally fails if there is no "ctrl_comp" while >>> a comment when adding a monitoring domain states "there may be no ctrl_comp >>> for the L3". >> >> Hmmm, confusing, but there is no real contradiction. Just some bad naming. >> >> Quoting the problem area: >> >> ctrl_comp = NULL; >> guard(srcu)(&mpam_srcu); >> list_for_each_entry_srcu(comp_iter, &class->components, class_list, >> srcu_read_lock_held(&mpam_srcu)) { >> if (cpumask_test_cpu(cpu, &comp_iter->affinity)) { >> ctrl_comp = comp_iter; >> break; >> } >> } > > Staring at this more above looks like an open-code of mpam_resctrl.c:find_component()? Yes, I should clean that up. > >> >> /* class has no component for this CPU */ >> if (WARN_ON_ONCE(!ctrl_comp)) >> return ERR_PTR(-EINVAL); >> >> >> In mpam_resctrl_alloc_domain() this initial 'ctrl_comp' check ensures that there is for the class >> associated with the given resource, res, a component which includes the given cpu in its affinity > > Why does the monitoring require that there is a component associated with the resource as > opposed to only relying on the component associated with the the monitoring class, specifically > the mpam_resctrl_mon::class? (more below) The class associated with a resource does double duty and is also used as an indication of whether the resource is in used at all, see also mpam_resctrl_online_cpu() and mpam_resctrl_offline_cpu(). When a resource only supports monitoring then the class providing the monitors is used for res->class. As such, a check that there is a component for the given cpu, in the monitor only resource case, is just a way to error out early when the monitoring class has no cpu. We shouldn't hit this though as all monitoring classes are check that they have the same topology of the l3, cover all cpus and there components correspond to l3 instances. The resource supports monitoring but not controls case only occurs when there are no l3 controls and the condidtions for pretending the memory bandwidth counters at the memory are at the l3 are met. Those conditions for pretending occur when there is single l3, a single NUMA node and no intermediate caches. > > >> mask. The class is all the L3 MSC or an equivalent of the same scope, see mpam_resctrl_monitor_init(). >> >> dom = kzalloc_node(sizeof(*dom), GFP_KERNEL, cpu_to_node(cpu)); >> if (!dom) >> return ERR_PTR(-ENOMEM); >> >> if (r->alloc_capable) { >> dom->ctrl_comp = ctrl_comp; >> >> If the resource is alloc capable this component is used as the domain ctrl_comp. >> >> ctrl_d = &dom->resctrl_ctrl_dom; >> mpam_resctrl_domain_hdr_init(cpu, ctrl_comp, r->rid, &ctrl_d->hdr); >> ctrl_d->hdr.type = RESCTRL_CTRL_DOMAIN; >> err = resctrl_online_ctrl_domain(r, ctrl_d); >> if (err) >> goto free_domain; >> >> mpam_resctrl_domain_insert(&r->ctrl_domains, &ctrl_d->hdr); >> } else { >> pr_debug("Skipped control domain online - no controls\n"); >> } >> >> if (r->mon_capable) { >> struct mpam_component *any_mon_comp = NULL; >> struct mpam_resctrl_mon *mon; >> enum resctrl_event_id eventid; >> >> /* >> * Even if the monitor domain is backed by a different >> * component, the L3 component IDs need to be used... only >> * there may be no ctrl_comp for the L3. >> * Search each event's class list for a component with >> * overlapping CPUs and set up the dom->mon_comp array. >> */ >> The MSC at the L3 may only have monitors and so no control component. > > The code that follows is: > > for_each_mpam_resctrl_mon(mon, eventid) { > struct mpam_component *mon_comp; > > if (!mon->class) > continue; // dummy resource > > mon_comp = find_component(mon->class, cpu); > > Is this find_component() perhaps sufficient by itself (without the earlier "ctrl_comp" check) > to determine if there is a valid component associated with this CPU to support this > monitoring feature? Yes, I think is the "ctrl_comp" check is taken away but would have failed we will end up not finding any relevant monitoring components. Although, as written it seems that it is ok for mon_comp to be NULL? Isn't any_mon_comp the relevant thing? > > It is not clear to me if a mon_comp of NULL is able to handle all scenarios since it looks > like mpam_resctrl_get_mon_domain_from_cpu() and mpam_resctrl_online_domain_hdr() does not > consider the component at all. Would that not cause monitoring features to depend on which > CPU of a domain comes online first? > > Could mon_comp perhaps be required to be !NULL here as a replacement for the earlier > "ctrl_comp" check to ensure there is a component with the CPU in its affinity mask? Doesn't the !any_mon_comp check provide this? > > dom->mon_comp[eventid] = mon_comp; > if (mon_comp) > any_mon_comp = mon_comp; > } > > >> >> >> >> >> >>> struct mpam_resctrl_mon_dom { >>> struct mpam_component *mon_comp[QOS_NUM_EVENTS]; >>> struct rdt_l3_mon_domain resctrl_mon_dom; >>> } >>> >>> struct mpam_resctrl_ctrl_dom { >>> struct mpam_component *ctrl_comp; >>> struct rdt_ctrl_domain resctrl_ctrl_dom; >>> }; >> >> What you have looks to work for me, with some local cmax, mbw_min, mbw_max additions but with the >> new layout also works. I gave it a go with this mechanical patch which uses the new layout. >> >> Thanks, >> >> Ben >> >> commit c67c624149474b96e07ccedda11a11ce968e5599 >> Author: Ben Horgan <ben.horgan@arm.com> >> Date: Tue Sep 1 17:36:29 2026 +0100 >> >> arm_mpam: resctrl: Separate monitor and control domain structure >> >> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h >> index 3304ef64fcae..855f06657554 100644 >> --- a/drivers/resctrl/mpam_internal.h >> +++ b/drivers/resctrl/mpam_internal.h >> @@ -393,18 +393,14 @@ struct mpam_resctrl_ctrl { >> struct resctrl_ctrl r_ctrl; >> }; >> >> -struct mpam_resctrl_dom { >> - struct mpam_component *ctrl_comp; >> - >> - /* >> - * There is no single mon_comp because different events may be backed >> - * by different class/components. mon_comp is indexed by the event >> - * number. >> - */ >> +struct mpam_resctrl_mon_dom { >> struct mpam_component *mon_comp[QOS_NUM_EVENTS]; >> + struct rdt_l3_mon_domain resctrl_mon_dom; >> +}; >> >> +struct mpam_resctrl_ctrl_dom { >> + struct mpam_component *ctrl_comp; >> struct rdt_ctrl_domain resctrl_ctrl_dom; >> - struct rdt_l3_mon_domain resctrl_mon_dom; >> }; >> > > Thank you very much for trying this out. I find this layout better since the > architecture domain structure only contains those members related to the domain. > I see your snippet is based on the PoC, would you prefer I incorporate it into a new > version of the PoC to get some more testing or to create a new version based on > current upstream so that we can work on its upstream inclusion for the multiple > controller support to build on? In the spirit of getting the precursors for your PoC upstream I think it would make sense to work on this for upstream. Whatever you think is best for progressing multiple control support is ok with me though. Thanks, Ben > > Thank you > > Reinette ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains 2026-09-07 17:01 ` Ben Horgan @ 2026-09-08 21:12 ` Reinette Chatre 2026-09-10 11:11 ` Ben Horgan 0 siblings, 1 reply; 10+ messages in thread From: Reinette Chatre @ 2026-09-08 21:12 UTC (permalink / raw) To: Ben Horgan, james.morse, Dave.Martin, fenghuay Cc: tony.luck, babu.moger, yu.c.chen, linux-arm-kernel, linux-kernel, patches Hi Ben, On 9/7/26 10:01 AM, Ben Horgan wrote: > On 03/09/2026 16:29, Reinette Chatre wrote: >> On 9/2/26 9:10 AM, Ben Horgan wrote: >>> On 01/09/2026 00:54, Reinette Chatre wrote: ... >>> >>> /* class has no component for this CPU */ >>> if (WARN_ON_ONCE(!ctrl_comp)) >>> return ERR_PTR(-EINVAL); >>> >>> >>> In mpam_resctrl_alloc_domain() this initial 'ctrl_comp' check ensures that there is for the class >>> associated with the given resource, res, a component which includes the given cpu in its affinity >> >> Why does the monitoring require that there is a component associated with the resource as >> opposed to only relying on the component associated with the the monitoring class, specifically >> the mpam_resctrl_mon::class? (more below) > > The class associated with a resource does double duty and is also used as an indication of whether > the resource is in used at all, see also mpam_resctrl_online_cpu() and mpam_resctrl_offline_cpu(). > When a resource only supports monitoring then the class providing the monitors is used for > res->class. As such, a check that there is a component for the given cpu, in the monitor only Regarding "class providing the monitors": From what I can tell each monitor event is separately mapped to a (potentially?) different MPAM class. With MPAM supporting two events, each associated with a class, which of these classes are used for the class associated with the resource? > resource case, is just a way to error out early when the monitoring class has no cpu. We shouldn't > hit this though as all monitoring classes are check that they have the same topology of the l3, > cover all cpus and there components correspond to l3 instances. It seems as though there is a requirement that all the monitoring classes as well as the control class cover the same CPUs with the implementation flexible to support otherwise while assuming it does? > > The resource supports monitoring but not controls case only occurs when there are no l3 controls and > the condidtions for pretending the memory bandwidth counters at the memory are at the l3 are met. > Those conditions for pretending occur when there is single l3, a single NUMA node and no > intermediate caches. > >> >> >>> mask. The class is all the L3 MSC or an equivalent of the same scope, see mpam_resctrl_monitor_init(). >>> >>> dom = kzalloc_node(sizeof(*dom), GFP_KERNEL, cpu_to_node(cpu)); >>> if (!dom) >>> return ERR_PTR(-ENOMEM); >>> >>> if (r->alloc_capable) { >>> dom->ctrl_comp = ctrl_comp; >>> >>> If the resource is alloc capable this component is used as the domain ctrl_comp. >>> >>> ctrl_d = &dom->resctrl_ctrl_dom; >>> mpam_resctrl_domain_hdr_init(cpu, ctrl_comp, r->rid, &ctrl_d->hdr); >>> ctrl_d->hdr.type = RESCTRL_CTRL_DOMAIN; >>> err = resctrl_online_ctrl_domain(r, ctrl_d); >>> if (err) >>> goto free_domain; >>> >>> mpam_resctrl_domain_insert(&r->ctrl_domains, &ctrl_d->hdr); >>> } else { >>> pr_debug("Skipped control domain online - no controls\n"); >>> } >>> >>> if (r->mon_capable) { >>> struct mpam_component *any_mon_comp = NULL; >>> struct mpam_resctrl_mon *mon; >>> enum resctrl_event_id eventid; >>> >>> /* >>> * Even if the monitor domain is backed by a different >>> * component, the L3 component IDs need to be used... only >>> * there may be no ctrl_comp for the L3. >>> * Search each event's class list for a component with >>> * overlapping CPUs and set up the dom->mon_comp array. >>> */ >>> The MSC at the L3 may only have monitors and so no control component. >> >> The code that follows is: >> >> for_each_mpam_resctrl_mon(mon, eventid) { >> struct mpam_component *mon_comp; >> >> if (!mon->class) >> continue; // dummy resource >> >> mon_comp = find_component(mon->class, cpu); >> >> Is this find_component() perhaps sufficient by itself (without the earlier "ctrl_comp" check) >> to determine if there is a valid component associated with this CPU to support this >> monitoring feature? > > Yes, I think is the "ctrl_comp" check is taken away but would have failed we will end up not finding > any relevant monitoring components. > > Although, as written it seems that it is ok for mon_comp to be NULL? Yes. Are there scenarios under which it will be NULL? If it can be NULL, does it not mean that the event that was already enabled during init cannot actually be supported in all domains/components? > > Isn't any_mon_comp the relevant thing? (answer below) > >> >> It is not clear to me if a mon_comp of NULL is able to handle all scenarios since it looks >> like mpam_resctrl_get_mon_domain_from_cpu() and mpam_resctrl_online_domain_hdr() does not >> consider the component at all. Would that not cause monitoring features to depend on which >> CPU of a domain comes online first? >> >> Could mon_comp perhaps be required to be !NULL here as a replacement for the earlier >> "ctrl_comp" check to ensure there is a component with the CPU in its affinity mask? > > Doesn't the !any_mon_comp check provide this? This is the part that I do not understand since any_mon_comp seems to support the scenario where a mon_comp may be NULL which is a scenario that I do not think resctrl can support. At a high level there seems to be three affinity masks used by the monitoring code: the CPU affinity of the component belonging to the control resource class, the CPU affinity of each component supporting each monitoring event, while these are three separate masks with code sometimes treating them as though they can be different they are actually required to be the same? > >> >> dom->mon_comp[eventid] = mon_comp; >> if (mon_comp) >> any_mon_comp = mon_comp; >> } >>> >>> >>> >>> >>> >>> >>>> struct mpam_resctrl_mon_dom { >>>> struct mpam_component *mon_comp[QOS_NUM_EVENTS]; >>>> struct rdt_l3_mon_domain resctrl_mon_dom; >>>> } >>>> >>>> struct mpam_resctrl_ctrl_dom { >>>> struct mpam_component *ctrl_comp; >>>> struct rdt_ctrl_domain resctrl_ctrl_dom; >>>> }; >>> >>> What you have looks to work for me, with some local cmax, mbw_min, mbw_max additions but with the >>> new layout also works. I gave it a go with this mechanical patch which uses the new layout. >>> >>> Thanks, >>> >>> Ben >>> >>> commit c67c624149474b96e07ccedda11a11ce968e5599 >>> Author: Ben Horgan <ben.horgan@arm.com> >>> Date: Tue Sep 1 17:36:29 2026 +0100 >>> >>> arm_mpam: resctrl: Separate monitor and control domain structure >>> >>> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h >>> index 3304ef64fcae..855f06657554 100644 >>> --- a/drivers/resctrl/mpam_internal.h >>> +++ b/drivers/resctrl/mpam_internal.h >>> @@ -393,18 +393,14 @@ struct mpam_resctrl_ctrl { >>> struct resctrl_ctrl r_ctrl; >>> }; >>> >>> -struct mpam_resctrl_dom { >>> - struct mpam_component *ctrl_comp; >>> - >>> - /* >>> - * There is no single mon_comp because different events may be backed >>> - * by different class/components. mon_comp is indexed by the event >>> - * number. >>> - */ >>> +struct mpam_resctrl_mon_dom { >>> struct mpam_component *mon_comp[QOS_NUM_EVENTS]; >>> + struct rdt_l3_mon_domain resctrl_mon_dom; >>> +}; >>> >>> +struct mpam_resctrl_ctrl_dom { >>> + struct mpam_component *ctrl_comp; >>> struct rdt_ctrl_domain resctrl_ctrl_dom; >>> - struct rdt_l3_mon_domain resctrl_mon_dom; >>> }; >>> >> >> Thank you very much for trying this out. I find this layout better since the >> architecture domain structure only contains those members related to the domain. >> I see your snippet is based on the PoC, would you prefer I incorporate it into a new >> version of the PoC to get some more testing or to create a new version based on >> current upstream so that we can work on its upstream inclusion for the multiple >> controller support to build on? > > In the spirit of getting the precursors for your PoC upstream I think it would make sense to work on > this for upstream. Whatever you think is best for progressing multiple control support is ok with me > though. I think that it will support the multiple control work if this is done upstream first. I will incorporate your changes in the next version ... I believe that would make you a co-author? Thank you very much. Reinette ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains 2026-09-08 21:12 ` Reinette Chatre @ 2026-09-10 11:11 ` Ben Horgan 2026-09-10 15:37 ` Reinette Chatre 0 siblings, 1 reply; 10+ messages in thread From: Ben Horgan @ 2026-09-10 11:11 UTC (permalink / raw) To: Reinette Chatre, james.morse, Dave.Martin, fenghuay Cc: tony.luck, babu.moger, yu.c.chen, linux-arm-kernel, linux-kernel, patches Hi Reinette, On 08/09/2026 22:12, Reinette Chatre wrote: > Hi Ben, > > On 9/7/26 10:01 AM, Ben Horgan wrote: >> On 03/09/2026 16:29, Reinette Chatre wrote: >>> On 9/2/26 9:10 AM, Ben Horgan wrote: >>>> On 01/09/2026 00:54, Reinette Chatre wrote: > > ... > >>>> >>>> /* class has no component for this CPU */ >>>> if (WARN_ON_ONCE(!ctrl_comp)) >>>> return ERR_PTR(-EINVAL); >>>> >>>> >>>> In mpam_resctrl_alloc_domain() this initial 'ctrl_comp' check ensures that there is for the class >>>> associated with the given resource, res, a component which includes the given cpu in its affinity >>> >>> Why does the monitoring require that there is a component associated with the resource as >>> opposed to only relying on the component associated with the the monitoring class, specifically >>> the mpam_resctrl_mon::class? (more below) >> >> The class associated with a resource does double duty and is also used as an indication of whether >> the resource is in used at all, see also mpam_resctrl_online_cpu() and mpam_resctrl_offline_cpu(). >> When a resource only supports monitoring then the class providing the monitors is used for >> res->class. As such, a check that there is a component for the given cpu, in the monitor only > > Regarding "class providing the monitors": From what I can tell each monitor event is separately > mapped to a (potentially?) different MPAM class. With MPAM supporting two events, each associated > with a class, which of these classes are used for the class associated with the resource? If the l3 resource has controls as chosen by mpam_resctrl_pick_cache/mba() then that control class is used. Otherwise, it's just the class for the event in the first call of by mpam_resctrl_monitor_init() which is is the lowest eventid as it's in the iteration in mpam_resctrl_setup(). If there are two events they must be QOS_L3_OCCUP_EVENT_ID and QOS_L3_MBM_LOCAL_EVENT_ID and so by the order of iteration it's QOS_L3_OCCUP_EVENT_ID. > >> resource case, is just a way to error out early when the monitoring class has no cpu. We shouldn't >> hit this though as all monitoring classes are check that they have the same topology of the l3, >> cover all cpus and there components correspond to l3 instances. > > It seems as though there is a requirement that all the monitoring classes as well as the control > class cover the same CPUs with the implementation flexible to support otherwise while assuming it does? Yes... Some parts of the driver are overly flexible and so overly complicated. I hope to find some time to make this cleaner. > >> >> The resource supports monitoring but not controls case only occurs when there are no l3 controls and >> the condidtions for pretending the memory bandwidth counters at the memory are at the l3 are met. >> Those conditions for pretending occur when there is single l3, a single NUMA node and no >> intermediate caches. >> >>> >>> >>>> mask. The class is all the L3 MSC or an equivalent of the same scope, see mpam_resctrl_monitor_init(). >>>> >>>> dom = kzalloc_node(sizeof(*dom), GFP_KERNEL, cpu_to_node(cpu)); >>>> if (!dom) >>>> return ERR_PTR(-ENOMEM); >>>> >>>> if (r->alloc_capable) { >>>> dom->ctrl_comp = ctrl_comp; >>>> >>>> If the resource is alloc capable this component is used as the domain ctrl_comp. >>>> >>>> ctrl_d = &dom->resctrl_ctrl_dom; >>>> mpam_resctrl_domain_hdr_init(cpu, ctrl_comp, r->rid, &ctrl_d->hdr); >>>> ctrl_d->hdr.type = RESCTRL_CTRL_DOMAIN; >>>> err = resctrl_online_ctrl_domain(r, ctrl_d); >>>> if (err) >>>> goto free_domain; >>>> >>>> mpam_resctrl_domain_insert(&r->ctrl_domains, &ctrl_d->hdr); >>>> } else { >>>> pr_debug("Skipped control domain online - no controls\n"); >>>> } >>>> >>>> if (r->mon_capable) { >>>> struct mpam_component *any_mon_comp = NULL; >>>> struct mpam_resctrl_mon *mon; >>>> enum resctrl_event_id eventid; >>>> >>>> /* >>>> * Even if the monitor domain is backed by a different >>>> * component, the L3 component IDs need to be used... only >>>> * there may be no ctrl_comp for the L3. >>>> * Search each event's class list for a component with >>>> * overlapping CPUs and set up the dom->mon_comp array. >>>> */ >>>> The MSC at the L3 may only have monitors and so no control component. >>> >>> The code that follows is: >>> >>> for_each_mpam_resctrl_mon(mon, eventid) { >>> struct mpam_component *mon_comp; >>> >>> if (!mon->class) >>> continue; // dummy resource >>> >>> mon_comp = find_component(mon->class, cpu); >>> >>> Is this find_component() perhaps sufficient by itself (without the earlier "ctrl_comp" check) >>> to determine if there is a valid component associated with this CPU to support this >>> monitoring feature? >> >> Yes, I think is the "ctrl_comp" check is taken away but would have failed we will end up not finding >> any relevant monitoring components. >> >> Although, as written it seems that it is ok for mon_comp to be NULL? > > Yes. Are there scenarios under which it will be NULL? If it can be NULL, does it not > mean that the event that was already enabled during init cannot actually be supported in all > domains/components? > >> >> Isn't any_mon_comp the relevant thing? > > (answer below) > >> >>> >>> It is not clear to me if a mon_comp of NULL is able to handle all scenarios since it looks >>> like mpam_resctrl_get_mon_domain_from_cpu() and mpam_resctrl_online_domain_hdr() does not >>> consider the component at all. Would that not cause monitoring features to depend on which >>> CPU of a domain comes online first? >>> >>> Could mon_comp perhaps be required to be !NULL here as a replacement for the earlier >>> "ctrl_comp" check to ensure there is a component with the CPU in its affinity mask? >> >> Doesn't the !any_mon_comp check provide this? > > This is the part that I do not understand since any_mon_comp seems to support the scenario > where a mon_comp may be NULL which is a scenario that I do not think resctrl can support. Ah, I see what you are getting at. As the monitor components are only considered when there topology matches the l3 cache (same cpu affinity for each instance) then the find_component() call will never fail and so mon_comp can't be NULL at this point. > > At a high level there seems to be three affinity masks used by the monitoring code: > the CPU affinity of the component belonging to the control resource class, the CPU affinity > of each component supporting each monitoring event, while these are three separate masks with > code sometimes treating them as though they can be different they are actually required to be the same? Monitor component CPU affinity is enforced by topology_matches_l3() to be the same as the L3. Additionally, traffic_matches_l3() adds extra conditions that mean that there can be considered an uninterrupted link between l3 and memory and so an MSC at either end is effectively the same. Namely the same restrictions I've mentioned before, only a single l3 cache, a single NUMA node and no intermediate caches. You list two rather than three here? Possibly you are also thinking the MSC cpu affinity which can be different from that of the components as it indicates which cpus the MSC is reliably accessible from. This may be a larger set of cpus than those affine to the component it controls, e.g. every CPU if the MSC is always on and it's register interface can be reached by all the cpus. > >> >>> >>> dom->mon_comp[eventid] = mon_comp; >>> if (mon_comp) >>> any_mon_comp = mon_comp; >>> } >>>> >>>> >>>> >>>> >>>> >>>> >>>>> struct mpam_resctrl_mon_dom { >>>>> struct mpam_component *mon_comp[QOS_NUM_EVENTS]; >>>>> struct rdt_l3_mon_domain resctrl_mon_dom; >>>>> } >>>>> >>>>> struct mpam_resctrl_ctrl_dom { >>>>> struct mpam_component *ctrl_comp; >>>>> struct rdt_ctrl_domain resctrl_ctrl_dom; >>>>> }; >>>> >>>> What you have looks to work for me, with some local cmax, mbw_min, mbw_max additions but with the >>>> new layout also works. I gave it a go with this mechanical patch which uses the new layout. >>>> >>>> Thanks, >>>> >>>> Ben >>>> >>>> commit c67c624149474b96e07ccedda11a11ce968e5599 >>>> Author: Ben Horgan <ben.horgan@arm.com> >>>> Date: Tue Sep 1 17:36:29 2026 +0100 >>>> >>>> arm_mpam: resctrl: Separate monitor and control domain structure >>>> >>>> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h >>>> index 3304ef64fcae..855f06657554 100644 >>>> --- a/drivers/resctrl/mpam_internal.h >>>> +++ b/drivers/resctrl/mpam_internal.h >>>> @@ -393,18 +393,14 @@ struct mpam_resctrl_ctrl { >>>> struct resctrl_ctrl r_ctrl; >>>> }; >>>> >>>> -struct mpam_resctrl_dom { >>>> - struct mpam_component *ctrl_comp; >>>> - >>>> - /* >>>> - * There is no single mon_comp because different events may be backed >>>> - * by different class/components. mon_comp is indexed by the event >>>> - * number. >>>> - */ >>>> +struct mpam_resctrl_mon_dom { >>>> struct mpam_component *mon_comp[QOS_NUM_EVENTS]; >>>> + struct rdt_l3_mon_domain resctrl_mon_dom; >>>> +}; >>>> >>>> +struct mpam_resctrl_ctrl_dom { >>>> + struct mpam_component *ctrl_comp; >>>> struct rdt_ctrl_domain resctrl_ctrl_dom; >>>> - struct rdt_l3_mon_domain resctrl_mon_dom; >>>> }; >>>> >>> >>> Thank you very much for trying this out. I find this layout better since the >>> architecture domain structure only contains those members related to the domain. >>> I see your snippet is based on the PoC, would you prefer I incorporate it into a new >>> version of the PoC to get some more testing or to create a new version based on >>> current upstream so that we can work on its upstream inclusion for the multiple >>> controller support to build on? >> >> In the spirit of getting the precursors for your PoC upstream I think it would make sense to work on >> this for upstream. Whatever you think is best for progressing multiple control support is ok with me >> though. > > I think that it will support the multiple control work if this is done upstream first. > I will incorporate your changes in the next version ... I believe that would make you > a co-author? I don't think co-author is necessary as I've just offered review and you mentioned you'd already tried out the new structures. Up to you though. Thanks for helping with MPAM, Ben > > Thank you very much. > > Reinette ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains 2026-09-10 11:11 ` Ben Horgan @ 2026-09-10 15:37 ` Reinette Chatre 2026-09-10 16:28 ` Ben Horgan 0 siblings, 1 reply; 10+ messages in thread From: Reinette Chatre @ 2026-09-10 15:37 UTC (permalink / raw) To: Ben Horgan, james.morse, Dave.Martin, fenghuay Cc: tony.luck, babu.moger, yu.c.chen, linux-arm-kernel, linux-kernel, patches Hi Ben, On 9/10/26 4:11 AM, Ben Horgan wrote: > On 08/09/2026 22:12, Reinette Chatre wrote: >> On 9/7/26 10:01 AM, Ben Horgan wrote: >>> On 03/09/2026 16:29, Reinette Chatre wrote: >>>> On 9/2/26 9:10 AM, Ben Horgan wrote: >>>>> On 01/09/2026 00:54, Reinette Chatre wrote: ... >>>> It is not clear to me if a mon_comp of NULL is able to handle all scenarios since it looks >>>> like mpam_resctrl_get_mon_domain_from_cpu() and mpam_resctrl_online_domain_hdr() does not >>>> consider the component at all. Would that not cause monitoring features to depend on which >>>> CPU of a domain comes online first? >>>> >>>> Could mon_comp perhaps be required to be !NULL here as a replacement for the earlier >>>> "ctrl_comp" check to ensure there is a component with the CPU in its affinity mask? >>> >>> Doesn't the !any_mon_comp check provide this? >> >> This is the part that I do not understand since any_mon_comp seems to support the scenario >> where a mon_comp may be NULL which is a scenario that I do not think resctrl can support. > > Ah, I see what you are getting at. As the monitor components are only considered when there topology > matches the l3 cache (same cpu affinity for each instance) then the find_component() call will never > fail and so mon_comp can't be NULL at this point. > >> >> At a high level there seems to be three affinity masks used by the monitoring code: >> the CPU affinity of the component belonging to the control resource class, the CPU affinity >> of each component supporting each monitoring event, while these are three separate masks with >> code sometimes treating them as though they can be different they are actually required to be the same? > > Monitor component CPU affinity is enforced by topology_matches_l3() to be the same as the L3. > Additionally, traffic_matches_l3() adds extra conditions that mean that there can be considered an > uninterrupted link between l3 and memory and so an MSC at either end is effectively the same. Namely > the same restrictions I've mentioned before, only a single l3 cache, a single NUMA node and no > intermediate caches. > > You list two rather than three here? Possibly you are also thinking the MSC cpu affinity which can I did mention three masks. Thank you for clarifying how the one mask (the "ctrl_comp" one) is associated with either that of the control class or the same as the class associated with the first event. Regarding the other two masks: it seems to me as though the CPU masks associated with the two supported events are managed separately. This means that theoretically the class associated with QOS_L3_OCCUP_EVENT_ID could have components with different affinity from the components of the class associated with QOS_L3_MBM_TOTAL_EVENT_ID. A CPU being onlined could thus be associated with QOS_L3_OCCUP_EVENT_ID (resulting in mon_comp being initialized for this CPU) but not with QOS_L3_MBM_TOTAL_EVENT_ID (mon_comp is NULL for the same CPU). In this scenario, "any_mon_comp" will be true and the domain created and onlined while it does not actually support both events? ... >> I think that it will support the multiple control work if this is done upstream first. >> I will incorporate your changes in the next version ... I believe that would make you >> a co-author? > > I don't think co-author is necessary as I've just offered review and you mentioned you'd already > tried out the new structures. Up to you though. Let's see how the next version goes as I become more familiar with MPAM. Thank you very much for all the insights. Reinette ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains 2026-09-10 15:37 ` Reinette Chatre @ 2026-09-10 16:28 ` Ben Horgan 2026-09-10 18:10 ` Reinette Chatre 0 siblings, 1 reply; 10+ messages in thread From: Ben Horgan @ 2026-09-10 16:28 UTC (permalink / raw) To: Reinette Chatre, james.morse, Dave.Martin, fenghuay Cc: tony.luck, babu.moger, yu.c.chen, linux-arm-kernel, linux-kernel, patches Hi Reinette, On 10/09/2026 16:37, Reinette Chatre wrote: > Hi Ben, > > On 9/10/26 4:11 AM, Ben Horgan wrote: >> On 08/09/2026 22:12, Reinette Chatre wrote: >>> On 9/7/26 10:01 AM, Ben Horgan wrote: >>>> On 03/09/2026 16:29, Reinette Chatre wrote: >>>>> On 9/2/26 9:10 AM, Ben Horgan wrote: >>>>>> On 01/09/2026 00:54, Reinette Chatre wrote: > > ... > >>>>> It is not clear to me if a mon_comp of NULL is able to handle all scenarios since it looks >>>>> like mpam_resctrl_get_mon_domain_from_cpu() and mpam_resctrl_online_domain_hdr() does not >>>>> consider the component at all. Would that not cause monitoring features to depend on which >>>>> CPU of a domain comes online first? >>>>> >>>>> Could mon_comp perhaps be required to be !NULL here as a replacement for the earlier >>>>> "ctrl_comp" check to ensure there is a component with the CPU in its affinity mask? >>>> >>>> Doesn't the !any_mon_comp check provide this? >>> >>> This is the part that I do not understand since any_mon_comp seems to support the scenario >>> where a mon_comp may be NULL which is a scenario that I do not think resctrl can support. >> >> Ah, I see what you are getting at. As the monitor components are only considered when there topology >> matches the l3 cache (same cpu affinity for each instance) then the find_component() call will never >> fail and so mon_comp can't be NULL at this point. >> >>> >>> At a high level there seems to be three affinity masks used by the monitoring code: >>> the CPU affinity of the component belonging to the control resource class, the CPU affinity >>> of each component supporting each monitoring event, while these are three separate masks with >>> code sometimes treating them as though they can be different they are actually required to be the same? >> >> Monitor component CPU affinity is enforced by topology_matches_l3() to be the same as the L3. >> Additionally, traffic_matches_l3() adds extra conditions that mean that there can be considered an >> uninterrupted link between l3 and memory and so an MSC at either end is effectively the same. Namely >> the same restrictions I've mentioned before, only a single l3 cache, a single NUMA node and no >> intermediate caches. >> >> You list two rather than three here? Possibly you are also thinking the MSC cpu affinity which can > > I did mention three masks. Thank you for clarifying how the one mask (the "ctrl_comp" one) is associated with > either that of the control class or the same as the class associated with the first event. Ah yes, I misunderstood. > > Regarding the other two masks: it seems to me as though the CPU masks associated with the two supported events > are managed separately. This means that theoretically the class associated with QOS_L3_OCCUP_EVENT_ID could have > components with different affinity from the components of the class associated with QOS_L3_MBM_TOTAL_EVENT_ID. A CPU > being onlined could thus be associated with QOS_L3_OCCUP_EVENT_ID (resulting in mon_comp being > initialized for this CPU) but not with QOS_L3_MBM_TOTAL_EVENT_ID (mon_comp is NULL for the same CPU). In > this scenario, "any_mon_comp" will be true and the domain created and onlined while it does not > actually support both events? Hmm, it depends what you mean by "theoretically". In mpam_resctrl_pick_counters() the class to back each event is chosen. For QOS_L3_OCCUP_EVENT_ID it will always be the class at the L3. For QOS_L3_MBM_TOTAL_EVENT_ID the topology_matches_l3() call will check the cpu mask matches the L3. Does that answer query or am I missing something else? Thanks, Ben > > ... >>> I think that it will support the multiple control work if this is done upstream first. >>> I will incorporate your changes in the next version ... I believe that would make you >>> a co-author? >> >> I don't think co-author is necessary as I've just offered review and you mentioned you'd already >> tried out the new structures. Up to you though. > Let's see how the next version goes as I become more familiar with MPAM. > > Thank you very much for all the insights. > > Reinette > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains 2026-09-10 16:28 ` Ben Horgan @ 2026-09-10 18:10 ` Reinette Chatre 2026-09-11 8:56 ` Ben Horgan 0 siblings, 1 reply; 10+ messages in thread From: Reinette Chatre @ 2026-09-10 18:10 UTC (permalink / raw) To: Ben Horgan, james.morse, Dave.Martin, fenghuay Cc: tony.luck, babu.moger, yu.c.chen, linux-arm-kernel, linux-kernel, patches Hi Ben, On 9/10/26 9:28 AM, Ben Horgan wrote: > Hi Reinette, > > On 10/09/2026 16:37, Reinette Chatre wrote: >> Hi Ben, >> >> On 9/10/26 4:11 AM, Ben Horgan wrote: >>> On 08/09/2026 22:12, Reinette Chatre wrote: >>>> On 9/7/26 10:01 AM, Ben Horgan wrote: >>>>> On 03/09/2026 16:29, Reinette Chatre wrote: >>>>>> On 9/2/26 9:10 AM, Ben Horgan wrote: >>>>>>> On 01/09/2026 00:54, Reinette Chatre wrote: >> >> ... >> >>>>>> It is not clear to me if a mon_comp of NULL is able to handle all scenarios since it looks >>>>>> like mpam_resctrl_get_mon_domain_from_cpu() and mpam_resctrl_online_domain_hdr() does not >>>>>> consider the component at all. Would that not cause monitoring features to depend on which >>>>>> CPU of a domain comes online first? >>>>>> >>>>>> Could mon_comp perhaps be required to be !NULL here as a replacement for the earlier >>>>>> "ctrl_comp" check to ensure there is a component with the CPU in its affinity mask? >>>>> >>>>> Doesn't the !any_mon_comp check provide this? >>>> >>>> This is the part that I do not understand since any_mon_comp seems to support the scenario >>>> where a mon_comp may be NULL which is a scenario that I do not think resctrl can support. >>> >>> Ah, I see what you are getting at. As the monitor components are only considered when there topology >>> matches the l3 cache (same cpu affinity for each instance) then the find_component() call will never >>> fail and so mon_comp can't be NULL at this point. >>> >>>> >>>> At a high level there seems to be three affinity masks used by the monitoring code: >>>> the CPU affinity of the component belonging to the control resource class, the CPU affinity >>>> of each component supporting each monitoring event, while these are three separate masks with >>>> code sometimes treating them as though they can be different they are actually required to be the same? >>> >>> Monitor component CPU affinity is enforced by topology_matches_l3() to be the same as the L3. >>> Additionally, traffic_matches_l3() adds extra conditions that mean that there can be considered an >>> uninterrupted link between l3 and memory and so an MSC at either end is effectively the same. Namely >>> the same restrictions I've mentioned before, only a single l3 cache, a single NUMA node and no >>> intermediate caches. >>> >>> You list two rather than three here? Possibly you are also thinking the MSC cpu affinity which can >> >> I did mention three masks. Thank you for clarifying how the one mask (the "ctrl_comp" one) is associated with >> either that of the control class or the same as the class associated with the first event. > > Ah yes, I misunderstood. > >> >> Regarding the other two masks: it seems to me as though the CPU masks associated with the two supported events >> are managed separately. This means that theoretically the class associated with QOS_L3_OCCUP_EVENT_ID could have >> components with different affinity from the components of the class associated with QOS_L3_MBM_TOTAL_EVENT_ID. A CPU >> being onlined could thus be associated with QOS_L3_OCCUP_EVENT_ID (resulting in mon_comp being >> initialized for this CPU) but not with QOS_L3_MBM_TOTAL_EVENT_ID (mon_comp is NULL for the same CPU). In >> this scenario, "any_mon_comp" will be true and the domain created and onlined while it does not >> actually support both events? > > Hmm, it depends what you mean by "theoretically". In mpam_resctrl_pick_counters() the class to back > each event is chosen. For QOS_L3_OCCUP_EVENT_ID it will always be the class at the L3. For > QOS_L3_MBM_TOTAL_EVENT_ID the topology_matches_l3() call will check the cpu mask matches the L3. > Does that answer query or am I missing something else? I mean "theoretically" because it seems that the CPU online code (specifically domain creation) supports the scenario where these CPU masks of the two events are different (mon_comp can be set for one event but not the other) while the rest of the driver seems to make an effort to keep these CPU masks identical (as you highlight) and there is not actually support for them being different. The planned changes discussed here are for the domain creation and will touch the code handling this scenario. I was hoping to just change the data structures while maintaining the current flows as closely as possible. I currently struggle with maintaining this flow that can never, and more importantly should never, be encountered. Do (admittedly crude) guardrails like below capture the existing driver requirements? diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c index 9d223057953a..dbd06371890c 100644 --- a/drivers/resctrl/mpam_resctrl.c +++ b/drivers/resctrl/mpam_resctrl.c @@ -1681,11 +1681,18 @@ mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res) continue; // dummy resource mon_comp = find_component(mon->class, cpu); + if (!mon_comp) { + WARN_ON_ONCE(0); + err = -EFAULT; + goto offline_ctrl_domain; + } dom->mon_comp[eventid] = mon_comp; - if (mon_comp) - any_mon_comp = mon_comp; + any_mon_comp = mon_comp; } - if (!any_mon_comp) { + + /* hack */ + if (!cpumask_equal(&dom->mon_comp[QOS_L3_OCCUP_EVENT_ID]->affinity, + &dom->mon_comp[QOS_L3_MBM_TOTAL_EVENT_ID]->affinity)) { WARN_ON_ONCE(0); err = -EFAULT; goto offline_ctrl_domain; Reinette ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains 2026-09-10 18:10 ` Reinette Chatre @ 2026-09-11 8:56 ` Ben Horgan 0 siblings, 0 replies; 10+ messages in thread From: Ben Horgan @ 2026-09-11 8:56 UTC (permalink / raw) To: Reinette Chatre, james.morse, Dave.Martin, fenghuay Cc: tony.luck, babu.moger, yu.c.chen, linux-arm-kernel, linux-kernel, patches Hi Reinette, On 10/09/2026 19:10, Reinette Chatre wrote: > Hi Ben, > > On 9/10/26 9:28 AM, Ben Horgan wrote: >> Hi Reinette, >> >> On 10/09/2026 16:37, Reinette Chatre wrote: >>> Hi Ben, >>> >>> On 9/10/26 4:11 AM, Ben Horgan wrote: >>>> On 08/09/2026 22:12, Reinette Chatre wrote: >>>>> On 9/7/26 10:01 AM, Ben Horgan wrote: >>>>>> On 03/09/2026 16:29, Reinette Chatre wrote: >>>>>>> On 9/2/26 9:10 AM, Ben Horgan wrote: >>>>>>>> On 01/09/2026 00:54, Reinette Chatre wrote: >>> >>> ... >>> >>>>>>> It is not clear to me if a mon_comp of NULL is able to handle all scenarios since it looks >>>>>>> like mpam_resctrl_get_mon_domain_from_cpu() and mpam_resctrl_online_domain_hdr() does not >>>>>>> consider the component at all. Would that not cause monitoring features to depend on which >>>>>>> CPU of a domain comes online first? >>>>>>> >>>>>>> Could mon_comp perhaps be required to be !NULL here as a replacement for the earlier >>>>>>> "ctrl_comp" check to ensure there is a component with the CPU in its affinity mask? >>>>>> >>>>>> Doesn't the !any_mon_comp check provide this? >>>>> >>>>> This is the part that I do not understand since any_mon_comp seems to support the scenario >>>>> where a mon_comp may be NULL which is a scenario that I do not think resctrl can support. >>>> >>>> Ah, I see what you are getting at. As the monitor components are only considered when there topology >>>> matches the l3 cache (same cpu affinity for each instance) then the find_component() call will never >>>> fail and so mon_comp can't be NULL at this point. >>>> >>>>> >>>>> At a high level there seems to be three affinity masks used by the monitoring code: >>>>> the CPU affinity of the component belonging to the control resource class, the CPU affinity >>>>> of each component supporting each monitoring event, while these are three separate masks with >>>>> code sometimes treating them as though they can be different they are actually required to be the same? >>>> >>>> Monitor component CPU affinity is enforced by topology_matches_l3() to be the same as the L3. >>>> Additionally, traffic_matches_l3() adds extra conditions that mean that there can be considered an >>>> uninterrupted link between l3 and memory and so an MSC at either end is effectively the same. Namely >>>> the same restrictions I've mentioned before, only a single l3 cache, a single NUMA node and no >>>> intermediate caches. >>>> >>>> You list two rather than three here? Possibly you are also thinking the MSC cpu affinity which can >>> >>> I did mention three masks. Thank you for clarifying how the one mask (the "ctrl_comp" one) is associated with >>> either that of the control class or the same as the class associated with the first event. >> >> Ah yes, I misunderstood. >> >>> >>> Regarding the other two masks: it seems to me as though the CPU masks associated with the two supported events >>> are managed separately. This means that theoretically the class associated with QOS_L3_OCCUP_EVENT_ID could have >>> components with different affinity from the components of the class associated with QOS_L3_MBM_TOTAL_EVENT_ID. A CPU >>> being onlined could thus be associated with QOS_L3_OCCUP_EVENT_ID (resulting in mon_comp being >>> initialized for this CPU) but not with QOS_L3_MBM_TOTAL_EVENT_ID (mon_comp is NULL for the same CPU). In >>> this scenario, "any_mon_comp" will be true and the domain created and onlined while it does not >>> actually support both events? >> >> Hmm, it depends what you mean by "theoretically". In mpam_resctrl_pick_counters() the class to back >> each event is chosen. For QOS_L3_OCCUP_EVENT_ID it will always be the class at the L3. For >> QOS_L3_MBM_TOTAL_EVENT_ID the topology_matches_l3() call will check the cpu mask matches the L3. >> Does that answer query or am I missing something else? > I mean "theoretically" because it seems that the CPU online code (specifically domain creation) supports > the scenario where these CPU masks of the two events are different (mon_comp can be set for one event but > not the other) while the rest of the driver seems to make an effort to keep these CPU masks identical (as > you highlight) and there is not actually support for them being different. > > The planned changes discussed here are for the domain creation and will touch the code handling this > scenario. I was hoping to just change the data structures while maintaining the current flows as closely > as possible. I currently struggle with maintaining this flow that can never, and more importantly should > never, be encountered. I see, makes sense. > > Do (admittedly crude) guardrails like below capture the existing driver requirements? > > diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c > index 9d223057953a..dbd06371890c 100644 > --- a/drivers/resctrl/mpam_resctrl.c > +++ b/drivers/resctrl/mpam_resctrl.c > @@ -1681,11 +1681,18 @@ mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res) > continue; // dummy resource > > mon_comp = find_component(mon->class, cpu); > + if (!mon_comp) { > + WARN_ON_ONCE(0); > + err = -EFAULT; > + goto offline_ctrl_domain; > + } > dom->mon_comp[eventid] = mon_comp; > - if (mon_comp) > - any_mon_comp = mon_comp; > + any_mon_comp = mon_comp; This first part which ensures that if there is a class for the monitor then a component can always be found for the given CPU. > } > - if (!any_mon_comp) { The any_mon_comp check could still be useful to confirm that if r->mon_capable then there is a component for at least one of the events. > + > + /* hack */ > + if (!cpumask_equal(&dom->mon_comp[QOS_L3_OCCUP_EVENT_ID]->affinity, > + &dom->mon_comp[QOS_L3_MBM_TOTAL_EVENT_ID]->affinity)) { There doesn't necessary need to be a monitoring class associated with any particular event. If there are classes and so components for each of the two supported events then this check looks correct. You could ensure that by checking existence: if (dom->mon_comp[QOS_L3_OCCUP_EVENT_ID] && dom->mon_comp[QOS_L3_MBM_TOTAL_EVENT_ID] && !cpumask_equal(&dom->mon_comp[QOS_L3_OCCUP_EVENT_ID]->affinity, &dom->mon_comp[QOS_L3_MBM_TOTAL_EVENT_ID]->affinity)) Thanks, Ben > WARN_ON_ONCE(0); > err = -EFAULT; > goto offline_ctrl_domain; > > > Reinette > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-11 8:56 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-31 23:54 [RFC PATCH] arm_mpam: resctrl: Separate MPAM domains Reinette Chatre 2026-09-02 16:10 ` Ben Horgan 2026-09-03 15:29 ` Reinette Chatre 2026-09-07 17:01 ` Ben Horgan 2026-09-08 21:12 ` Reinette Chatre 2026-09-10 11:11 ` Ben Horgan 2026-09-10 15:37 ` Reinette Chatre 2026-09-10 16:28 ` Ben Horgan 2026-09-10 18:10 ` Reinette Chatre 2026-09-11 8:56 ` Ben Horgan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox