Re: [PATCH v14 02/32] x86,fs/resctrl: Consolidate monitor event descriptions

[Reinette Chatre]

Hi Babu/Tony,

On 6/13/25 2:04 PM, Babu Moger wrote:
> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
> index 0a1eedba2b03..20e2c45cea64 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

nit: I still think "Properties" is more appropriate.

>   * @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++)
> +

>From what I can tell this series does not build on some core changes
made by this patch:
- note that resource ID is added to struct mon_evt and the events
  are *removed* from the evt_list associated with the resource. I'll try to point
  out where I see it but this series still behaves as though it is traversing
  evt_list associated with the resource. Take for example
  patch #24 "fs/resctrl: Add event configuration directory under info/L3_MON/":
  resctrl_mkdir_counter_configs() traverses mon_event_all[] that, after this
  patch, contains all events for *all* resources, yet resctrl_mkdir_counter_configs(),
  even though it has a struct rdt_resource as parameter, assumes that all events are
  associated its resource but there is no checking to enforce this. 
- note the new for_each_mon_event() above. This should be used throughout
  instead of open-coding the loop because the array starts at index 0 but
  the first valid entry is at index 1. The above macro makes this easier to
  get right.

Reinette