Re: [PATCH v1 1/4] x86,fs/resctrl: Make resctrl_arch_is_evt_configurable() aware of mbm_assign_mode

[Ben Horgan]

Hi Reinette,

On 3/5/26 17:22, Reinette Chatre wrote:
> Hi Ben,
> 
> On 3/5/26 2:01 AM, Ben Horgan wrote:
>> On 3/4/26 22:50, Reinette Chatre wrote:
>>> On 3/4/26 1:01 PM, Ben Horgan wrote:
>   
>>>> So, to try and bring this back to what we can be done now for MPAM to
>>>> fit into the counter mode assignment interface. Just support
>>>> mbm_total_bytes and then num_mbm_cntrs is correct (nothing to do). Make
>>>> the event_filter file always display all the bandwidth types and make
>>>> that the only value that be the only value it accepts (instead of hiding
>>>> the event_filter file). If you agree I'll respin with that.
>>>
>>> From resctrl side this sounds fine. I don't have any insight into what, if any,
>>> kind of gymnastics the MPAM driver needs to do to make the discovered MSCs with
>>> their varying scope and internal vs external counts fit into this. If initial
>>> implementation indeed forces some components into categories that are not a good
>>> match then when resctrl later does get support for diverse components there may
>>> be surprises to user space along the way. For example, user space may not see the
>>> same memory bandwidth numbers reported by the same events on the same system as
>>> the interface evolves.
>>
>> Indeed, we have already weeded a few things out of the MPAM driver for
>> similar reasons. If we start with mpam only supporting a
>> non-configurable mbm_total_bytes with ABMC I think we're ok. I'll drop
>> the non-ABMC bandwidth counter support from the MPAM driver as even if
>> we've got enough counters, one per (CTRL_MON/MON, evt), we can use ABMC.
> 
> It sounds like the expectation is that when there are enough counters the user
> will then run with "mbm_assign_on_mkdir" set so that counters are always dynamically
> assigned and thus essentially be "non-ABMC bandwidth counter support"?

Yes, that's the intent.

> Since "mbm_assign_on_mkdir" is set by default then user space should get sane
> behavior by default.
> 
> Alternatively, a user space would just always run with "mbm_assign_on_mkdir"
> off and have full control over monitor assignment even when it is not necessary.
> 
> Sounds good to me.
> 
>> Also, when event configuration (read/write filtering) using user defined
>> (or new) events is added this will mean that enough counters becomes a
>> higher limit. That will mean that the software controller is not usable
>> but for now I think we can just fail when that mount option, mba_MBps,
>> is used. Later we can consider using non-ABMC bandwidth counters when
>> the software controller is requested.
> 
> ok. Since info/last_cmd_status is not available to to user when mount fail,
> please do add a message to kernel log to help diagnose resctrl mount failure.
>

Yep, patch 3 does this.

> 
>>
>>>
>>> "make that the only value that be the only value it accepts" - are you saying that
>>> whatever is displayed when user views the "event_filter" file is what the
>>> user can write to the "event_filter" file? I find this a challenging interface
>>> for user space to use. The expectation is that the user can write any supported
>>> memory transaction to that file and when writing fails it can only be because
>>> of an invalid memory transaction. How can user space know that events are not
>>> configurable at all? It sounds as though user space is expected to try configuring
>>> the event with a memory transaction and then, presumably, check last_cmd_status?
>>>
>>> Could this not be simplified by making the "event_filter" file read-only on
>>> MPAM systems?
>>
>> Yes, we'll need some finer grained control for which sets of bandwidth
>> types can be configured further down the line but going with read-only
>> for when there is only one fixed set seems good to me.
> 
> Thank you very much.
> 
> Reinette
> 

Sounds like we've got a plan :) Thanks!

Ben