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

[Ben Horgan <ben.horgan@arm.com>]

Hi Fenghua,

On 7/2/26 17:30, Fenghua Yu wrote:
> 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 .."

How about "An MSC measures bandwidth for a path determined by it's
location in hardware."?

Thanks,

Ben

> 
>> +             * 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