Re: [PATCH v4 5/7] fs/resctrl: Continue counter allocation after failure

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

Hi Ben,

On 4/15/26 9:31 AM, Ben Horgan wrote:
> On 4/15/26 16:38, Reinette Chatre wrote:
>> On 4/15/26 7:46 AM, Ben Horgan wrote:
>>> On 4/15/26 15:27, Reinette Chatre wrote:
>>>> On 4/14/26 7:42 AM, Ben Horgan wrote:
>>>>> On 3/27/26 16:21, Reinette Chatre wrote:
>>>>
>>>>>> Consider a changelog like below that just focuses on problem being solved
>>>>>> (but please correct me if you find I am missing the point):
>>>>>>
>>>>>> 	In mbm_event mode, with mbm_assign_on_mkdir set to 1, when a user
>>>>>> 	creates a new CTRL_MON or MON group resctrl attempts to allocate
>>>>>> 	counters for each of the supported MBM events on each resctrl
>>>>>> 	domain. As counters are limited, such allocation may fail and
>>>>>> 	when it does counter allocations for the remaining domains are
>>>>>> 	skipped even if the domains have available counters.                                                                       
>>>>>>                                                                                 
>>>>>> 	Since a counter allocation failure may result in counter allocation
>>>>>> 	skipped on other domains the user needs to view the resource group's
>>>>>
>>>>> skipped -> being skipped
>>>>>
>>>>>> 	mbm_L3_assignments files to get an accurate view of counter assignment
>>>>>> 	in a new resource group and then manually create counters in the skipped
>>>>>> 	domains	with available counters.                                             
>>>>>>                                                                                 
>>>>>> 	Writes to mbm_L3_assignments using the wildcard format, <event>:*=e,            
>>>>>> 	also skip counter allocation in other domains after a counter allocation      
>>>>>> 	failure.                                                                        
>>>>>>                                                                                 
>>>>>> 	When handling a request to create counters in all domains it is unnecessary
>>>>>> 	for a counter allocation in one domain to prevent counter allocation in
>>>>>> 	other domains. Always attempt to allocate all the counters requested.     
>>>>>
>>>>> I can use this but how about if I add,
>>>>>
>>>>> Skipping counter allocation in subsequent domains after failure makes predicting which
>>>>> counters will be allocated harder for the user as they need to know the ordering of the
>>>>> domains as well as the expected failures.
>>>>
>>>> I do not see why the user needs to make any predictions with the current implementation.
>>>> mbm_L3_assignments will always contain accurate information regarding counter assignment, no?
>>>
>>> They can see the result with mbm_L3_assignments. In general, if the user is doing any operation it helps for them to
>>> know what they can expect from that operation before doing it.
>>
>> I totally agree. I seem to be missing the goal here. Are you saying that currently it is not clear
>> what the user can expect when running these commands? I believe that is clarified with the documentation
>> update in patch #6?
> 
> With this patch and the documentation in patch #6 the user should be clear on what to expect. Without this patch the

Agree with "With this patch ...".

> expectation is to iterate through the domains allocating counters but not consider subsequent domains after a failure.

I hear what you are trying to say with the "Without this patch ..." part but I am hesitant to
equate current undocumented kernel behavior as established user expectations. Instead I think
when user requests counter allocation in all domains then expectation would/should be that
counters will be allocated in all domains (that have available counters). I thus see this work
more as a fix that brings behavior closer to match what user may expect when running
impacted commands.

One possible expectation that user space may develop based on experience with existing implementation
is that requests to allocate counters may fail even if counters are available in some domains. If user
space does have such expectation then they still need to consider mbm_L3_assignments to know
what was actually allocated and I still do not see how they need to make any predictions to
use resctrl.

We thus seem to disagree on user expectations? Since this is quite subjective that may be ok since
I believe this work addresses all aspects?

> This means that the order of the domains are considered effects the outcome of command.

Right.

> 
> With this patch: each counter is attempted to be allocated
> Without this patch: Domains are considered in order of ascending id and each counter in the domain is attempted to be
> allocated but if a counter allocation fails no subsequent domains are considered.

Understood.

> 
> I think the behaviour with this patch is easier to understand and what would be most useful to the user.

I agree.

> 
> Suppose you have two domains, domain 0 and 1 and one counter remaining on one of the domains and create a new group
> (with mbm_assign_on_mkdir set to 1). Without this patch the available counter is only allocated if that domain is
> considered first (lower id) but with this patch it allocated regardless of which domain it's in.

Understood.

>>> Happy to drop the extra sentence if you don't think it adds anything. Making all allocations of multiple counters best
>>> effort is the main point.
>>
>> I do not object adding extra sentences but the proposal mentions how user space needs to
>> predict behaviors and know about kernel internals which I do not believe is required now nor
>> with planned changes. I really seem to be missing something here so would appreciate if you
>> could elaborate the goals here.
> 
> It seems that we agree on the patch but I've somehow confused you on the motivation. In essence, it's making a better
> effort at best effort counter allocation. Does this help clarify things?

I understand motivation and I agree this patch improves current behavior. You need not 
motivate this patch. My comment was about the additional information that you propose
adding to the changelog where I still do not see user needing to ever make any predictions to
use the existing interfaces.

Reinette