[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <MW3PR12MB4553C14CBF929B68413D4A3B95549@MW3PR12MB4553.namprd12.prod.outlook.com>
Date: Wed, 28 Sep 2022 12:56:25 +0000
From: "Moger, Babu" <Babu.Moger@....com>
To: "Yu, Fenghua" <fenghua.yu@...el.com>,
"corbet@....net" <corbet@....net>,
"Chatre, Reinette" <reinette.chatre@...el.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"bp@...en8.de" <bp@...en8.de>
CC: "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
"paulmck@...nel.org" <paulmck@...nel.org>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"quic_neeraju@...cinc.com" <quic_neeraju@...cinc.com>,
"rdunlap@...radead.org" <rdunlap@...radead.org>,
"damien.lemoal@...nsource.wdc.com" <damien.lemoal@...nsource.wdc.com>,
"songmuchun@...edance.com" <songmuchun@...edance.com>,
"peterz@...radead.org" <peterz@...radead.org>,
"jpoimboe@...nel.org" <jpoimboe@...nel.org>,
"pbonzini@...hat.com" <pbonzini@...hat.com>,
"Bae, Chang Seok" <chang.seok.bae@...el.com>,
"pawan.kumar.gupta@...ux.intel.com"
<pawan.kumar.gupta@...ux.intel.com>,
"jmattson@...gle.com" <jmattson@...gle.com>,
"daniel.sneddon@...ux.intel.com" <daniel.sneddon@...ux.intel.com>,
"Das1, Sandipan" <Sandipan.Das@....com>,
"Luck, Tony" <tony.luck@...el.com>,
"james.morse@....com" <james.morse@....com>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"bagasdotme@...il.com" <bagasdotme@...il.com>,
"Eranian, Stephane" <eranian@...gle.com>
Subject: RE: [PATCH v5 06/12] x86/resctrl: Introduce data structure to support
monitor configuration
[AMD Official Use Only - General]
Hi Fenghua,
> -----Original Message-----
> From: Yu, Fenghua <fenghua.yu@...el.com>
> Sent: Tuesday, September 27, 2022 5:25 PM
> To: Moger, Babu <Babu.Moger@....com>; corbet@....net; Chatre, Reinette
> <reinette.chatre@...el.com>; tglx@...utronix.de; mingo@...hat.com;
> bp@...en8.de
> Cc: dave.hansen@...ux.intel.com; x86@...nel.org; hpa@...or.com;
> paulmck@...nel.org; akpm@...ux-foundation.org; quic_neeraju@...cinc.com;
> rdunlap@...radead.org; damien.lemoal@...nsource.wdc.com;
> songmuchun@...edance.com; peterz@...radead.org; jpoimboe@...nel.org;
> pbonzini@...hat.com; Bae, Chang Seok <chang.seok.bae@...el.com>;
> pawan.kumar.gupta@...ux.intel.com; jmattson@...gle.com;
> daniel.sneddon@...ux.intel.com; Das1, Sandipan <Sandipan.Das@....com>;
> Luck, Tony <tony.luck@...el.com>; james.morse@....com; linux-
> doc@...r.kernel.org; linux-kernel@...r.kernel.org; bagasdotme@...il.com;
> Eranian, Stephane <eranian@...gle.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@....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
Powered by blists - more mailing lists