Re: [RFC PATCH v2 05/10] arm_mpam: resctrl: Give the monitoring domain its own structure

[Jinjie Ruan <ruanjinjie@huawei.com>]

在 2026/10/27 0:55, Reinette Chatre 写道:
> Hi Jinjie,
> 
> On 10/26/26 5:14 AM, Jinjie Ruan wrote:
>> 在 2026/10/20 6:07, Reinette Chatre 写道:
> 
> ...
> 
>>> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
>>> index 246560247a28..ce5519635c51 100644
>>> --- a/drivers/resctrl/mpam_resctrl.c
>>> +++ b/drivers/resctrl/mpam_resctrl.c
> 
> ...
> 
>>> @@ -542,7 +542,7 @@ int resctrl_arch_cntr_read(struct rdt_resource *r, struct rdt_l3_mon_domain *d,
>>>  			   enum resctrl_event_id eventid, u64 *val)
>>>  {
>>>  	struct mpam_resctrl_mon *mon = &mpam_resctrl_counters[eventid];
>>> -	struct mpam_resctrl_dom *l3_dom;
>>> +	struct mpam_resctrl_mon_dom *l3_dom;
>>>  	struct mpam_component *mon_comp;
>>>  
>>>  	if (!mpam_is_enabled())
>>> @@ -551,7 +551,7 @@ int resctrl_arch_cntr_read(struct rdt_resource *r, struct rdt_l3_mon_domain *d,
>>>  	if (eventid == QOS_L3_OCCUP_EVENT_ID || !mon->class)
>>>  		return -EINVAL;
>>>  
>>> -	l3_dom = container_of(d, struct mpam_resctrl_dom, resctrl_mon_dom);
>>> +	l3_dom = container_of(d, struct mpam_resctrl_mon_dom, resctrl_mon_dom);
>>>  	mon_comp = l3_dom->mon_comp[eventid];
>>
>> Maybe following code could be extracted into a helper
>>
>> 553 >-------if (!mpam_is_enabled())
>>  554 >------->-------return -EINVAL;
>>  555
>>  556 >-------if (eventid == QOS_L3_OCCUP_EVENT_ID || !mon->class)
>>  557 >------->-------return -EINVAL;
>>  558
>>  559 >-------l3_dom = container_of(d, struct mpam_resctrl_dom,
>> resctrl_mon_dom);
>>  560 >-------mon_comp = l3_dom->mon_comp[eventid];
>>
> 
> Below shows an example of such helper created (for convenience) on top of this
> series. Only compile tested. Is this what you had in mind? If it looks ok I will
> rebase to the beginning of this series as a new preparatory patch since it refactors
> code adjacent to this series.

Right. It looks fine to me. resctrl_arch_rmid_read() seems a little
different.

> 
> ---8<---
>  drivers/resctrl/mpam_resctrl.c | 62 ++++++++++++++++++++++------------
>  1 file changed, 41 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> index 61fbde9c0960..d75954f8a0ad 100644
> --- a/drivers/resctrl/mpam_resctrl.c
> +++ b/drivers/resctrl/mpam_resctrl.c
> @@ -7,6 +7,7 @@
>  #include <linux/cacheinfo.h>
>  #include <linux/cpu.h>
>  #include <linux/cpumask.h>
> +#include <linux/err.h>
>  #include <linux/errno.h>
>  #include <linux/limits.h>
>  #include <linux/list.h>
> @@ -536,26 +537,51 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
>  				 closid, rmid, val);
>  }
>  
> -/* MBWU counters when in ABMC mode */
> -int resctrl_arch_cntr_read(struct rdt_resource *r, struct rdt_l3_mon_domain *d,
> -			   u32 closid, u32 rmid, int mon_idx,
> -			   enum resctrl_event_id eventid, u64 *val)
> +/**
> + * find_mbwu_mon_comp() - Find the component counting @eventid in a domain
> + * @d:		The monitoring domain to search.
> + * @eventid:	The event to find the counting component for.
> + *
> + * The counters reached through this component are the monitors MPAM
> + * pre-allocated for bandwidth counting. Cache occupancy is not counted by
> + * those monitors, it is read on demand via resctrl_arch_rmid_read().
> + *
> + * Return: the component counting @eventid in @d, or an error pointer when
> + * MPAM is disabled, when @eventid is cache occupancy, or when no class was
> + * picked to count @eventid.
> + */
> +static struct mpam_component *find_mbwu_mon_comp(struct rdt_l3_mon_domain *d,
> +						 enum resctrl_event_id eventid)
>  {
> -	struct mpam_resctrl_mon *mon = &mpam_resctrl_counters[eventid];
>  	struct mpam_resctrl_mon_dom *l3_dom;
> -	struct mpam_component *mon_comp;
> +	struct mpam_resctrl_mon *mon;
>  
>  	if (!mpam_is_enabled())
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
> +	mon = &mpam_resctrl_counters[eventid];
>  	if (eventid == QOS_L3_OCCUP_EVENT_ID || !mon->class)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	l3_dom = container_of(d, struct mpam_resctrl_mon_dom, resctrl_mon_dom);
> -	mon_comp = l3_dom->mon_comp[eventid];
>  
> -	return read_mon_cdp_safe(mon, mon_comp, mpam_feat_msmon_mbwu,
> -				 USE_PRE_ALLOCATED, closid, rmid, val);
> +	return l3_dom->mon_comp[eventid];
> +}
> +
> +/* MBWU counters when in ABMC mode */
> +int resctrl_arch_cntr_read(struct rdt_resource *r, struct rdt_l3_mon_domain *d,
> +			   u32 closid, u32 rmid, int mon_idx,
> +			   enum resctrl_event_id eventid, u64 *val)
> +{
> +	struct mpam_component *mon_comp;
> +
> +	mon_comp = find_mbwu_mon_comp(d, eventid);
> +	if (IS_ERR(mon_comp))
> +		return PTR_ERR(mon_comp);
> +
> +	return read_mon_cdp_safe(&mpam_resctrl_counters[eventid], mon_comp,
> +				 mpam_feat_msmon_mbwu, USE_PRE_ALLOCATED,
> +				 closid, rmid, val);
>  }
>  
>  static void __reset_mon(struct mpam_resctrl_mon *mon, struct mpam_component *mon_comp,
> @@ -598,20 +624,14 @@ void resctrl_arch_reset_cntr(struct rdt_resource *r, struct rdt_l3_mon_domain *d
>  			     u32 closid, u32 rmid, int cntr_id,
>  			     enum resctrl_event_id eventid)
>  {
> -	struct mpam_resctrl_mon *mon = &mpam_resctrl_counters[eventid];
> -	struct mpam_resctrl_mon_dom *l3_dom;
>  	struct mpam_component *mon_comp;
>  
> -	if (!mpam_is_enabled())
> -		return;
> -
> -	if (eventid == QOS_L3_OCCUP_EVENT_ID || !mon->class)
> +	mon_comp = find_mbwu_mon_comp(d, eventid);
> +	if (IS_ERR(mon_comp))
>  		return;
>  
> -	l3_dom = container_of(d, struct mpam_resctrl_mon_dom, resctrl_mon_dom);
> -	mon_comp = l3_dom->mon_comp[eventid];
> -
> -	reset_mon_cdp_safe(mon, mon_comp, USE_PRE_ALLOCATED, closid, rmid);
> +	reset_mon_cdp_safe(&mpam_resctrl_counters[eventid], mon_comp,
> +			   USE_PRE_ALLOCATED, closid, rmid);
>  }
>  
>  /*
> ---8<--- 
> 
>>> @@ -1770,11 +1723,97 @@ mpam_resctrl_get_domain_from_cpu(int cpu, struct mpam_resctrl_res *res)
>>>  			return dom;
>>>  	}
>>>  
>>> -	if (r->rid != RDT_RESOURCE_L3)
>>> -		return NULL;
>>> +	return NULL;
>>
>> LGTM
>> Reviewed-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>
> Thank you very much.
> 
> Reinette

-- 
Best regards,
Jinjie