Re: [PATCH v2 28/45] arm_mpam: resctrl: Pick classes for use as mbm counters

Jonathan Cameron
2026-01-06T14:01:34Z

On Fri, 19 Dec 2025 18:11:30 +0000
Ben Horgan <ben.horgan@arm.com> 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

> 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

> @@ -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).

> +{
> + 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.

> + 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;
> +}