When the counter assignment mode is mbm_event resctrl assumes the mbm
events are configurable and exposes the 'event_filter' files. These files
live at info/L3_MON/event_configs/<event>/event_filter and are used to
display and set the event configuration. The MPAM driver has no support for
changing event configuration and so mbm_event mode can't be used.
In order to support mbm_event event with MPAM make the 'event_filter' files
read only if the event configuration can't be changed. A user can still
chmod the file and so also return an error from event_filter_write().
Signed-off-by: Ben Horgan <ben.horgan@arm.com>
---
fs/resctrl/monitor.c | 6 ++++++
fs/resctrl/rdtgroup.c | 16 ++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index d25622ee22c5..a2da771d82df 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -1414,6 +1414,12 @@ ssize_t event_filter_write(struct kernfs_open_file *of, char *buf, size_t nbytes
rdt_last_cmd_clear();
+ if (!resctrl_arch_is_evt_configurable(mevt->evtid, true)) {
+ rdt_last_cmd_printf("event_filter is not configurable for %s\n", mevt->name);
+ ret = -EPERM;
+ goto out_unlock;
+ }
+
r = resctrl_arch_get_resource(mevt->rid);
if (!resctrl_arch_mbm_cntr_assign_enabled(r)) {
rdt_last_cmd_puts("mbm_event counter assignment mode is not enabled\n");
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 4753841c2ca3..2f0a509b313a 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -2341,6 +2341,22 @@ static int resctrl_mkdir_event_configs(struct rdt_resource *r, struct kernfs_nod
ret = rdtgroup_add_files(kn_subdir2, RFTYPE_ASSIGN_CONFIG);
if (ret)
return ret;
+
+ if (!resctrl_arch_is_evt_configurable(mevt->evtid, true)) {
+ struct iattr iattr = {.ia_valid = ATTR_MODE,};
+ struct kernfs_node *kn;
+
+ kn = kernfs_find_and_get_ns(kn_subdir2, "event_filter", NULL);
+ if (!kn)
+ return -ENOENT;
+
+ iattr.ia_mode = S_IFREG | 0444;
+
+ ret = kernfs_setattr(kn, &iattr);
+ kernfs_put(kn);
+ if (ret)
+ return ret;
+ }
}
return 0;
--
2.43.0On Fri, Mar 13, 2026 at 05:45:21PM +0000, Ben Horgan wrote:
> When the counter assignment mode is mbm_event resctrl assumes the mbm
> events are configurable and exposes the 'event_filter' files. These files
> live at info/L3_MON/event_configs/<event>/event_filter and are used to
> display and set the event configuration. The MPAM driver has no support for
> changing event configuration and so mbm_event mode can't be used.
>
> In order to support mbm_event event with MPAM make the 'event_filter' files
> read only if the event configuration can't be changed. A user can still
> chmod the file and so also return an error from event_filter_write().
> +++ b/fs/resctrl/rdtgroup.c
> @@ -2341,6 +2341,22 @@ static int resctrl_mkdir_event_configs(struct rdt_resource *r, struct kernfs_nod
> ret = rdtgroup_add_files(kn_subdir2, RFTYPE_ASSIGN_CONFIG);
> if (ret)
> return ret;
> +
> + if (!resctrl_arch_is_evt_configurable(mevt->evtid, true)) {
> + struct iattr iattr = {.ia_valid = ATTR_MODE,};
> + struct kernfs_node *kn;
> +
> + kn = kernfs_find_and_get_ns(kn_subdir2, "event_filter", NULL);
> + if (!kn)
> + return -ENOENT;
> +
> + iattr.ia_mode = S_IFREG | 0444;
> +
> + ret = kernfs_setattr(kn, &iattr);
> + kernfs_put(kn);
> + if (ret)
> + return ret;
> + }
Instead of making the file writable, and then fixing the mode. Maybe
patch the mode in res_common_files[] before calling rdtgroup_add_files():
if (!resctrl_arch_is_evt_configurable(mevt->evtid, true)) {
struct rftype *rft;
rft = rdtgroup_get_rftype_by_name("event_filter");
if (rft)
rft->mode = 0444;
}
-Tony
Hi Tony,
On 3/13/26 18:33, Luck, Tony wrote:
> On Fri, Mar 13, 2026 at 05:45:21PM +0000, Ben Horgan wrote:
>> When the counter assignment mode is mbm_event resctrl assumes the mbm
>> events are configurable and exposes the 'event_filter' files. These files
>> live at info/L3_MON/event_configs/<event>/event_filter and are used to
>> display and set the event configuration. The MPAM driver has no support for
>> changing event configuration and so mbm_event mode can't be used.
>>
>> In order to support mbm_event event with MPAM make the 'event_filter' files
>> read only if the event configuration can't be changed. A user can still
>> chmod the file and so also return an error from event_filter_write().
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -2341,6 +2341,22 @@ static int resctrl_mkdir_event_configs(struct rdt_resource *r, struct kernfs_nod
>> ret = rdtgroup_add_files(kn_subdir2, RFTYPE_ASSIGN_CONFIG);
>> if (ret)
>> return ret;
>> +
>> + if (!resctrl_arch_is_evt_configurable(mevt->evtid, true)) {
>> + struct iattr iattr = {.ia_valid = ATTR_MODE,};
>> + struct kernfs_node *kn;
>> +
>> + kn = kernfs_find_and_get_ns(kn_subdir2, "event_filter", NULL);
>> + if (!kn)
>> + return -ENOENT;
>> +
>> + iattr.ia_mode = S_IFREG | 0444;
>> +
>> + ret = kernfs_setattr(kn, &iattr);
>> + kernfs_put(kn);
>> + if (ret)
>> + return ret;
>> + }
>
>
> 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?
static int resctrl_mkdir_event_configs(struct rdt_resource *r, struct kernfs_node *l3_mon_kn)
{
+ struct rftype *rft = rdtgroup_get_rftype_by_name("event_filter");
struct kernfs_node *kn_subdir, *kn_subdir2;
struct mon_evt *mevt;
int ret;
+ if (!rft)
+ return -ENOENT;
+
kn_subdir = kernfs_create_dir(l3_mon_kn, "event_configs", l3_mon_kn->mode, NULL);
if (IS_ERR(kn_subdir))
return PTR_ERR(kn_subdir);
@@ -2325,6 +2329,8 @@ static int resctrl_mkdir_event_configs(struct rdt_resource *r, struct kernfs_nod
return ret;
for_each_mon_event(mevt) {
+ umode_t saved_mode;
+
if (mevt->rid != r->rid || !mevt->enabled || !resctrl_is_mbm_event(mevt->evtid))
continue;
@@ -2338,25 +2344,14 @@ static int resctrl_mkdir_event_configs(struct rdt_resource *r, struct kernfs_nod
if (ret)
return ret;
+ saved_mode = rft->mode;
+ if (!resctrl_arch_is_evt_configurable(mevt->evtid, true))
+ rft->mode = 0444;
+
ret = rdtgroup_add_files(kn_subdir2, RFTYPE_ASSIGN_CONFIG);
+ rft->mode = saved_mode;
if (ret)
return ret;
Thanks,
Ben
>
>
> if (!resctrl_arch_is_evt_configurable(mevt->evtid, true)) {
> struct rftype *rft;
>
> rft = rdtgroup_get_rftype_by_name("event_filter");
> if (rft)
> rft->mode = 0444;
> }
>
> -Tony
> > 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? -Tony
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. Thanks, Ben
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
Hi Reinette, On 3/16/26 17:29, Reinette Chatre wrote: > 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(). Ok, this works if we shift to the rdt_resource::resctrl_mon::mbm_cntr_configurable property you suggest below as that is scoped to all mbm events. > > 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. Sure, we can consider them separately. > > 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. Seems reasonable. I'll give this a go. > > 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()? As we've already got mbm_cntr_assignable and mbm_assign_on_mkdir in resctrl_mon it won't seem out of place. I'll update this too. > > Reinette Thanks, Ben
© 2016 - 2026 Red Hat, Inc.