Re: [PATCH v6 25/40] arm_mpam: resctrl: Add support for 'MB' resource

[James Morse <james.morse@arm.com>]

Hi Gavin,

On 23/03/2026 23:09, Gavin Shan wrote:
> On 3/14/26 12:46 AM, Ben Horgan wrote:
>> From: James Morse <james.morse@arm.com>
>>
>> resctrl supports 'MB', as a percentage throttling of traffic from the
>> L3. This is the control that mba_sc uses, so ideally the class chosen
>> should be as close as possible to the counters used for mbm_total. If there
>> is a single L3, it's the last cache, and the topology of the memory matches
>> then the traffic at the memory controller will be equivalent to that at
>> egress of the L3. If these conditions are met allow the memory class to
>> back MB.
>>
>> MB's percentage control should be backed either with the fixed point
>> fraction MBW_MAX or bandwidth portion bitmaps. The bandwidth portion
>> bitmaps is not used as its tricky to pick which bits to use to avoid
>> contention, and may be possible to expose this as something other than a
>> percentage in the future.

> One comment below and it deserves to be addressed if we have another respin:
> 
> Reviewed-by: Gavin Shan <gshan@redhat.com>

Thanks!


>> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
>> index 93c8a9608ed4..cad65cf7d12d 100644
>> --- a/drivers/resctrl/mpam_resctrl.c
>> +++ b/drivers/resctrl/mpam_resctrl.c
>> @@ -317,6 +344,166 @@ static u16 percent_to_mbw_max(u8 pc, struct mpam_props *cprops)

>> +/*
>> + * Test if the traffic for a class matches that at egress from the L3. For
>> + * MSC at memory controllers this is only possible if there is a single L3
>> + * as otherwise the counters at the memory can include bandwidth from the
>> + * non-local L3.
>> + */
>> +static bool traffic_matches_l3(struct mpam_class *class)
>> +{
>> +    int err, cpu;
>> +
>> +    lockdep_assert_cpus_held();
>> +
>> +    if (class->type == MPAM_CLASS_CACHE && class->level == 3)
>> +        return true;
>> +
>> +    if (class->type == MPAM_CLASS_CACHE && class->level != 3) {
>> +        pr_debug("class %u is a different cache from L3\n", class->level);
>> +        return false;
>> +    }
>> +
>> +    if (class->type != MPAM_CLASS_MEMORY) {
>> +        pr_debug("class %u is neither of type cache or memory\n", class->level);
>> +        return false;
>> +    }
>> +
> 
> We bail if the calss isn't MPAM_CLASS_MEMORY here ...
> 
>> +    cpumask_var_t __free(free_cpumask_var) tmp_cpumask = CPUMASK_VAR_NULL;
>> +    if (!alloc_cpumask_var(&tmp_cpumask, GFP_KERNEL)) {
>> +        pr_debug("cpumask allocation failed\n");
>> +        return false;
>> +    }
>> +
>> +    if (class->type != MPAM_CLASS_MEMORY) {
>> +        pr_debug("class %u is neither of type cache or memory\n",
>> +             class->level);
>> +        return false;
>> +    }
>> +
> 
> Duplicated check here as the previous one. So this check can be dropped.

Heh, that looks like a rebase conflict! Thanks for spotting it.

Fixed locally.


James