Re: [PATCH v4] fs/resctrl: Add CONFIG_RESCTRL_ASSIGN_FIXED Kconfig entry

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Ben,

On 3/3/26 6:24 AM, Ben Horgan wrote:
> Hi Reinette,
> 
> On Mon, Mar 02, 2026 at 03:32:39PM -0800, Reinette Chatre wrote:
>> Hi Ben,
>>
>> On 3/2/26 2:02 AM, Ben Horgan wrote:
>>> Hi Boris,
>>>
>>> On 3/1/26 10:49, Borislav Petkov wrote:
>>>> On Wed, Feb 04, 2026 at 04:19:52PM +0000, Ben Horgan wrote:
>>>>> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
>>>>> index 572a9925bd6c..4e9f55ee3267 100644
>>>>> --- a/fs/resctrl/monitor.c
>>>>> +++ b/fs/resctrl/monitor.c
>>>>> @@ -1451,6 +1451,12 @@ ssize_t resctrl_mbm_assign_mode_write(struct kernfs_open_file *of, char *buf,
>>>>>  	}
>>>>>  
>>>>>  	if (enable != resctrl_arch_mbm_cntr_assign_enabled(r)) {
>>>>> +		if (IS_ENABLED(CONFIG_RESCTRL_ASSIGN_FIXED)) {
>>>>
>>>> Does it need to be a Kconfig entry or you can figure out programatically from
>>>> the architecture that that is the case and avoid yet another CONFIG_ knob?
>>>>
>>>> From reading this, I'm thinking
>>>>
>>>> 		if (IS ARM64) {
>>>>
>>>> should do or...?
>>>
>>> Yes, we don't necessarily need the Kconfig and we don't necessarily need this condition at all in
>>> resctrl_mbm_assign_mode_write(), we do need the existing one in resctrl_mbm_assign_mode_show() though.
>>
>> It is not obvious to me why the one in resctrl_mbm_assign_mode_show() is required if
>> a new arch helper could suffice in other parts 
> 
> I think a new arch helper could cover both.
> 
>>
>>>
>>> The Kconfig was effectively added in:
>>>
>>> commit 3b497c3f4f04 ("fs/resctrl: Introduce the interface to display monitoring modes")
>>>
>>> and I was just filling in the missing piece. Most arch specific things in resctrl seem
>>> to be dealt with using resctrl_arch_* hooks. Would it be preferable to create a new one of these?
>>
>> It looks to me as though this patch fits in with the other series you posted at
>> https://lore.kernel.org/lkml/20260225201905.3568624-1-ben.horgan@arm.com/
>>
>> When looking at this patch together with the other series there may indeed be an arch
>> helper that can replace CONFIG_RESCTRL_ASSIGN_FIXED but as I mentioned in that series
>> the MPAM capabilities and strategies for resctrl to support them are not clear to me
>> at this time.
> 
> If you like, I can drop this patch and tag a resctrl_arch_ hook based hook
> version on to that series when I respin. Do you prefer a new hook,
> resctrl_arch_mbm_cntr_assign_fixed(), or using/abusing the error return of
> resctrl_arch_mbm_assign_set()?

I am now focusing on understanding the MPAM requirements around assignable counters that
are being discussed as part of the other series you sent recently. Without that insight it
is difficult to have preference for an interface. 
I do think it will be helpful to keep this work together in one series.

>>> It could be resctrl_arch_mbm_cntr_assign_fixed() although we seem to be getting too many
>>> of these too. Perhaps we could extend the meaning of another hook, resctrl_arch_mbm_assign_set() could
>>
>> Could you please elaborate which category of helper is getting to be too many? We can surely
>> explore improvements.
> 
> Nothing major or anything that necessarily needs to change. Just that for each
> new feature we get a few new resctrl_arch_* and they all need to be updated in
> the MPAM driver even if the feature isn't used.

I see. These helpers evolved as part of the arch fs split. One option may be to have
an arch register callbacks instead that resctrl fs could test for existence and not require
every arch to have a stub.

Reinette