Re: [PATCH] arm_mpam: resctrl: Leave CDP off when there are too few PARTIDs

[Jinjie Ruan <ruanjinjie01@gmail.com>]

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

Hi Ben,

> On 06/11/2026 08:35, Jinjie Ruan wrote:
>> Hi Ben,
>>
>> Is this modification okay?
> 
> Yes it's ok but there is more we can do here. Sashiko has raised a bug that points out we don't take
> into account whether or not a resource is cdp_capable or not [1].
> 
> This leads to an additional check along the lines of:
> 
> if (enable && (!the_res.alloc_capable || !the_res.cdp_capable))
>      return -EINVAL;

Ok, I will fix it together.

> 
> On a process note, it would helpful to me for reviewing and later on when collecting the patches
> together if you could put related patches together in a series. In the case of this patch it it is
> solving a cdp related problem, as is your patch "arm_mpam: resctrl: Limit num_rmids to the CLOSIDs
> in use". fs/resctrl patches though should be kept separately unless there is a particular reason why
> they should be in a series with MPAM patches, e.g. new resctrl_arch_ helpers and such like.

OK, I will send them in a patch set for the mpam cdp patches.

Best Regards,
Jinjie

> 
> [1] https://sashiko.dev/#/bug/linux-05c19551-0bba-4a06-aa2c-3c90fab5853d
> 
> Thanks,
> 
> Ben
> 
>>
>> 在 2026/10/31 10:52, Jinjie Ruan 写道:
>>> Enabling CDP requires at least 2 PARTIDs. If there are too few PARTIDs,
>>> resctrl_arch_set_cdp_enabled() returns -EINVAL, but only after it has
>>> recorded CDP as enabled, halved the monitor count and turned off the
>>> mbw_max MBA control. resctrl then believes CDP is off, which leaves the
>>> two sides out of step and the halved monitor count and the missing MBA
>>> control are not undone.
>>>
>>> Check the PARTID limit before anything is changed so a failed enable has
>>> no effect.
>>>
>>> Fixes: 6789fb99282c ("arm_mpam: resctrl: Add CDP emulation")
>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>> ---
>>> RFC v2 -> v1:
>>> - Check mpam_partid_max ahead and The partid_d/partid_i determination
>>>    remains in place.
>>> - Update the commit message as Ben suggested.
>>> ---
>>>   drivers/resctrl/mpam_resctrl.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
>>> index 5468367776e2..aa3b3a093857 100644
>>> --- a/drivers/resctrl/mpam_resctrl.c
>>> +++ b/drivers/resctrl/mpam_resctrl.c
>>> @@ -210,6 +210,9 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
>>>   	if (enable)
>>>   		pr_warn("CDP is an expert feature and may cause MPAM to malfunction.\n");
>>>   
>>> +	if (enable && mpam_partid_max < 1)
>>> +		return -EINVAL;
>>> +
>>>   	/*
>>>   	 * resctrl_arch_set_cdp_enabled() is only called with enable set to
>>>   	 * false on error and unmount.
>>> @@ -239,9 +242,6 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
>>>   		mpam_resctrl_controls[RDT_RESOURCE_MBA].resctrl_res.alloc_capable = true;
>>>   
>>>   	if (enable) {
>>> -		if (mpam_partid_max < 1)
>>> -			return -EINVAL;
>>> -
>>>   		partid_d = resctrl_get_config_index(RESCTRL_RESERVED_CLOSID, CDP_DATA);
>>>   		partid_i = resctrl_get_config_index(RESCTRL_RESERVED_CLOSID, CDP_CODE);
>>>   	}
>>
>