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@xxxxxxxxx> > > I cannot recall suggesting this. > > (/me digs) > > Are you perhaps referring to https://lore.kernel.org/lkml/d2966a26-4483-4808-a538-bb20973dd2a1@xxxxxxxxx/ 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@xxxxxxx> >> --- >> 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
