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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e9eb906f-d463-4c1e-9e15-5ed795fe5366@intel.com>
Date: Tue, 24 Jun 2025 14:28:42 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Babu Moger <babu.moger@....com>, <corbet@....net>, <tony.luck@...el.com>,
	<Dave.Martin@....com>, <james.morse@....com>, <tglx@...utronix.de>,
	<mingo@...hat.com>, <bp@...en8.de>, <dave.hansen@...ux.intel.com>
CC: <x86@...nel.org>, <hpa@...or.com>, <akpm@...ux-foundation.org>,
	<rostedt@...dmis.org>, <paulmck@...nel.org>, <thuth@...hat.com>,
	<ardb@...nel.org>, <gregkh@...uxfoundation.org>, <seanjc@...gle.com>,
	<thomas.lendacky@....com>, <pawan.kumar.gupta@...ux.intel.com>,
	<manali.shukla@....com>, <perry.yuan@....com>, <kai.huang@...el.com>,
	<peterz@...radead.org>, <xiaoyao.li@...el.com>, <kan.liang@...ux.intel.com>,
	<mario.limonciello@....com>, <xin3.li@...el.com>, <gautham.shenoy@....com>,
	<xin@...or.com>, <chang.seok.bae@...el.com>, <fenghuay@...dia.com>,
	<peternewman@...gle.com>, <maciej.wieczor-retman@...el.com>,
	<eranian@...gle.com>, <linux-doc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v14 02/32] x86,fs/resctrl: Consolidate monitor event
 descriptions

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