Re: [PATCH v4 2/7] x86,fs/resctrl: Make 'event_filter' files read only if they're not configurable

Ben Horgan
2026-03-27T17:15:08Z

Hi Reinette,

On 3/27/26 16:13, Reinette Chatre wrote:
> Hi Ben,
>
> nit: Since there is some precedent for resctrl files changing permissions
> after creation I think it would support this work to make clear that the
> permissions are set before the files are created and as such they are
> created with correct permissions.
>
> This could just be a simple: "Make" -> "Create" in subject.

Makes sense. I'll update to use "Create".

>
> On 3/26/26 10:25 AM, 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.
>
> <split here into separata paragraphs>

Ack

>
> The MPAM architecture has support
>> for configuring the memory bandwidth utilization (MBWU) counters to only
>> count reads or only count writes. However, In MPAM, this event filtering
>> support is optional in the hardware (and not yet implemented in the MPAM
>> driver) but MBM counter assignment is always possible for MPAM MBWU
>> counters.
>>
>> In order to support mbm_event mode with MPAM, make the 'event_filter' files
>
> "make" -> "create"

Ack

>
>> read only if the event configuration can't be changed. A user can still
>> chmod the file and so also return early with an error from
>> event_filter_write().
>
> I went back-and-forth a few times on whether we should add a permissions
> check to rdtgroup_file_write() to not call rft->write() (and thus event_filter_write())
> at all if the file is not writable. In the end I think this patch is ok since
> there is the last_cmd_status help to give insight into why user is unable to
> write to the file that may be writable under other circumstances. Any opinions
> welcome here.
>
>>
>> Introduce a new monitor property, mbm_cntr_configurable, to indicate
>> whether or not assignable MBM counters are configurable. On x86, set this
>> to true whenever mbm_cntr_assignable is true to keep existing behaviour.
>>
>> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
>> ---
>> Changes since v2:
>> Use property, mbm_cntr_configurable, rather than arch hook
>> Change the event_filter mode to read only in res_common_files[]
>> Add resctrl_file_mode_init() and use in resctrl_l3_mon_resource_init()
>> set mbm_cntr_configurable for x86 ABMC and mention in commit message
>> ---
>> arch/x86/kernel/cpu/resctrl/monitor.c | 1 +
>> fs/resctrl/internal.h | 2 ++
>> fs/resctrl/monitor.c | 7 +++++++
>> fs/resctrl/rdtgroup.c | 11 ++++++++++-
>> include/linux/resctrl.h | 16 +++++++++-------
>> 5 files changed, 29 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
>> index 9bd87bae4983..794a6fb175e4 100644
>> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
>> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
>> @@ -454,6 +454,7 @@ int __init rdt_get_l3_mon_config(struct rdt_resource *r)
>> (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL) ||
>> rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))) {
>> r->mon.mbm_cntr_assignable = true;
>> + r->mon.mbm_cntr_configurable = true;
>> cpuid_count(0x80000020, 5, &eax, &ebx, &ecx, &edx);
>> r->mon.num_mbm_cntrs = (ebx & GENMASK(15, 0)) + 1;
>> hw_res->mbm_cntr_assign_enabled = true;
>> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
>> index 1a9b29119f88..48af75b9dc85 100644
>> --- a/fs/resctrl/internal.h
>> +++ b/fs/resctrl/internal.h
>> @@ -408,6 +408,8 @@ void __check_limbo(struct rdt_l3_mon_domain *d, bool force_free);
>>
>> void resctrl_file_fflags_init(const char *config, unsigned long fflags);
>>
>> +void resctrl_file_mode_init(const char *config, umode_t mode);
>> +
>> void rdt_staged_configs_clear(void);
>>
>> bool closid_allocated(unsigned int closid);
>> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
>> index 49f3f6b846b2..8fec3dea33c3 100644
>> --- a/fs/resctrl/monitor.c
>> +++ b/fs/resctrl/monitor.c
>> @@ -1420,6 +1420,11 @@ ssize_t event_filter_write(struct kernfs_open_file *of, char *buf, size_t nbytes
>> ret = -EINVAL;
>> goto out_unlock;
>> }
>> + if (!r->mon.mbm_cntr_configurable) {
>> + rdt_last_cmd_puts("event_filter is not configurable\n");
>> + ret = -EPERM;
>> + goto out_unlock;
>> + }
>>
>> ret = resctrl_parse_mem_transactions(buf, &evt_cfg);
>> if (!ret && mevt->evt_cfg != evt_cfg) {
>> @@ -1884,6 +1889,8 @@ int resctrl_l3_mon_resource_init(void)
>> resctrl_file_fflags_init("available_mbm_cntrs",
>> RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
>> resctrl_file_fflags_init("event_filter", RFTYPE_ASSIGN_CONFIG);
>> + if (r->mon.mbm_cntr_configurable)
>> + resctrl_file_mode_init("event_filter", 0644);
>> resctrl_file_fflags_init("mbm_assign_on_mkdir", RFTYPE_MON_INFO |
>> RFTYPE_RES_CACHE);
>> resctrl_file_fflags_init("mbm_L3_assignments", RFTYPE_MON_BASE);
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index 4753841c2ca3..fa5712db3778 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -2020,7 +2020,7 @@ static struct rftype res_common_files[] = {
>> },
>> {
>> .name = "event_filter",
>> - .mode = 0644,
>> + .mode = 0444,
>> .kf_ops = &rdtgroup_kf_single_ops,
>> .seq_show = event_filter_show,
>> .write = event_filter_write,
>> @@ -2213,6 +2213,15 @@ void resctrl_file_fflags_init(const char *config, unsigned long fflags)
>> rft->fflags = fflags;
>> }
>>
>> +void resctrl_file_mode_init(const char *config, umode_t mode)
>> +{
>> + struct rftype *rft;
>> +
>> + rft = rdtgroup_get_rftype_by_name(config);
>> + if (rft)
>> + rft->mode = mode;
>> +}
>> +
>> /**
>> * rdtgroup_kn_mode_restrict - Restrict user access to named resctrl file
>> * @r: The resource group with which the file is associated.
>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index 006e57fd7ca5..06e8c72e8660 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -286,13 +286,14 @@ enum resctrl_schema_fmt {
>>
>> /**
>> * struct resctrl_mon - Monitoring related data of a resctrl resource.
>> - * @num_rmid: Number of RMIDs available.
>> - * @mbm_cfg_mask: Memory transactions that can be tracked when bandwidth
>> - * monitoring events can be configured.
>> - * @num_mbm_cntrs: Number of assignable counters.
>> - * @mbm_cntr_assignable:Is system capable of supporting counter assignment?
>> - * @mbm_assign_on_mkdir:True if counters should automatically be assigned to MBM
>> - * events of monitor groups created via mkdir.
>> + * @num_rmid: Number of RMIDs available.
>> + * @mbm_cfg_mask: Memory transactions that can be tracked when
>> + * bandwidth monitoring events can be configured.
>> + * @num_mbm_cntrs: Number of assignable counters.
>> + * @mbm_cntr_assignable: Is system capable of supporting counter assignment?
>> + * @mbm_assign_on_mkdir: True if counters should automatically be assigned to MBM
>> + * events of monitor groups created via mkdir.
>> + * @mbm_cntr_configurable: True if assignable counters are configurable.
>> */
>> struct resctrl_mon {
>> u32 num_rmid;
>> @@ -300,6 +301,7 @@ struct resctrl_mon {
>> int num_mbm_cntrs;
>> bool mbm_cntr_assignable;
>> bool mbm_assign_on_mkdir;
>> + bool mbm_cntr_configurable;
>> };
>>
>> /**
>
> The above looks good to me. The documentation does still specifically state that "event_filter"
> is read/write so it needs a change to match. For example, like something below but please feel
> free to improve:
>
> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
> index 3ec6b3b1b603..70a4ae89d8e1 100644
> --- a/Documentation/filesystems/resctrl.rst
> +++ b/Documentation/filesystems/resctrl.rst
> @@ -419,9 +419,9 @@ with the following files:
>
> Two MBM events are supported by default: mbm_local_bytes and mbm_total_bytes.
> Each MBM event's sub-directory contains a file named "event_filter" that is
> - used to view and modify which memory transactions the MBM event is configured
> - with. The file is accessible only when "mbm_event" counter assignment mode is
> - enabled.
> + used to view and (if writable) modify which memory transactions the MBM
> + event is configured with. The file is accessible only when "mbm_event" counter
> + assignment mode is enabled.
>
> List of memory transaction types supported:
>
> @@ -446,9 +446,8 @@ with the following files:
> # cat /sys/fs/resctrl/info/L3_MON/event_configs/mbm_local_bytes/event_filter
> local_reads,local_non_temporal_writes,local_reads_slow_memory
>
> - Modify the event configuration by writing to the "event_filter" file within
> - the "event_configs" directory. The read/write "event_filter" file contains the
> - configuration of the event that reflects which memory transactions are counted by it.
> + The memory transactions the MBM event is configured with can be changed
> + if "event_filter" is writable.

Thanks for the wording. It reads well to me.

Ben

>
> For example::
>
> Reinette
>
>
>
>