There are currently only three monitor events, all associated with
the RDT_RESOURCE_L3 resource. Growing support for additional events
will be easier with some restructuring to have a single point in
file system code where all attributes of all events are defined.

Place all event descriptions into an array mon_event_all[]. Doing
this has the beneficial side effect of removing the need for
rdt_resource::evt_list.

Add resctrl_event_id::QOS_FIRST_EVENT for a lower bound on range
checks for event ids and as the starting index to scan mon_event_all[].

Drop the code that builds evt_list and change the two places where
the list is scanned to scan mon_event_all[] instead using a new
helper macro for_each_mon_event().

Architecture code now informs file system code which events are
available with resctrl_enable_mon_event().

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 include/linux/resctrl.h            |  4 +-
 include/linux/resctrl_types.h      | 12 ++++--
 fs/resctrl/internal.h              | 13 ++++--
 arch/x86/kernel/cpu/resctrl/core.c | 12 ++++--
 fs/resctrl/monitor.c               | 63 +++++++++++++++---------------
 fs/resctrl/rdtgroup.c              | 11 +++---
 6 files changed, 66 insertions(+), 49 deletions(-)

diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 9ba771f2ddea..1c87e1ed235f 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -269,7 +269,6 @@ enum resctrl_schema_fmt {
  * @mon_domains:	RCU list of all monitor domains for this resource
  * @name:		Name to use in "schemata" file.
  * @schema_fmt:		Which format string and parser is used for this schema.
- * @evt_list:		List of monitoring events
  * @mbm_cfg_mask:	Bandwidth sources that can be tracked when bandwidth
  *			monitoring events can be configured.
  * @cdp_capable:	Is the CDP feature available on this resource
@@ -287,7 +286,6 @@ struct rdt_resource {
 	struct list_head	mon_domains;
 	char			*name;
 	enum resctrl_schema_fmt	schema_fmt;
-	struct list_head	evt_list;
 	unsigned int		mbm_cfg_mask;
 	bool			cdp_capable;
 };
@@ -372,6 +370,8 @@ u32 resctrl_arch_get_num_closid(struct rdt_resource *r);
 u32 resctrl_arch_system_num_rmid_idx(void);
 int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid);
 
+void resctrl_enable_mon_event(enum resctrl_event_id eventid);
+
 bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt);
 
 /**
diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
index a25fb9c4070d..2dadbc54e4b3 100644
--- a/include/linux/resctrl_types.h
+++ b/include/linux/resctrl_types.h
@@ -34,11 +34,15 @@
 /* Max event bits supported */
 #define MAX_EVT_CONFIG_BITS		GENMASK(6, 0)
 
-/*
- * Event IDs, the values match those used to program IA32_QM_EVTSEL before
- * reading IA32_QM_CTR on RDT systems.
- */
+/* Event IDs */
 enum resctrl_event_id {
+	/* Must match value of first event below */
+	QOS_FIRST_EVENT			= 0x01,
+
+	/*
+	 * These values match those used to program IA32_QM_EVTSEL before
+	 * reading IA32_QM_CTR on RDT systems.
+	 */
 	QOS_L3_OCCUP_EVENT_ID		= 0x01,
 	QOS_L3_MBM_TOTAL_EVENT_ID	= 0x02,
 	QOS_L3_MBM_LOCAL_EVENT_ID	= 0x03,
diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
index 9a8cf6f11151..71963255ad36 100644
--- a/fs/resctrl/internal.h
+++ b/fs/resctrl/internal.h
@@ -52,19 +52,26 @@ static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
 }
 
 /**
- * struct mon_evt - Entry in the event list of a resource
+ * struct mon_evt - Description of a monitor event
  * @evtid:		event id
+ * @rid:		index of the resource for this event
  * @name:		name of the event
  * @configurable:	true if the event is configurable
- * @list:		entry in &rdt_resource->evt_list
+ * @enabled:		true if the event is enabled
  */
 struct mon_evt {
 	enum resctrl_event_id	evtid;
+	enum resctrl_res_level	rid;
 	char			*name;
 	bool			configurable;
-	struct list_head	list;
+	bool			enabled;
 };
 
+extern struct mon_evt mon_event_all[QOS_NUM_EVENTS];
+
+#define for_each_mon_event(mevt) for (mevt = &mon_event_all[QOS_FIRST_EVENT];	\
+				      mevt < &mon_event_all[QOS_NUM_EVENTS]; mevt++)
+
 /**
  * struct mon_data - Monitoring details for each event file.
  * @list:            Member of the global @mon_data_kn_priv_list list.
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 7109cbfcad4f..7b235b7691aa 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -862,12 +862,18 @@ static __init bool get_rdt_mon_resources(void)
 {
 	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
 
-	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
+	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
+		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID);
 		rdt_mon_features |= (1 << QOS_L3_OCCUP_EVENT_ID);
-	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL))
+	}
+	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {
+		resctrl_enable_mon_event(QOS_L3_MBM_TOTAL_EVENT_ID);
 		rdt_mon_features |= (1 << QOS_L3_MBM_TOTAL_EVENT_ID);
-	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))
+	}
+	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL)) {
+		resctrl_enable_mon_event(QOS_L3_MBM_LOCAL_EVENT_ID);
 		rdt_mon_features |= (1 << QOS_L3_MBM_LOCAL_EVENT_ID);
+	}
 
 	if (!rdt_mon_features)
 		return false;
diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index bde2801289d3..90093e54a279 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -842,38 +842,39 @@ static void dom_data_exit(struct rdt_resource *r)
 	mutex_unlock(&rdtgroup_mutex);
 }
 
-static struct mon_evt llc_occupancy_event = {
-	.name		= "llc_occupancy",
-	.evtid		= QOS_L3_OCCUP_EVENT_ID,
-};
-
-static struct mon_evt mbm_total_event = {
-	.name		= "mbm_total_bytes",
-	.evtid		= QOS_L3_MBM_TOTAL_EVENT_ID,
-};
-
-static struct mon_evt mbm_local_event = {
-	.name		= "mbm_local_bytes",
-	.evtid		= QOS_L3_MBM_LOCAL_EVENT_ID,
-};
-
 /*
- * Initialize the event list for the resource.
- *
- * Note that MBM events are also part of RDT_RESOURCE_L3 resource
- * because as per the SDM the total and local memory bandwidth
- * are enumerated as part of L3 monitoring.
+ * All available events. Architecture code marks the ones that
+ * are supported by a system using resctrl_enable_mon_event()
+ * to set .enabled.
  */
-static void l3_mon_evt_init(struct rdt_resource *r)
+struct mon_evt mon_event_all[QOS_NUM_EVENTS] = {
+	[QOS_L3_OCCUP_EVENT_ID] = {
+		.name	= "llc_occupancy",
+		.evtid	= QOS_L3_OCCUP_EVENT_ID,
+		.rid	= RDT_RESOURCE_L3,
+	},
+	[QOS_L3_MBM_TOTAL_EVENT_ID] = {
+		.name	= "mbm_total_bytes",
+		.evtid	= QOS_L3_MBM_TOTAL_EVENT_ID,
+		.rid	= RDT_RESOURCE_L3,
+	},
+	[QOS_L3_MBM_LOCAL_EVENT_ID] = {
+		.name	= "mbm_local_bytes",
+		.evtid	= QOS_L3_MBM_LOCAL_EVENT_ID,
+		.rid	= RDT_RESOURCE_L3,
+	},
+};
+
+void resctrl_enable_mon_event(enum resctrl_event_id eventid)
 {
-	INIT_LIST_HEAD(&r->evt_list);
+	if (WARN_ON_ONCE(eventid < QOS_FIRST_EVENT || eventid >= QOS_NUM_EVENTS))
+		return;
+	if (mon_event_all[eventid].enabled) {
+		pr_warn("Duplicate enable for event %d\n", eventid);
+		return;
+	}
 
-	if (resctrl_arch_is_llc_occupancy_enabled())
-		list_add_tail(&llc_occupancy_event.list, &r->evt_list);
-	if (resctrl_arch_is_mbm_total_enabled())
-		list_add_tail(&mbm_total_event.list, &r->evt_list);
-	if (resctrl_arch_is_mbm_local_enabled())
-		list_add_tail(&mbm_local_event.list, &r->evt_list);
+	mon_event_all[eventid].enabled = true;
 }
 
 /**
@@ -900,15 +901,13 @@ int resctrl_mon_resource_init(void)
 	if (ret)
 		return ret;
 
-	l3_mon_evt_init(r);
-
 	if (resctrl_arch_is_evt_configurable(QOS_L3_MBM_TOTAL_EVENT_ID)) {
-		mbm_total_event.configurable = true;
+		mon_event_all[QOS_L3_MBM_TOTAL_EVENT_ID].configurable = true;
 		resctrl_file_fflags_init("mbm_total_bytes_config",
 					 RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
 	}
 	if (resctrl_arch_is_evt_configurable(QOS_L3_MBM_LOCAL_EVENT_ID)) {
-		mbm_local_event.configurable = true;
+		mon_event_all[QOS_L3_MBM_LOCAL_EVENT_ID].configurable = true;
 		resctrl_file_fflags_init("mbm_local_bytes_config",
 					 RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
 	}
diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index cc37f58b47dd..e4e4b6af5147 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -1150,7 +1150,9 @@ static int rdt_mon_features_show(struct kernfs_open_file *of,
 	struct rdt_resource *r = rdt_kn_parent_priv(of->kn);
 	struct mon_evt *mevt;
 
-	list_for_each_entry(mevt, &r->evt_list, list) {
+	for_each_mon_event(mevt) {
+		if (mevt->rid != r->rid || !mevt->enabled)
+			continue;
 		seq_printf(seq, "%s\n", mevt->name);
 		if (mevt->configurable)
 			seq_printf(seq, "%s_config\n", mevt->name);
@@ -3055,10 +3057,9 @@ static int mon_add_all_files(struct kernfs_node *kn, struct rdt_mon_domain *d,
 	struct mon_evt *mevt;
 	int ret, domid;
 
-	if (WARN_ON(list_empty(&r->evt_list)))
-		return -EPERM;
-
-	list_for_each_entry(mevt, &r->evt_list, list) {
+	for_each_mon_event(mevt) {
+		if (mevt->rid != r->rid || !mevt->enabled)
+			continue;
 		domid = do_sum ? d->ci->id : d->hdr.id;
 		priv = mon_get_kn_priv(r->rid, domid, mevt, do_sum);
 		if (WARN_ON_ONCE(!priv))
-- 
2.49.0

On 6/4/2025 2:22 PM, Tony Luck wrote:
> There are currently only three monitor events, all associated with
> the RDT_RESOURCE_L3 resource. Growing support for additional events
> will be easier with some restructuring to have a single point in
> file system code where all attributes of all events are defined.
>
> Place all event descriptions into an array mon_event_all[]. Doing
> this has the beneficial side effect of removing the need for
> rdt_resource::evt_list.
>
> Add resctrl_event_id::QOS_FIRST_EVENT for a lower bound on range
> checks for event ids and as the starting index to scan mon_event_all[].
>
> Drop the code that builds evt_list and change the two places where
> the list is scanned to scan mon_event_all[] instead using a new
> helper macro for_each_mon_event().
>
> Architecture code now informs file system code which events are
> available with resctrl_enable_mon_event().
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>   include/linux/resctrl.h            |  4 +-
>   include/linux/resctrl_types.h      | 12 ++++--
>   fs/resctrl/internal.h              | 13 ++++--
>   arch/x86/kernel/cpu/resctrl/core.c | 12 ++++--
>   fs/resctrl/monitor.c               | 63 +++++++++++++++---------------
>   fs/resctrl/rdtgroup.c              | 11 +++---
>   6 files changed, 66 insertions(+), 49 deletions(-)
>
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 9ba771f2ddea..1c87e1ed235f 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -269,7 +269,6 @@ enum resctrl_schema_fmt {
>    * @mon_domains:	RCU list of all monitor domains for this resource
>    * @name:		Name to use in "schemata" file.
>    * @schema_fmt:		Which format string and parser is used for this schema.
> - * @evt_list:		List of monitoring events
>    * @mbm_cfg_mask:	Bandwidth sources that can be tracked when bandwidth
>    *			monitoring events can be configured.
>    * @cdp_capable:	Is the CDP feature available on this resource
> @@ -287,7 +286,6 @@ struct rdt_resource {
>   	struct list_head	mon_domains;
>   	char			*name;
>   	enum resctrl_schema_fmt	schema_fmt;
> -	struct list_head	evt_list;
>   	unsigned int		mbm_cfg_mask;
>   	bool			cdp_capable;
>   };
> @@ -372,6 +370,8 @@ u32 resctrl_arch_get_num_closid(struct rdt_resource *r);
>   u32 resctrl_arch_system_num_rmid_idx(void);
>   int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid);
>   
> +void resctrl_enable_mon_event(enum resctrl_event_id eventid);
> +
>   bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt);
>   
>   /**
> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
> index a25fb9c4070d..2dadbc54e4b3 100644
> --- a/include/linux/resctrl_types.h
> +++ b/include/linux/resctrl_types.h
> @@ -34,11 +34,15 @@
>   /* Max event bits supported */
>   #define MAX_EVT_CONFIG_BITS		GENMASK(6, 0)
>   
> -/*
> - * Event IDs, the values match those used to program IA32_QM_EVTSEL before
> - * reading IA32_QM_CTR on RDT systems.
> - */
> +/* Event IDs */
>   enum resctrl_event_id {
> +	/* Must match value of first event below */
> +	QOS_FIRST_EVENT			= 0x01,
> +
> +	/*
> +	 * These values match those used to program IA32_QM_EVTSEL before
> +	 * reading IA32_QM_CTR on RDT systems.
> +	 */
>   	QOS_L3_OCCUP_EVENT_ID		= 0x01,
>   	QOS_L3_MBM_TOTAL_EVENT_ID	= 0x02,
>   	QOS_L3_MBM_LOCAL_EVENT_ID	= 0x03,
> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
> index 9a8cf6f11151..71963255ad36 100644
> --- a/fs/resctrl/internal.h
> +++ b/fs/resctrl/internal.h
> @@ -52,19 +52,26 @@ static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
>   }
>   
>   /**
> - * struct mon_evt - Entry in the event list of a resource
> + * struct mon_evt - Description of a monitor event
>    * @evtid:		event id
> + * @rid:		index of the resource for this event
>    * @name:		name of the event
>    * @configurable:	true if the event is configurable
> - * @list:		entry in &rdt_resource->evt_list
> + * @enabled:		true if the event is enabled
>    */
>   struct mon_evt {
>   	enum resctrl_event_id	evtid;
> +	enum resctrl_res_level	rid;
>   	char			*name;
>   	bool			configurable;
> -	struct list_head	list;
> +	bool			enabled;
>   };
>   
> +extern struct mon_evt mon_event_all[QOS_NUM_EVENTS];
> +
> +#define for_each_mon_event(mevt) for (mevt = &mon_event_all[QOS_FIRST_EVENT];	\
> +				      mevt < &mon_event_all[QOS_NUM_EVENTS]; mevt++)
> +
>   /**
>    * struct mon_data - Monitoring details for each event file.
>    * @list:            Member of the global @mon_data_kn_priv_list list.
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 7109cbfcad4f..7b235b7691aa 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -862,12 +862,18 @@ static __init bool get_rdt_mon_resources(void)
>   {
>   	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
>   
> -	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
> +	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
> +		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID);
>   		rdt_mon_features |= (1 << QOS_L3_OCCUP_EVENT_ID);
> -	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL))
> +	}
> +	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {
> +		resctrl_enable_mon_event(QOS_L3_MBM_TOTAL_EVENT_ID);
>   		rdt_mon_features |= (1 << QOS_L3_MBM_TOTAL_EVENT_ID);
> -	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))
> +	}
> +	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL)) {
> +		resctrl_enable_mon_event(QOS_L3_MBM_LOCAL_EVENT_ID);
>   		rdt_mon_features |= (1 << QOS_L3_MBM_LOCAL_EVENT_ID);
> +	}
>   
>   	if (!rdt_mon_features)
>   		return false;
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> index bde2801289d3..90093e54a279 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -842,38 +842,39 @@ static void dom_data_exit(struct rdt_resource *r)
>   	mutex_unlock(&rdtgroup_mutex);
>   }
>   
> -static struct mon_evt llc_occupancy_event = {
> -	.name		= "llc_occupancy",
> -	.evtid		= QOS_L3_OCCUP_EVENT_ID,
> -};
> -
> -static struct mon_evt mbm_total_event = {
> -	.name		= "mbm_total_bytes",
> -	.evtid		= QOS_L3_MBM_TOTAL_EVENT_ID,
> -};
> -
> -static struct mon_evt mbm_local_event = {
> -	.name		= "mbm_local_bytes",
> -	.evtid		= QOS_L3_MBM_LOCAL_EVENT_ID,
> -};
> -
>   /*
> - * Initialize the event list for the resource.
> - *
> - * Note that MBM events are also part of RDT_RESOURCE_L3 resource
> - * because as per the SDM the total and local memory bandwidth
> - * are enumerated as part of L3 monitoring.
> + * All available events. Architecture code marks the ones that
> + * are supported by a system using resctrl_enable_mon_event()
> + * to set .enabled.
>    */
> -static void l3_mon_evt_init(struct rdt_resource *r)
> +struct mon_evt mon_event_all[QOS_NUM_EVENTS] = {
> +	[QOS_L3_OCCUP_EVENT_ID] = {
> +		.name	= "llc_occupancy",
> +		.evtid	= QOS_L3_OCCUP_EVENT_ID,
> +		.rid	= RDT_RESOURCE_L3,
> +	},
> +	[QOS_L3_MBM_TOTAL_EVENT_ID] = {
> +		.name	= "mbm_total_bytes",
> +		.evtid	= QOS_L3_MBM_TOTAL_EVENT_ID,
> +		.rid	= RDT_RESOURCE_L3,
> +	},
> +	[QOS_L3_MBM_LOCAL_EVENT_ID] = {
> +		.name	= "mbm_local_bytes",
> +		.evtid	= QOS_L3_MBM_LOCAL_EVENT_ID,
> +		.rid	= RDT_RESOURCE_L3,
> +	},
> +};

As we start adding many more events to this struct(including region 
aware specific events), this this becomes too stressful on the eyes to 
read.....can you consider simplifying this with #define something like 
below in your next version? NOTE: For the feature that I am adding along 
with Chen Yu, we started to define a macro as shown below.

#define MON_EVENT(_id, _name, _res) \
     [_id] = {                      \
         .name  = _name,           \
         .evtid = _id,             \
         .rid   = _res,            \
     }

Above #define can to into include/linux/resctrl_types.h file and the 
above code reduces to using MON_EVENT as shown below.

struct mon_evt mon_event_all[QOS_NUM_EVENTS] = {
     MON_EVENT(QOS_L3_OCCUP_EVENT_ID,    "llc_occupancy", RDT_RESOURCE_L3),
     MON_EVENT(QOS_L3_MBM_TOTAL_EVENT_ID, "mbm_total_bytes", 
RDT_RESOURCE_L3),
     MON_EVENT(QOS_L3_MBM_LOCAL_EVENT_ID, "mbm_local_bytes", 
RDT_RESOURCE_L3),
};

> +
> +void resctrl_enable_mon_event(enum resctrl_event_id eventid)
>   {
> -	INIT_LIST_HEAD(&r->evt_list);
> +	if (WARN_ON_ONCE(eventid < QOS_FIRST_EVENT || eventid >= QOS_NUM_EVENTS))
> +		return;
> +	if (mon_event_all[eventid].enabled) {
> +		pr_warn("Duplicate enable for event %d\n", eventid);
> +		return;
> +	}
>   
> -	if (resctrl_arch_is_llc_occupancy_enabled())
> -		list_add_tail(&llc_occupancy_event.list, &r->evt_list);
> -	if (resctrl_arch_is_mbm_total_enabled())
> -		list_add_tail(&mbm_total_event.list, &r->evt_list);
> -	if (resctrl_arch_is_mbm_local_enabled())
> -		list_add_tail(&mbm_local_event.list, &r->evt_list);
> +	mon_event_all[eventid].enabled = true;
>   }
>   
>   /**
> @@ -900,15 +901,13 @@ int resctrl_mon_resource_init(void)
>   	if (ret)
>   		return ret;
>   
> -	l3_mon_evt_init(r);
> -
>   	if (resctrl_arch_is_evt_configurable(QOS_L3_MBM_TOTAL_EVENT_ID)) {
> -		mbm_total_event.configurable = true;
> +		mon_event_all[QOS_L3_MBM_TOTAL_EVENT_ID].configurable = true;
>   		resctrl_file_fflags_init("mbm_total_bytes_config",
>   					 RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
>   	}
>   	if (resctrl_arch_is_evt_configurable(QOS_L3_MBM_LOCAL_EVENT_ID)) {
> -		mbm_local_event.configurable = true;
> +		mon_event_all[QOS_L3_MBM_LOCAL_EVENT_ID].configurable = true;
>   		resctrl_file_fflags_init("mbm_local_bytes_config",
>   					 RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
>   	}
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index cc37f58b47dd..e4e4b6af5147 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -1150,7 +1150,9 @@ static int rdt_mon_features_show(struct kernfs_open_file *of,
>   	struct rdt_resource *r = rdt_kn_parent_priv(of->kn);
>   	struct mon_evt *mevt;
>   
> -	list_for_each_entry(mevt, &r->evt_list, list) {
> +	for_each_mon_event(mevt) {
> +		if (mevt->rid != r->rid || !mevt->enabled)
> +			continue;
>   		seq_printf(seq, "%s\n", mevt->name);
>   		if (mevt->configurable)
>   			seq_printf(seq, "%s_config\n", mevt->name);
> @@ -3055,10 +3057,9 @@ static int mon_add_all_files(struct kernfs_node *kn, struct rdt_mon_domain *d,
>   	struct mon_evt *mevt;
>   	int ret, domid;
>   
> -	if (WARN_ON(list_empty(&r->evt_list)))
> -		return -EPERM;
> -
> -	list_for_each_entry(mevt, &r->evt_list, list) {
> +	for_each_mon_event(mevt) {
> +		if (mevt->rid != r->rid || !mevt->enabled)
> +			continue;
>   		domid = do_sum ? d->ci->id : d->hdr.id;
>   		priv = mon_get_kn_priv(r->rid, domid, mevt, do_sum);
>   		if (WARN_ON_ONCE(!priv))

Hi Anil,

On 6/4/25 3:21 PM, Keshavamurthy, Anil S wrote:
> 
> On 6/4/2025 2:22 PM, Tony Luck wrote:

...

>> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
>> index 9a8cf6f11151..71963255ad36 100644
>> --- a/fs/resctrl/internal.h
>> +++ b/fs/resctrl/internal.h
>> @@ -52,19 +52,26 @@ static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
>>   }
>>     /**
>> - * struct mon_evt - Entry in the event list of a resource
>> + * struct mon_evt - Description of a monitor event
>>    * @evtid:        event id
>> + * @rid:        index of the resource for this event
>>    * @name:        name of the event
>>    * @configurable:    true if the event is configurable
>> - * @list:        entry in &rdt_resource->evt_list
>> + * @enabled:        true if the event is enabled
>>    */
>>   struct mon_evt {
>>       enum resctrl_event_id    evtid;
>> +    enum resctrl_res_level    rid;
>>       char            *name;
>>       bool            configurable;
>> -    struct list_head    list;
>> +    bool            enabled;
>>   };
>>   +extern struct mon_evt mon_event_all[QOS_NUM_EVENTS];
>> +
>> +#define for_each_mon_event(mevt) for (mevt = &mon_event_all[QOS_FIRST_EVENT];    \
>> +                      mevt < &mon_event_all[QOS_NUM_EVENTS]; mevt++)
>> +
>>   /**
>>    * struct mon_data - Monitoring details for each event file.
>>    * @list:            Member of the global @mon_data_kn_priv_list list.

...

>> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
>> index bde2801289d3..90093e54a279 100644
>> --- a/fs/resctrl/monitor.c
>> +++ b/fs/resctrl/monitor.c
>> @@ -842,38 +842,39 @@ static void dom_data_exit(struct rdt_resource *r)
>>       mutex_unlock(&rdtgroup_mutex);
>>   }
>>   -static struct mon_evt llc_occupancy_event = {
>> -    .name        = "llc_occupancy",
>> -    .evtid        = QOS_L3_OCCUP_EVENT_ID,
>> -};
>> -
>> -static struct mon_evt mbm_total_event = {
>> -    .name        = "mbm_total_bytes",
>> -    .evtid        = QOS_L3_MBM_TOTAL_EVENT_ID,
>> -};
>> -
>> -static struct mon_evt mbm_local_event = {
>> -    .name        = "mbm_local_bytes",
>> -    .evtid        = QOS_L3_MBM_LOCAL_EVENT_ID,
>> -};
>> -
>>   /*
>> - * Initialize the event list for the resource.
>> - *
>> - * Note that MBM events are also part of RDT_RESOURCE_L3 resource
>> - * because as per the SDM the total and local memory bandwidth
>> - * are enumerated as part of L3 monitoring.
>> + * All available events. Architecture code marks the ones that
>> + * are supported by a system using resctrl_enable_mon_event()
>> + * to set .enabled.
>>    */
>> -static void l3_mon_evt_init(struct rdt_resource *r)
>> +struct mon_evt mon_event_all[QOS_NUM_EVENTS] = {
>> +    [QOS_L3_OCCUP_EVENT_ID] = {
>> +        .name    = "llc_occupancy",
>> +        .evtid    = QOS_L3_OCCUP_EVENT_ID,
>> +        .rid    = RDT_RESOURCE_L3,
>> +    },
>> +    [QOS_L3_MBM_TOTAL_EVENT_ID] = {
>> +        .name    = "mbm_total_bytes",
>> +        .evtid    = QOS_L3_MBM_TOTAL_EVENT_ID,
>> +        .rid    = RDT_RESOURCE_L3,
>> +    },
>> +    [QOS_L3_MBM_LOCAL_EVENT_ID] = {
>> +        .name    = "mbm_local_bytes",
>> +        .evtid    = QOS_L3_MBM_LOCAL_EVENT_ID,
>> +        .rid    = RDT_RESOURCE_L3,
>> +    },
>> +};
> 
> As we start adding many more events to this struct(including region aware specific events), this this becomes too stressful on the eyes to read.....can you consider simplifying this with #define something like below in your next version? NOTE: For the feature that I am adding along with Chen Yu, we started to define a macro as shown below.
> 
> #define MON_EVENT(_id, _name, _res) \
>     [_id] = {                      \
>         .name  = _name,           \
>         .evtid = _id,             \
>         .rid   = _res,            \
>     }
> 
> Above #define can to into include/linux/resctrl_types.h file and the above code reduces to using MON_EVENT as shown below.

Any reason why the events need to leak outside resctrl fs? At the moment it is kept
inside resctrl fs with helpers for only those properties (not descriptions!) that
can/should be set by archs. Enabling arch full control of the event is not ideal
since many of the properties are required to be the same across all archs and
dictate the user interface that is ABI.

Reinette

On 6/4/2025 3:30 PM, Reinette Chatre wrote:
> Hi Anil,
>
> On 6/4/25 3:21 PM, Keshavamurthy, Anil S wrote:
>> On 6/4/2025 2:22 PM, Tony Luck wrote:
> ...
>
>>> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
>>> index 9a8cf6f11151..71963255ad36 100644
>>> --- a/fs/resctrl/internal.h
>>> +++ b/fs/resctrl/internal.h
>>> @@ -52,19 +52,26 @@ static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
>>>    }
>>>      /**
>>> - * struct mon_evt - Entry in the event list of a resource
>>> + * struct mon_evt - Description of a monitor event
>>>     * @evtid:        event id
>>> + * @rid:        index of the resource for this event
>>>     * @name:        name of the event
>>>     * @configurable:    true if the event is configurable
>>> - * @list:        entry in &rdt_resource->evt_list
>>> + * @enabled:        true if the event is enabled
>>>     */
>>>    struct mon_evt {
>>>        enum resctrl_event_id    evtid;
>>> +    enum resctrl_res_level    rid;
>>>        char            *name;
>>>        bool            configurable;
>>> -    struct list_head    list;
>>> +    bool            enabled;
>>>    };
>>>    +extern struct mon_evt mon_event_all[QOS_NUM_EVENTS];
>>> +
>>> +#define for_each_mon_event(mevt) for (mevt = &mon_event_all[QOS_FIRST_EVENT];    \
>>> +                      mevt < &mon_event_all[QOS_NUM_EVENTS]; mevt++)
>>> +
>>>    /**
>>>     * struct mon_data - Monitoring details for each event file.
>>>     * @list:            Member of the global @mon_data_kn_priv_list list.
> ...
>
>>> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
>>> index bde2801289d3..90093e54a279 100644
>>> --- a/fs/resctrl/monitor.c
>>> +++ b/fs/resctrl/monitor.c
>>> @@ -842,38 +842,39 @@ static void dom_data_exit(struct rdt_resource *r)
>>>        mutex_unlock(&rdtgroup_mutex);
>>>    }
>>>    -static struct mon_evt llc_occupancy_event = {
>>> -    .name        = "llc_occupancy",
>>> -    .evtid        = QOS_L3_OCCUP_EVENT_ID,
>>> -};
>>> -
>>> -static struct mon_evt mbm_total_event = {
>>> -    .name        = "mbm_total_bytes",
>>> -    .evtid        = QOS_L3_MBM_TOTAL_EVENT_ID,
>>> -};
>>> -
>>> -static struct mon_evt mbm_local_event = {
>>> -    .name        = "mbm_local_bytes",
>>> -    .evtid        = QOS_L3_MBM_LOCAL_EVENT_ID,
>>> -};
>>> -
>>>    /*
>>> - * Initialize the event list for the resource.
>>> - *
>>> - * Note that MBM events are also part of RDT_RESOURCE_L3 resource
>>> - * because as per the SDM the total and local memory bandwidth
>>> - * are enumerated as part of L3 monitoring.
>>> + * All available events. Architecture code marks the ones that
>>> + * are supported by a system using resctrl_enable_mon_event()
>>> + * to set .enabled.
>>>     */
>>> -static void l3_mon_evt_init(struct rdt_resource *r)
>>> +struct mon_evt mon_event_all[QOS_NUM_EVENTS] = {
>>> +    [QOS_L3_OCCUP_EVENT_ID] = {
>>> +        .name    = "llc_occupancy",
>>> +        .evtid    = QOS_L3_OCCUP_EVENT_ID,
>>> +        .rid    = RDT_RESOURCE_L3,
>>> +    },
>>> +    [QOS_L3_MBM_TOTAL_EVENT_ID] = {
>>> +        .name    = "mbm_total_bytes",
>>> +        .evtid    = QOS_L3_MBM_TOTAL_EVENT_ID,
>>> +        .rid    = RDT_RESOURCE_L3,
>>> +    },
>>> +    [QOS_L3_MBM_LOCAL_EVENT_ID] = {
>>> +        .name    = "mbm_local_bytes",
>>> +        .evtid    = QOS_L3_MBM_LOCAL_EVENT_ID,
>>> +        .rid    = RDT_RESOURCE_L3,
>>> +    },
>>> +};
>> As we start adding many more events to this struct(including region aware specific events), this this becomes too stressful on the eyes to read.....can you consider simplifying this with #define something like below in your next version? NOTE: For the feature that I am adding along with Chen Yu, we started to define a macro as shown below.
>>
>> #define MON_EVENT(_id, _name, _res) \
>>      [_id] = {                      \
>>          .name  = _name,           \
>>          .evtid = _id,             \
>>          .rid   = _res,            \
>>      }
>>
>> Above #define can to into include/linux/resctrl_types.h file and the above code reduces to using MON_EVENT as shown below.
> Any reason why the events need to leak outside resctrl fs? At the moment it is kept
> inside resctrl fs with helpers for only those properties (not descriptions!) that
> can/should be set by archs. Enabling arch full control of the event is not ideal
> since many of the properties are required to be the same across all archs and
> dictate the user interface that is ABI.
>
> Reinette

Sorry, I just picked a random header file there....ignore that part but 
my real suggestion was to reduce the lines when declaring "struct 
mon_evt mon_event_all[]" with above #define so more events can fit in 
one screen when viewing the code.

> Sorry, I just picked a random header file there....ignore that part but 
> my real suggestion was to reduce the lines when declaring "struct 
> mon_evt mon_event_all[]" with above #define so more events can fit in 
> one screen when viewing the code.

The macro can go into fs/resctrl/monitor.c ... it isn’t needed in any
other file.

Sounds like a good idea.

-Tony