Re: [PATCH v2 3/6] fs/resctrl: Make 'event_filter' files read only if they're not configurable

[Reinette Chatre]

Hi Ben,

On 3/16/26 10:02 AM, Ben Horgan wrote:
> Hi Tony,
> 
> On 3/16/26 16:27, Luck, Tony wrote:
>>>> Instead of making the file writable, and then fixing the mode. Maybe
>>>> patch the mode in res_common_files[] before calling rdtgroup_add_files():
>>>
>>>
>>> I initially rejected this idea because res_common_files[] is global and the
>>> read/write choice is per event type but we can safely save restore the mode as
>>> we're holding the rdtgroup mutex. How about this?
>>
>> That seems more complicated. Is there an expectation that event_filter
>> needs to be writable on the "total" event but not on the "local" one?
> 
> No, at least not currently. For mpam only the total event is supported.
> 
> Another way of doing it could be to patch the mode in res_common_files[] from
> resctrl_mkdir_event_configs() for both cases, 0444 and 0644, and change the
> starting value to something invalid, e.g. 0.

"event_filter"'s mode could be initialized similar to its fflags. Please see
resctrl_l3_mon_resource_init().

Stepping back, based on the v1 discussion I understand that BMEC does not
apply to MPAM so patching the BMEC specific resctrl_arch_is_evt_configurable() 
in patch #1 to support MPAM does not look right. From what I can tell we need a
new "configurable" property that is specific to assignable counters. I understand
naming is not ideal here but forcing the two independent features together like this
just because resctrl_arch_is_evt_configurable() has a generically relevant name is
not right.

Consider, for example, a new rdt_resource::resctrl_mon::mbm_cntr_configurable
property that arch can set. From resctrl fs side it would be a property that is
only considered if rdt_resource::resctrl_mon::mbm_cntr_assignable == true.

res_common_files[] can initialize the default mode of "event_filter" to be
read-only and resctrl_l3_mon_resource_init() can be expanded to to make
"event_filter" writable if both rdt_resource::resctrl_mon::mbm_cntr_assignable == true
AND rdt_resource::resctrl_mon::mbm_cntr_configurable == true.

Looking at this we do seem to have options between resctrl fs properties
and arch callbacks to do this and considering your earlier concern with
callbacks I wonder if resctrl fs properties would be better?
How about another new property, rdt_resource::resctrl_mon::mbm_cntr_assign_fixed
to replace resctrl_arch_mbm_cntr_assign_fixed()?

Reinette