Re: [PATCH v16 25/34] fs/resctrl: Add event configuration directory under info/L3_MON/

[Moger, Babu]

Hi Reinette,

On 8/8/2025 10:12 AM, Reinette Chatre wrote:
> Hi Babu,
>
> On 8/8/25 6:56 AM, Moger, Babu wrote:
> > On 7/30/2025 3:04 PM, Reinette Chatre wrote:
> > > On 7/25/25 11:29 AM, Babu Moger wrote:
> > > > diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> > > > index 16bcfeeb89e6..fa5f63126682 100644
> > > > --- a/fs/resctrl/monitor.c
> > > > +++ b/fs/resctrl/monitor.c
> > > > @@ -929,6 +929,29 @@ struct mbm_transaction mbm_transactions[NUM_MBM_TRANSACTIONS] = {
> > > >        {"dirty_victim_writes_all", DIRTY_VICTIMS_TO_ALL_MEM},
> > > >    };
> > > >    +int event_filter_show(struct kernfs_open_file *of, struct seq_file *seq, void *v)
> > > > +{
> > > > +    struct mon_evt *mevt = rdt_kn_parent_priv(of->kn);
> > > > +    bool sep = false;
> > > > +    int i;
> > > > +
> > > > +    mutex_lock(&rdtgroup_mutex);
> > > > +
> > > There is inconsistency among the files introduced on how
> > > "mbm_event mode disabled" case is handled. Some files return failure
> > > from their _show()/_write() when "mbm_event mode is disabled", some don't.
> > >
> > > The "event_filter" file always prints the MBM transactions monitored
> > > when assignable counters are supported, whether mbm_event mode is enabled
> > > or not. This means that the MBM event's configuration values are printed
> > > when "default" mode is enabled.  I have two concerns about this
> > > 1) This is potentially very confusing since switching to "default" will
> > >      make the BMEC files visible that will enable the user to modify the
> > >      event configurations per domain. Having this file print a global event
> > >      configuration while there are potentially various different domain-specific
> > >      configuration active will be confusing.
> > Yes. I agree.
> hmmm ... ok, you say that you agree but I cannot tell if you are going to
> do anything about it.
>
> On a system with BMEC enabled the mbm_total_bytes_config and mbm_local_bytes_config
> files should be the *only* source of MBM event configuration information, no?

That is correct.


> It may be ok to have event_filter print configuration when assignable counters are disabled
> if BMEC is not supported but that would require that this information will always be
> known for a "default" monitoring setup. While this may be true for AMD it is not obvious
> to me that this is universally true. Once this file exists in this form resctrl will always
> be required to provide data for the event configuration and it is not clear to me that
> this can always be guaranteed.

Yea. It is not true universally true. I don't know what these values are 
for Intel and ARM.

> > > 2) Can it be guaranteed that the MBM events will monitor the default
> > >      assignable counter memory transactions when in "default" mode? It has
> > >      never been possible to query which memory transactions are monitored by
> > >      the default X86_FEATURE_CQM_MBM_TOTAL and X86_FEATURE_CQM_MBM_LOCAL features
> > >      so this seems to use one feature to deduce capabilities or another?
> > So, initialize both total and local event configuration to default values when switched to "default mode"?
> >
> > Something like this?
> >
> > mon_event_all[QOS_L3_MBM_TOTAL_EVENT_ID].evt_cfg = r->mon.mbm_cfg_mask;
> >
> > mon_event_all[QOS_L3_MBM_LOCAL_EVENT_ID].evt_cfg = READS_TO_LOCAL_MEM | READS_TO_LOCAL_S_MEM | NON_TEMP_WRITE_TO_LOCAL_MEM;
> >
> > We are already doing that right (in patch 32)?
> Yes, but it creates this strange dependency where the "default" monitoring mode
> (that has been supported long before configurable events and assignable counters came
> along) requires support of "assignable counter mode".
>
> Consider it from another view, if resctrl wants to make event configuration available
> for the "default" mode then the "event_filter" file could always be visible when MBM
> local/total is supported to give users insight into what is monitored, whether
> assignable counters are supported or not. This is not possible on systems that do
> not support ABMC though, right?

That is correct. With BMEC, each domain can have its own settings.  So, 
printing the default will not be accurate.

What options do we have here.

How about adding the check if (resctrl_arch_mbm_cntr_assign_enabled())?  
Only print the values when ABMC is supported else print information in 
"last_cmd_status".

Thanks

Babu