Re: [PATCH v3 37/47] arm_mpam: resctrl: Update the rmid reallocation limit

Ben Horgan
2026-01-15T16:02:54Z

Hi Shaopeng,

On 1/15/26 10:05, Shaopeng Tan (Fujitsu) wrote:
> Hello Ben,
>
>> From: James Morse <james.morse@arm.com>
>>
>> resctrl's limbo code needs to be told when the data left in a cache is
>> small enough for the partid+pmg value to be re-allocated.
>>
>> x86 uses the cache size divided by the number of rmid users the cache may
>> have. Do the same, but for the smallest cache, and with the number of
>> partid-and-pmg users.
>>
>> 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 v2:
>> Move waiting for cache info into it's own patch
>> ---
>> drivers/resctrl/mpam_resctrl.c | 35 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
>> index 5adc78f9c96f..a6be3ce84241 100644
>> --- a/drivers/resctrl/mpam_resctrl.c
>> +++ b/drivers/resctrl/mpam_resctrl.c
>> @@ -561,6 +561,38 @@ void resctrl_arch_reset_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
>> reset_mon_cdp_safe(mon, mon_comp, USE_PRE_ALLOCATED, closid, rmid);
>> }
>>
>> +/*
>> + * The rmid realloc threshold should be for the smallest cache exposed to
>> + * resctrl.
>> + */
>> +static int update_rmid_limits(struct mpam_class *class)
>> +{
>> + u32 num_unique_pmg = resctrl_arch_system_num_rmid_idx();
>> + struct mpam_props *cprops = &class->props;
>> + struct cacheinfo *ci;
>> +
>> + lockdep_assert_cpus_held();
>> +
>> + /* Assume cache levels are the same size for all CPUs... */
>> + ci = get_cpu_cacheinfo_level(smp_processor_id(), class->level);
>> + if (!ci || ci->size == 0) {
>> + pr_debug("Could not read cache size for class %u\n",
>> + class->level);
>> + return -EINVAL;
>> + }
>> +
>> + if (!mpam_has_feature(mpam_feat_msmon_csu, cprops))
>> + return 0;
>
> Shouldn't it be return -EOPNOTSUPP;?

The intent of returning 0 here is that if csu is not supported on this
class then there is nothing to do and hence no error.

>
> However, before the function update_rmid_limits() is called, there is a check: if (cache_has_usable_csu(class) && topology_matches_l3(class)).
> cache_has_usable_csu(class) already contains an identical check. Therefore, I think it's safe to remove this redundant one.

Yes, the check is redundant and can be removed. (If it were to stay it
should be moved to be before call get_cpu_cacheinfo_level() call so that
wouldn't give spurious errors for non-cache cpus.)

Thanks,

Ben