[PATCH v5 06/12] x86/resctrl: Introduce data structure to support monitor configuration

Babu Moger posted 12 patches 3 years, 6 months ago
There is a newer version of this series
[PATCH v5 06/12] x86/resctrl: Introduce data structure to support monitor configuration
Posted by Babu Moger 3 years, 6 months ago
Add couple of fields in mon_evt to support Bandwidth Monitoring Event
Configuratio (BMEC) and also update the "mon_features".

The sysfs file "mon_features" will display the monitor configuration if
supported.

Before the change.
	$cat /sys/fs/resctrl/info/L3_MON/mon_features
	llc_occupancy
	mbm_total_bytes
	mbm_local_bytes

After the change if BMEC is supported.
	$cat /sys/fs/resctrl/info/L3_MON/mon_features
	llc_occupancy
	mbm_total_bytes
	mbm_total_config
	mbm_local_bytes
	mbm_local_config

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/core.c     |    3 ++-
 arch/x86/kernel/cpu/resctrl/internal.h |    6 +++++-
 arch/x86/kernel/cpu/resctrl/monitor.c  |    9 ++++++++-
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |    5 ++++-
 4 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 56c96607259c..513e6a00f58e 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -849,6 +849,7 @@ static __init bool get_rdt_alloc_resources(void)
 static __init bool get_rdt_mon_resources(void)
 {
 	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
+	bool mon_configurable = rdt_cpu_has(X86_FEATURE_BMEC);
 
 	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
 		rdt_mon_features |= (1 << QOS_L3_OCCUP_EVENT_ID);
@@ -860,7 +861,7 @@ static __init bool get_rdt_mon_resources(void)
 	if (!rdt_mon_features)
 		return false;
 
-	return !rdt_get_mon_l3_config(r);
+	return !rdt_get_mon_l3_config(r, mon_configurable);
 }
 
 static __init void __check_quirks_intel(void)
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index c049a274383c..4d03f443b353 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -72,11 +72,15 @@ DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
  * struct mon_evt - Entry in the event list of a resource
  * @evtid:		event id
  * @name:		name of the event
+ * @configurable:	true if the event is configurable
+ * @config_name:	sysfs file name of the event if configurable
  * @list:		entry in &rdt_resource->evt_list
  */
 struct mon_evt {
 	u32			evtid;
 	char			*name;
+	bool			configurable;
+	char			*config_name;
 	struct list_head	list;
 };
 
@@ -529,7 +533,7 @@ int closids_supported(void);
 void closid_free(int closid);
 int alloc_rmid(void);
 void free_rmid(u32 rmid);
-int rdt_get_mon_l3_config(struct rdt_resource *r);
+int rdt_get_mon_l3_config(struct rdt_resource *r, bool configurable);
 void mon_event_count(void *info);
 int rdtgroup_mondata_show(struct seq_file *m, void *arg);
 void rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index eaf25a234ff5..dc97aa7a3b3d 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -656,11 +656,13 @@ static struct mon_evt llc_occupancy_event = {
 static struct mon_evt mbm_total_event = {
 	.name		= "mbm_total_bytes",
 	.evtid		= QOS_L3_MBM_TOTAL_EVENT_ID,
+	.config_name	= "mbm_total_config",
 };
 
 static struct mon_evt mbm_local_event = {
 	.name		= "mbm_local_bytes",
 	.evtid		= QOS_L3_MBM_LOCAL_EVENT_ID,
+	.config_name	= "mbm_local_config",
 };
 
 /*
@@ -682,7 +684,7 @@ static void l3_mon_evt_init(struct rdt_resource *r)
 		list_add_tail(&mbm_local_event.list, &r->evt_list);
 }
 
-int rdt_get_mon_l3_config(struct rdt_resource *r)
+int rdt_get_mon_l3_config(struct rdt_resource *r, bool configurable)
 {
 	unsigned int mbm_offset = boot_cpu_data.x86_cache_mbm_width_offset;
 	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
@@ -714,6 +716,11 @@ int rdt_get_mon_l3_config(struct rdt_resource *r)
 	if (ret)
 		return ret;
 
+	if (configurable) {
+		mbm_total_event.configurable = true;
+		mbm_local_event.configurable = true;
+	}
+
 	l3_mon_evt_init(r);
 
 	r->mon_capable = true;
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 04b519bca50d..834a55d78e3f 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1001,8 +1001,11 @@ static int rdt_mon_features_show(struct kernfs_open_file *of,
 	struct rdt_resource *r = of->kn->parent->priv;
 	struct mon_evt *mevt;
 
-	list_for_each_entry(mevt, &r->evt_list, list)
+	list_for_each_entry(mevt, &r->evt_list, list) {
 		seq_printf(seq, "%s\n", mevt->name);
+		if (mevt->configurable)
+			seq_printf(seq, "%s\n", mevt->config_name);
+	}
 
 	return 0;
 }

RE: [PATCH v5 06/12] x86/resctrl: Introduce data structure to support monitor configuration
Posted by Yu, Fenghua 3 years, 6 months ago
Hi, Babu,

> Add couple of fields in mon_evt to support Bandwidth Monitoring Event
> Configuratio (BMEC) and also update the "mon_features".

s/Configuratio/ Configuration/

> 
> The sysfs file "mon_features" will display the monitor configuration if supported.
> 
> Before the change.
> 	$cat /sys/fs/resctrl/info/L3_MON/mon_features
> 	llc_occupancy
> 	mbm_total_bytes
> 	mbm_local_bytes
> 
> After the change if BMEC is supported.
> 	$cat /sys/fs/resctrl/info/L3_MON/mon_features
> 	llc_occupancy
> 	mbm_total_bytes
> 	mbm_total_config
> 	mbm_local_bytes
> 	mbm_local_config
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kernel/cpu/resctrl/core.c     |    3 ++-
>  arch/x86/kernel/cpu/resctrl/internal.h |    6 +++++-
>  arch/x86/kernel/cpu/resctrl/monitor.c  |    9 ++++++++-
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c |    5 ++++-
>  4 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c
> b/arch/x86/kernel/cpu/resctrl/core.c
> index 56c96607259c..513e6a00f58e 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -849,6 +849,7 @@ static __init bool get_rdt_alloc_resources(void)  static
> __init bool get_rdt_mon_resources(void)  {
>  	struct rdt_resource *r =
> &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> +	bool mon_configurable = rdt_cpu_has(X86_FEATURE_BMEC);
> 
>  	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
>  		rdt_mon_features |= (1 << QOS_L3_OCCUP_EVENT_ID); @@ -
> 860,7 +861,7 @@ static __init bool get_rdt_mon_resources(void)
>  	if (!rdt_mon_features)
>  		return false;
> 
> -	return !rdt_get_mon_l3_config(r);
> +	return !rdt_get_mon_l3_config(r, mon_configurable);
>  }
> 
>  static __init void __check_quirks_intel(void) diff --git
> a/arch/x86/kernel/cpu/resctrl/internal.h
> b/arch/x86/kernel/cpu/resctrl/internal.h
> index c049a274383c..4d03f443b353 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -72,11 +72,15 @@ DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
>   * struct mon_evt - Entry in the event list of a resource
>   * @evtid:		event id
>   * @name:		name of the event
> + * @configurable:	true if the event is configurable
> + * @config_name:	sysfs file name of the event if configurable
>   * @list:		entry in &rdt_resource->evt_list
>   */
>  struct mon_evt {
>  	u32			evtid;
>  	char			*name;
> +	bool			configurable;
> +	char			*config_name;

Seems config_name is only used to be shown in mon_features. Is it necessary to have the field?

>  	struct list_head	list;
>  };
> 
> @@ -529,7 +533,7 @@ int closids_supported(void);  void closid_free(int closid);
> int alloc_rmid(void);  void free_rmid(u32 rmid); -int
> rdt_get_mon_l3_config(struct rdt_resource *r);
> +int rdt_get_mon_l3_config(struct rdt_resource *r, bool configurable);
>  void mon_event_count(void *info);
>  int rdtgroup_mondata_show(struct seq_file *m, void *arg);  void
> rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r, diff --git
> a/arch/x86/kernel/cpu/resctrl/monitor.c
> b/arch/x86/kernel/cpu/resctrl/monitor.c
> index eaf25a234ff5..dc97aa7a3b3d 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -656,11 +656,13 @@ static struct mon_evt llc_occupancy_event = {  static
> struct mon_evt mbm_total_event = {
>  	.name		= "mbm_total_bytes",
>  	.evtid		= QOS_L3_MBM_TOTAL_EVENT_ID,
> +	.config_name	= "mbm_total_config",
>  };

Struct mon_evt mbm_total_config_event = {
	.name = "mbm_total_config",

> 
>  static struct mon_evt mbm_local_event = {
>  	.name		= "mbm_local_bytes",
>  	.evtid		= QOS_L3_MBM_LOCAL_EVENT_ID,
> +	.config_name	= "mbm_local_config",
>  };
> 
>  /*
> @@ -682,7 +684,7 @@ static void l3_mon_evt_init(struct rdt_resource *r)
>  		list_add_tail(&mbm_local_event.list, &r->evt_list);  }
> 
> -int rdt_get_mon_l3_config(struct rdt_resource *r)
> +int rdt_get_mon_l3_config(struct rdt_resource *r, bool configurable)
>  {
>  	unsigned int mbm_offset =
> boot_cpu_data.x86_cache_mbm_width_offset;
>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); @@ -714,6
> +716,11 @@ int rdt_get_mon_l3_config(struct rdt_resource *r)
>  	if (ret)
>  		return ret;
> 
> +	if (configurable) {
> +		mbm_total_event.configurable = true;
> +		mbm_local_event.configurable = true;
> +	}
> +
>  	l3_mon_evt_init(r);
> 
>  	r->mon_capable = true;
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 04b519bca50d..834a55d78e3f 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -1001,8 +1001,11 @@ static int rdt_mon_features_show(struct
> kernfs_open_file *of,
>  	struct rdt_resource *r = of->kn->parent->priv;
>  	struct mon_evt *mevt;
> 
> -	list_for_each_entry(mevt, &r->evt_list, list)
> +	list_for_each_entry(mevt, &r->evt_list, list) {
>  		seq_printf(seq, "%s\n", mevt->name);
> +		if (mevt->configurable)
> +			seq_printf(seq, "%s\n", mevt->config_name);

If "config_name" is not defined, it could be:
		If (mevt->configurable)
			Seq_printf(seq, "%s_config\n", mevt->name);

Thanks.

-Fenghua
RE: [PATCH v5 06/12] x86/resctrl: Introduce data structure to support monitor configuration
Posted by Moger, Babu 3 years, 6 months ago
[AMD Official Use Only - General]

Hi Fenghua,

> -----Original Message-----
> From: Yu, Fenghua <fenghua.yu@intel.com>
> Sent: Tuesday, September 27, 2022 5:25 PM
> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net; Chatre, Reinette
> <reinette.chatre@intel.com>; tglx@linutronix.de; mingo@redhat.com;
> bp@alien8.de
> Cc: dave.hansen@linux.intel.com; x86@kernel.org; hpa@zytor.com;
> paulmck@kernel.org; akpm@linux-foundation.org; quic_neeraju@quicinc.com;
> rdunlap@infradead.org; damien.lemoal@opensource.wdc.com;
> songmuchun@bytedance.com; peterz@infradead.org; jpoimboe@kernel.org;
> pbonzini@redhat.com; Bae, Chang Seok <chang.seok.bae@intel.com>;
> pawan.kumar.gupta@linux.intel.com; jmattson@google.com;
> daniel.sneddon@linux.intel.com; Das1, Sandipan <Sandipan.Das@amd.com>;
> Luck, Tony <tony.luck@intel.com>; james.morse@arm.com; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; bagasdotme@gmail.com;
> Eranian, Stephane <eranian@google.com>
> Subject: RE: [PATCH v5 06/12] x86/resctrl: Introduce data structure to support
> monitor configuration
> 
> Hi, Babu,
> 
> > Add couple of fields in mon_evt to support Bandwidth Monitoring Event
> > Configuratio (BMEC) and also update the "mon_features".
> 
> s/Configuratio/ Configuration/

Sure.
> 
> >
> > The sysfs file "mon_features" will display the monitor configuration if
> supported.
> >
> > Before the change.
> > 	$cat /sys/fs/resctrl/info/L3_MON/mon_features
> > 	llc_occupancy
> > 	mbm_total_bytes
> > 	mbm_local_bytes
> >
> > After the change if BMEC is supported.
> > 	$cat /sys/fs/resctrl/info/L3_MON/mon_features
> > 	llc_occupancy
> > 	mbm_total_bytes
> > 	mbm_total_config
> > 	mbm_local_bytes
> > 	mbm_local_config
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  arch/x86/kernel/cpu/resctrl/core.c     |    3 ++-
> >  arch/x86/kernel/cpu/resctrl/internal.h |    6 +++++-
> >  arch/x86/kernel/cpu/resctrl/monitor.c  |    9 ++++++++-
> >  arch/x86/kernel/cpu/resctrl/rdtgroup.c |    5 ++++-
> >  4 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c
> > b/arch/x86/kernel/cpu/resctrl/core.c
> > index 56c96607259c..513e6a00f58e 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -849,6 +849,7 @@ static __init bool get_rdt_alloc_resources(void)
> > static __init bool get_rdt_mon_resources(void)  {
> >  	struct rdt_resource *r =
> > &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> > +	bool mon_configurable = rdt_cpu_has(X86_FEATURE_BMEC);
> >
> >  	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
> >  		rdt_mon_features |= (1 << QOS_L3_OCCUP_EVENT_ID); @@ -
> > 860,7 +861,7 @@ static __init bool get_rdt_mon_resources(void)
> >  	if (!rdt_mon_features)
> >  		return false;
> >
> > -	return !rdt_get_mon_l3_config(r);
> > +	return !rdt_get_mon_l3_config(r, mon_configurable);
> >  }
> >
> >  static __init void __check_quirks_intel(void) diff --git
> > a/arch/x86/kernel/cpu/resctrl/internal.h
> > b/arch/x86/kernel/cpu/resctrl/internal.h
> > index c049a274383c..4d03f443b353 100644
> > --- a/arch/x86/kernel/cpu/resctrl/internal.h
> > +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> > @@ -72,11 +72,15 @@ DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
> >   * struct mon_evt - Entry in the event list of a resource
> >   * @evtid:		event id
> >   * @name:		name of the event
> > + * @configurable:	true if the event is configurable
> > + * @config_name:	sysfs file name of the event if configurable
> >   * @list:		entry in &rdt_resource->evt_list
> >   */
> >  struct mon_evt {
> >  	u32			evtid;
> >  	char			*name;
> > +	bool			configurable;
> > +	char			*config_name;
> 
> Seems config_name is only used to be shown in mon_features. Is it necessary to
> have the field?

Sure. I can remove it.

> 
> >  	struct list_head	list;
> >  };
> >
> > @@ -529,7 +533,7 @@ int closids_supported(void);  void closid_free(int
> > closid); int alloc_rmid(void);  void free_rmid(u32 rmid); -int
> > rdt_get_mon_l3_config(struct rdt_resource *r);
> > +int rdt_get_mon_l3_config(struct rdt_resource *r, bool configurable);
> >  void mon_event_count(void *info);
> >  int rdtgroup_mondata_show(struct seq_file *m, void *arg);  void
> > rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r, diff --git
> > a/arch/x86/kernel/cpu/resctrl/monitor.c
> > b/arch/x86/kernel/cpu/resctrl/monitor.c
> > index eaf25a234ff5..dc97aa7a3b3d 100644
> > --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> > @@ -656,11 +656,13 @@ static struct mon_evt llc_occupancy_event = {
> > static struct mon_evt mbm_total_event = {
> >  	.name		= "mbm_total_bytes",
> >  	.evtid		= QOS_L3_MBM_TOTAL_EVENT_ID,
> > +	.config_name	= "mbm_total_config",
> >  };
> 
> Struct mon_evt mbm_total_config_event = {
> 	.name = "mbm_total_config",
> 
> >
> >  static struct mon_evt mbm_local_event = {
> >  	.name		= "mbm_local_bytes",
> >  	.evtid		= QOS_L3_MBM_LOCAL_EVENT_ID,
> > +	.config_name	= "mbm_local_config",
> >  };
> >
> >  /*
> > @@ -682,7 +684,7 @@ static void l3_mon_evt_init(struct rdt_resource *r)
> >  		list_add_tail(&mbm_local_event.list, &r->evt_list);  }
> >
> > -int rdt_get_mon_l3_config(struct rdt_resource *r)
> > +int rdt_get_mon_l3_config(struct rdt_resource *r, bool configurable)
> >  {
> >  	unsigned int mbm_offset =
> > boot_cpu_data.x86_cache_mbm_width_offset;
> >  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); @@ -714,6
> > +716,11 @@ int rdt_get_mon_l3_config(struct rdt_resource *r)
> >  	if (ret)
> >  		return ret;
> >
> > +	if (configurable) {
> > +		mbm_total_event.configurable = true;
> > +		mbm_local_event.configurable = true;
> > +	}
> > +
> >  	l3_mon_evt_init(r);
> >
> >  	r->mon_capable = true;
> > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > index 04b519bca50d..834a55d78e3f 100644
> > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > @@ -1001,8 +1001,11 @@ static int rdt_mon_features_show(struct
> > kernfs_open_file *of,
> >  	struct rdt_resource *r = of->kn->parent->priv;
> >  	struct mon_evt *mevt;
> >
> > -	list_for_each_entry(mevt, &r->evt_list, list)
> > +	list_for_each_entry(mevt, &r->evt_list, list) {
> >  		seq_printf(seq, "%s\n", mevt->name);
> > +		if (mevt->configurable)
> > +			seq_printf(seq, "%s\n", mevt->config_name);
> 
> If "config_name" is not defined, it could be:
> 		If (mevt->configurable)
> 			Seq_printf(seq, "%s_config\n", mevt->name);
> 
Sure. Thanks 
Babu