lkml.org 
[lkml]   [2022]   [Sep]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v5 06/12] x86/resctrl: Introduce data structure to support monitor configuration
Date
[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
\
 
 \ /
  Last update: 2022-09-28 14:56    [W:0.104 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site