Re: [PATCH v4 1/5] arm_mpam: resctrl: Pick classes for use as MBM counters

[Fenghua Yu <fenghuay@nvidia.com>]

Hi, Ben,

On 5/20/26 14:24, Ben Horgan wrote:
> From: James Morse <james.morse@arm.com>
> 
> resctrl has two types of bandwidth counters, NUMA-local and global. MPAM
> can only count globally; either using MSC at the L3 cache or in the memory
> controllers. When global and local equate to the same thing continue just
> to call it global.
> 
> Pick the corresponding MPAM classes to back the MBM counters. As resctrl
> requires all monitors to be at the L3 cache, we can only use the counters
> at the memory controllers when they have the same topology as the L3 cache
> and the traffic they see if the same. In particular, for the bandwidth
> counters at the memory controllers to be exposed to resctrl it is required
> there is a single L3 cache and a single NUMA node as otherwise cross NUMA
> traffic will be counted at the wrong instance.
> 
> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Tested-by: Zeng Heng <zengheng4@huawei.com>
> Reviewed-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
> ---
> Changes since rfc v1:
> Move finding any_mon_comp into monitor boilerplate patch
> Move mpam_resctrl_get_domain_from_cpu() into monitor boilerplate
> Remove free running check
> Trim commit message
> 
> Changes since v3:
> Extra paragraph in commit message
> ---
>   drivers/resctrl/mpam_resctrl.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> index 226ff6f532fa..f70fa65d39e4 100644
> --- a/drivers/resctrl/mpam_resctrl.c
> +++ b/drivers/resctrl/mpam_resctrl.c
> @@ -606,6 +606,16 @@ static bool cache_has_usable_csu(struct mpam_class *class)
>   	return true;
>   }
>   
> +static bool class_has_usable_mbwu(struct mpam_class *class)
> +{
> +	struct mpam_props *cprops = &class->props;
> +
> +	if (!mpam_has_feature(mpam_feat_msmon_mbwu, cprops))
> +		return false;
> +
> +	return true;
> +}
> +
>   /*
>    * Calculate the worst-case percentage change from each implemented step
>    * in the control.
> @@ -983,6 +993,22 @@ static void mpam_resctrl_pick_counters(void)
>   				break;
>   			}
>   		}
> +
> +		if (class_has_usable_mbwu(class) &&
> +		    topology_matches_l3(class) &&
> +		    traffic_matches_l3(class)) {
> +			pr_debug("class %u has usable MBWU, and matches L3 topology and traffic\n",
> +				 class->level);
> +
> +			/*
> +			 * We can't distinguish traffic by destination so
> +			 * we don't know if it's staying on the same NUMA

This sentence makes me think counting only on mbm_total is a "software" 
limitation. In fact, it's a MPAM hardware feature (or limitation).

Maybe add a sentence like this to make it clear why mbm_total only comes 
from originally?

MPAM only provides one memory bandwidth usage value for each MSC that 
supports memory bandwidth usage . We can't distinguish traffic .."

> +			 * node. Hence, we can't calculate mbm_local except
> +			 * when we only have one L3 and it's equivalent to
> +			 * mbm_total and so always use mbm_total.
> +			 */
> +			counter_update_class(QOS_L3_MBM_TOTAL_EVENT_ID, class);
> +		}
>   	}
>   }
>   

Thanks.

-Fenghua