Re: [PATCH v3] arm_mpam: resctrl: Limit num_rmids to the CLOSIDs in use

[Jinjie Ruan <ruanjinjie@huawei.com>]

在 2026/11/6 19:01, Ben Horgan 写道:
> Hi Jinjie,

Hi Ben,

> 
> On 06/11/2026 08:19, Jinjie Ruan wrote:
>> On MPAM an RMID is a PMG within a PARTID, so resctrl can only allocate
>> the PMGs of the CLOSIDs it uses. info/L3_MON/num_rmids is instead set
>> to the whole PARTID and PMG space when the monitors are initialised,
>> and halved when CDP is enabled. It reports more RMIDs than resctrl can
>> allocate in two cases:
>>
>> - With CDP each CLOSID takes two PARTIDs, and resctrl halves the
>>   CLOSID count rounding down, while MPAM halves the whole RMID space.
>>   With 5 PARTIDs and 2 PMGs resctrl has 2 CLOSIDs and 4 RMIDs, but
>>   num_rmids reads 5. The count is too high whenever the number of
>>   PARTIDs is odd and there is more than one PMG.
>>
>> - On a monitor-only system there are no control groups and resctrl
>>   only uses the default CLOSID, but num_rmids still covers every
>>   PARTID. With 5 PARTIDs and 2 PMGs it reads 10 while only 2 RMIDs
>>   can be allocated.
>>
>> RMID allocation is limited to the CLOSIDs in use, so only the value
>> reported to userspace is too high. Creating as many monitor groups as
>> num_rmids allows fails with -ENOSPC before that number is reached.
>>
>> Work out the number of CLOSIDs with the rules resctrl applies when it
>> is mounted: schemata_list_add() halves a resource's CLOSID count
>> while CDP is enabled on it, rounding down, closid_init() takes the
>> smallest across the resources, and a monitor-only system has only the
>> default CLOSID. Report (pmg_max + 1) RMIDs for each of those CLOSIDs,
>> counting at monitor initialisation and from
>> resctrl_arch_set_cdp_enabled(), which resctrl calls whenever a mount
>> or unmount enables or disables CDP.
>>
>> Fixes: 264c285999fc ("arm_mpam: resctrl: Add monitor initialisation and domain boilerplate")
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> Cc: <stable@vger.kernel.org> # 7.1.x
>> Cc: James Morse <james.morse@arm.com>
>> Cc: Ben Horgan <ben.horgan@arm.com>
>> Cc: Reinette Chatre <reinette.chatre@intel.com>
>> Cc: Fenghua Yu <fenghuay@nvidia.com>
>> ---
>> v3:
>> - Drop resctrl_arch_set_mon_rmid_count() and count the CLOSIDs in the
>>   MPAM driver instead, as Ben suggested.
>> - v2: https://lore.kernel.org/all/20261105123304.2281987-1-ruanjinjie@huawei.com/
>> v2:
>> - Also fix num_rmids on monitor-only systems, as Ben suggested.
>> - Retitle from "arm_mpam: Round the CDP RMID count down to whole CLOSIDs".
>> - v1: https://lore.kernel.org/all/20261104101622.563280-1-ruanjinjie@huawei.com/
>> ---
>>  drivers/resctrl/mpam_resctrl.c | 45 +++++++++++++++++++++++-----------
>>  1 file changed, 31 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
>> index 5468367776e2..6859849f299b 100644
>> --- a/drivers/resctrl/mpam_resctrl.c
>> +++ b/drivers/resctrl/mpam_resctrl.c
>> @@ -191,6 +191,34 @@ static void mpam_resctrl_monitor_sync_abmc_vals(struct rdt_resource *l3)
>>  	 */
>>  }
>>  
>> +/* The RMID count follows the CLOSIDs resctrl uses. */
>> +static void mpam_resctrl_set_num_rmid(void)
>> +{
>> +	struct mpam_resctrl_res *res = &mpam_resctrl_controls[RDT_RESOURCE_L3];
>> +	struct rdt_resource *l3 = &res->resctrl_res;
>> +	enum resctrl_res_level rid;
>> +	u32 num_closid = UINT_MAX;
>> +
>> +	for_each_mpam_resctrl_control(res, rid) {
>> +		u32 res_num_closid;
>> +
>> +		if (!res->resctrl_res.alloc_capable)
>> +			continue;
>> +
>> +		res_num_closid = mpam_partid_max + 1;
>> +		if (res->cdp_enabled)
>> +			res_num_closid /= 2;
>> +
>> +		num_closid = min(num_closid, res_num_closid);
>> +	}
> 
> There are global variables mpam_partid_max, cdp_enabled and a helper resctrl_arch_alloc_capable().
> Using these should make this simpler.

Make sense.

> 
>> +
>> +	/* Monitor-only systems have only the default CLOSID. */
>> +	if (num_closid == UINT_MAX)
>> +		num_closid = 1;
>> +
>> +	l3->mon.num_rmid = (mpam_pmg_max + 1) * num_closid;
>> +}
>> +
>>  int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
>>  {
>>  	u32 partid_i = RESCTRL_RESERVED_CLOSID, partid_d = RESCTRL_RESERVED_CLOSID;
>> @@ -217,11 +245,6 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
>>  	cdp_enabled = enable;
>>  	mpam_resctrl_controls[rid].cdp_enabled = enable;
>>  
>> -	if (enable)
>> -		l3->mon.num_rmid = resctrl_arch_system_num_rmid_idx() / 2;
>> -	else
>> -		l3->mon.num_rmid = resctrl_arch_system_num_rmid_idx();
>> -
>>  	/* The mbw_max feature can't hide cdp as it's a per-partid maximum. */
>>  	if (cdp_enabled && !mpam_resctrl_controls[RDT_RESOURCE_MBA].cdp_enabled)
>>  		mpam_resctrl_controls[RDT_RESOURCE_MBA].resctrl_res.alloc_capable = false;
>> @@ -238,6 +261,8 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
>>  	if (!cdp_enabled && mpam_resctrl_controls[RDT_RESOURCE_MBA].class)
>>  		mpam_resctrl_controls[RDT_RESOURCE_MBA].resctrl_res.alloc_capable = true;
>>  
>> +	mpam_resctrl_set_num_rmid();
>> +
> 
> Why move the location of where num_rmid is set?

The loop needs to look at which resources can be allocated, and the code
above will turn off MBA when CDP is enabled and turn it back on when CDP
is disabled, so it must wait for it to finish making the changes before
calculating.

> 
> Thanks,
> 
> Ben
> 
>>  	if (enable) {
>>  		if (mpam_partid_max < 1)
>>  			return -EINVAL;
>> @@ -1344,15 +1369,7 @@ static int mpam_resctrl_monitor_init(struct mpam_resctrl_mon *mon,
>>  	l3->name = "L3";
>>  	l3->mon_scope = RESCTRL_L3_CACHE;
>>  
>> -	/*
>> -	 * num-rmid is the upper bound for the number of monitoring groups that
>> -	 * can exist simultaneously, including the default monitoring group for
>> -	 * each control group. Hence, advertise the whole rmid_idx space even
>> -	 * though each control group has its own pmg/rmid space. Unfortunately,
>> -	 * this does mean userspace needs to know the architecture to correctly
>> -	 * interpret this value.
>> -	 */
>> -	l3->mon.num_rmid = resctrl_arch_system_num_rmid_idx();
>> +	mpam_resctrl_set_num_rmid();
>>  
>>  	if (type == QOS_L3_MBM_TOTAL_EVENT_ID) {
>>  		int err;
> 

-- 
Best regards,
Jinjie