[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <be1f6d4e-6310-4345-a1b5-090fde96b7fe@amd.com>
Date: Thu, 29 May 2025 10:21:16 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, corbet@....net,
tony.luck@...el.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com
Cc: james.morse@....com, dave.martin@....com, fenghuay@...dia.com,
x86@...nel.org, hpa@...or.com, paulmck@...nel.org,
akpm@...ux-foundation.org, thuth@...hat.com, rostedt@...dmis.org,
ardb@...nel.org, gregkh@...uxfoundation.org, daniel.sneddon@...ux.intel.com,
jpoimboe@...nel.org, alexandre.chartre@...cle.com,
pawan.kumar.gupta@...ux.intel.com, thomas.lendacky@....com,
perry.yuan@....com, seanjc@...gle.com, kai.huang@...el.com,
xiaoyao.li@...el.com, kan.liang@...ux.intel.com, xin3.li@...el.com,
ebiggers@...gle.com, xin@...or.com, sohil.mehta@...el.com,
andrew.cooper3@...rix.com, mario.limonciello@....com,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
peternewman@...gle.com, maciej.wieczor-retman@...el.com, eranian@...gle.com,
Xiaojian.Du@....com, gautham.shenoy@....com
Subject: Re: [PATCH v13 12/27] x86/resctrl: Introduce event configuration
modes
Hi Reinette,
On 5/22/25 17:05, Reinette Chatre wrote:
> Hi Babu,
>
> On 5/15/25 3:51 PM, Babu Moger wrote:
>> MBM events can be configured using either BMEC (Bandwidth Monitoring Event
>> Configuration) or the mbm_cntr_assign mode.
>>
>> Introduce a data structure to represent the various event configuration
>> modes and their corresponding values.
>>
>> Suggested-by: Reinette Chatre <reinette.chatre@...el.com>
>
> I cannot recall suggesting this.
>
> (/me digs)
>
> Are you perhaps referring to https://lore.kernel.org/lkml/d2966a26-4483-4808-a538-bb20973dd2a1@intel.com/
Yes.
>
> This is not referring to new modes but the existing mbm_cntr_assign modes.
> resctrl knows which "mbm_cntr_assign" mode is active and it can use that
> to determine whether BMEC can be exposed to user space or not. There is
> already enough information in resctrl to know whether BMEC files should be
> exposed or not.
>
> I think this work self makes clear that these modes are useless since
> patch #25 that determines whether to hide BMEC files doesn't even
> use it.
Sure. I will remove change related to mbm_mode.
>
>
>> Signed-off-by: Babu Moger <babu.moger@....com>
>> ---
>> v13: New patch to handle different event configuration types with
>> mbm_cntr_assign mode.
>> ---
>> fs/resctrl/internal.h | 6 ++++--
>> fs/resctrl/monitor.c | 4 ++--
>> fs/resctrl/rdtgroup.c | 2 +-
>> include/linux/resctrl_types.h | 11 +++++++++++
>> 4 files changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
>> index 9a8cf6f11151..0fae374559ba 100644
>> --- a/fs/resctrl/internal.h
>> +++ b/fs/resctrl/internal.h
>> @@ -55,13 +55,15 @@ static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
>> * 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
>> + * @mbm_mode: monitoring mode (BMEC or mbm_cntr_assign)
>> + * @evt_cfg: event configuration value decoding reads, writes.
>> * @list: entry in &rdt_resource->evt_list
>> */
>> struct mon_evt {
>> enum resctrl_event_id evtid;
>> char *name;
>> - bool configurable;
>> + enum resctrl_mbm_mode mbm_mode;
>> + u32 evt_cfg;
>
> This very important yet totally unrelated member sneaked in without
> any mention.
Yes. We only need evt_cfg. I will just add it as separate patch.
>
>> struct list_head list;
>> };
>>
>> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
>> index 2548aee0151c..8e403587a02f 100644
>> --- a/fs/resctrl/monitor.c
>> +++ b/fs/resctrl/monitor.c
>> @@ -903,12 +903,12 @@ int resctrl_mon_resource_init(void)
>> l3_mon_evt_init(r);
>>
>> if (resctrl_arch_is_evt_configurable(QOS_L3_MBM_TOTAL_EVENT_ID)) {
>> - mbm_total_event.configurable = true;
>> + mbm_total_event.mbm_mode = MBM_MODE_BMEC;
>> resctrl_file_fflags_init("mbm_total_bytes_config",
>> RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
>> }
>> if (resctrl_arch_is_evt_configurable(QOS_L3_MBM_LOCAL_EVENT_ID)) {
>> - mbm_local_event.configurable = true;
>> + mbm_local_event.mbm_mode = MBM_MODE_BMEC;
>> resctrl_file_fflags_init("mbm_local_bytes_config",
>> RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
>> }
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index 752750e3e443..f192b2736a77 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -1152,7 +1152,7 @@ static int rdt_mon_features_show(struct kernfs_open_file *of,
>>
>> list_for_each_entry(mevt, &r->mon.evt_list, list) {
>> seq_printf(seq, "%s\n", mevt->name);
>> - if (mevt->configurable)
>> + if (mevt->mbm_mode == MBM_MODE_BMEC)
>
> This can instead be a call to a utility that returns whether BMEC should be
> visible based on resctrl_mon::mbm_cntr_assignable and rdt_hw_resource::mbm_cntr_assign_enabled
> (via resctrl_arch_mbm_cntr_assign_enabled() of course).
Sure. Will do.
>
>> seq_printf(seq, "%s_config\n", mevt->name);
>> }
>>
>> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
>> index a25fb9c4070d..26cd1fec72db 100644
>> --- a/include/linux/resctrl_types.h
>> +++ b/include/linux/resctrl_types.h
>> @@ -47,4 +47,15 @@ enum resctrl_event_id {
>> QOS_NUM_EVENTS,
>> };
>>
>> +/*
>> + * Event configuration mode.
>> + * Events can be configured either in BMEC (Bandwidth Monitoring Event
>> + * Configuration) mode or mbm_cntr_assign mode.
>> + */
>> +enum resctrl_mbm_mode {
>> + MBM_MODE_NONE,
>> + MBM_MODE_BMEC,
>> + MBM_MODE_ASSIGN,
>> +};
>> +
>> #endif /* __LINUX_RESCTRL_TYPES_H */
>
> Reinette
>
--
Thanks
Babu Moger
Powered by blists - more mailing lists