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

[Ben Horgan <ben.horgan@arm.com>]

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