Re: [PATCH v2 28/45] arm_mpam: resctrl: Pick classes for use as mbm counters
From: Jonathan Cameron <jonathan.cameron@huawei.com>
Date: 2026-01-06 14:01:34
Also in:
kvmarm, lkml
On Fri, 19 Dec 2025 18:11:30 +0000 Ben Horgan [off-list ref] wrote:
From: James Morse <james.morse@arm.com> resctrl has two types of counters, NUMA-local and global. MPAM has only bandwidth counters, but the position of the MSC may mean it counts NUMA-local, or global traffic. But the topology information is not available. Apply a heuristic: the L2 or L3 supports bandwidth monitors, these are probably NUMA-local. If the memory controller supports bandwidth monitors, they are probably global. This also allows us to assert that we don't have the same class backing two different resctrl events. Because the class or component backing the event may not be 'the L3', it is necessary for mpam_resctrl_get_domain_from_cpu() to search the monitor domains too. This matters the most for 'monitor only' systems, where 'the L3' control domains may be empty, and the ctrl_comp pointer NULL. resctrl expects there to be enough monitors for every possible control and monitor group to have one. Such a system gets called 'free running' as the monitors can be programmed once and left running. Any other platform will need to emulate ABMC. Signed-off-by: James Morse <james.morse@arm.com> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
Hi Ben, A few minor comments inline. + one question on a worrying sounding todo. Jonathan
quoted hunk ↗ jump to hunk
diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c index 5fde610cc9d7..51caf3b82392 100644 --- a/drivers/resctrl/mpam_resctrl.c +++ b/drivers/resctrl/mpam_resctrl.c
quoted hunk ↗ jump to hunk
@@ -925,6 +982,20 @@ static void mpam_resctrl_domain_insert(struct list_head *list, list_add_tail_rcu(&new->list, pos); } +static struct mpam_component *find_component(struct mpam_class *victim, int cpu)
This is a lovely generic sounding thing, but then the term victim comes in which is very usecase specific. Maybe something could have a better name? (either function or avoid the victim naming).
quoted hunk ↗ jump to hunk
+{ + struct mpam_component *victim_comp; + + guard(srcu)(&mpam_srcu); + list_for_each_entry_srcu(victim_comp, &victim->components, class_list, + srcu_read_lock_held(&mpam_srcu)) { + if (cpumask_test_cpu(cpu, &victim_comp->affinity)) + return victim_comp; + } + + return NULL; +} + static struct mpam_resctrl_dom * mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res) {@@ -973,8 +1044,32 @@ mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res) } if (exposed_mon_capable) { + int i; + struct mpam_component *mon_comp, *any_mon_comp; + + /* + * 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 (i = 0; i < QOS_NUM_EVENTS; i++) {
For consistency with other loops (some of them anyway, I've not done a detailed survey ;) I'd do for (int i = 0; ... Probably bring scope of the mon_comp in here too.
quoted hunk ↗ jump to hunk
+ struct mpam_resctrl_mon *mon; + + mon = &mpam_resctrl_counters[i]; + if (!mon->class) + continue; // dummy resource + + mon_comp = find_component(mon->class, cpu); + dom->mon_comp[i] = mon_comp; + if (mon_comp) + any_mon_comp = mon_comp; + } + WARN_ON_ONCE(!any_mon_comp); + mon_d = &dom->resctrl_mon_dom; - mpam_resctrl_domain_hdr_init(cpu, ctrl_comp, &mon_d->hdr); + mpam_resctrl_domain_hdr_init(cpu, any_mon_comp, &mon_d->hdr); mon_d->hdr.type = RESCTRL_MON_DOMAIN; mpam_resctrl_domain_insert(&r->mon_domains, &mon_d->hdr); err = resctrl_online_mon_domain(r, mon_d);@@ -996,6 +1091,39 @@ mpam_resctrl_alloc_domain(unsigned int cpu, struct mpam_resctrl_res *res) return dom; } +/* + * We know all the monitors are associated with the L3, even if there are no + * controls and therefore no control component. Find the cache-id for the CPU + * and use that to search for existing resctrl domains. + * 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) +{ + u32 cache_id; + struct rdt_mon_domain *mon_d; + struct mpam_resctrl_dom *dom; + struct mpam_resctrl_res *l3 = &mpam_resctrl_controls[RDT_RESOURCE_L3]; + + lockdep_assert_cpus_held(); + + if (!l3->class) + return NULL; + /* TODO: how does this order with cacheinfo updates under cpuhp? */
Considered a blocking todo or something that is future work to resolve if there is an issue?
+ cache_id = get_cpu_cacheinfo_id(cpu, 3);
+ if (cache_id == ~0)
+ return NULL;
+
+ list_for_each_entry_rcu(mon_d, &l3->resctrl_res.mon_domains, hdr.list) {
+ dom = container_of(mon_d, struct mpam_resctrl_dom, resctrl_mon_dom);Might as well move this under the condition. I'm assuming no later patch needs dom for other reasons. if (mon_d->hdr.id == cache_id) return container_of(mon_d, struct mpam_resctrl_dom, resctrl_mon_dom);
+ + if (mon_d->hdr.id == cache_id) + return dom; + } + + return NULL; +}