Re: [PATCH v16 26/34] fs/resctrl: Provide interface to update the event configurations

[Moger, Babu]

Hi Reinette,

On 7/30/2025 3:05 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 7/25/25 11:29 AM, Babu Moger wrote:
> > When "mbm_event" counter assignment mode is supported, users can modify
> "supported" -> "enabled"?

Sure.


> > the event configuration by writing to the 'event_filter' resctrl file.
> > The event configurations for mbm_event mode are located in
> > /sys/fs/resctrl/info/L3_MON/event_configs/.
> >
> > Update the assignments of all CTRL_MON and MON resource groups when the
> > event configuration is modified.
> >
> > Example:
> > $ mount -t resctrl resctrl /sys/fs/resctrl
> >
> > $ cd /sys/fs/resctrl/
> >
> > $ cat info/L3_MON/event_configs/mbm_local_bytes/event_filter
> >    local_reads,local_non_temporal_writes,local_reads_slow_memory
> >
> > $ echo "local_reads,local_non_temporal_writes" >
> >    info/L3_MON/event_configs/mbm_total_bytes/event_filter
> >
> > $ cat info/L3_MON/event_configs/mbm_total_bytes/event_filter
> >    local_reads,local_non_temporal_writes
> >
> > Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> > ---
> ...
>
> > ---
> >   Documentation/filesystems/resctrl.rst |  12 +++
> >   fs/resctrl/internal.h                 |   4 +
> >   fs/resctrl/monitor.c                  | 114 ++++++++++++++++++++++++++
> >   fs/resctrl/rdtgroup.c                 |   3 +-
> >   4 files changed, 132 insertions(+), 1 deletion(-)
> ...
>
> > diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
> > index e082d8718199..e2e3fc0c5fab 100644
> > --- a/fs/resctrl/internal.h
> > +++ b/fs/resctrl/internal.h
> > @@ -409,11 +409,15 @@ void rdtgroup_unassign_cntr_event(struct rdt_mon_domain *d, struct rdtgroup *rdt
> >   				  struct mon_evt *mevt);
> >   int mbm_cntr_get(struct rdt_resource *r, struct rdt_mon_domain *d,
> >   		 struct rdtgroup *rdtgrp, enum resctrl_event_id evtid);
> > +void resctrl_update_cntr_allrdtgrp(struct mon_evt *mevt);
> Is there some code ordering issue in monitor.c? Looks like this function
> is only used in monitor.c so seeing it here is unexpected.
Yes. Not required anymore.
> >   
> >   void *rdt_kn_parent_priv(struct kernfs_node *kn);
> >   
> >   int event_filter_show(struct kernfs_open_file *of, struct seq_file *seq, void *v);
> >   
> > +ssize_t event_filter_write(struct kernfs_open_file *of, char *buf, size_t nbytes,
> > +			   loff_t off);
> > +
> >   #ifdef CONFIG_RESCTRL_FS_PSEUDO_LOCK
> >   int rdtgroup_locksetup_enter(struct rdtgroup *rdtgrp);
> >   
> > diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> > index fa5f63126682..8efbeb910f77 100644
> > --- a/fs/resctrl/monitor.c
> > +++ b/fs/resctrl/monitor.c
> ...
>
> > @@ -1193,3 +1264,46 @@ void rdtgroup_unassign_cntr_event(struct rdt_mon_domain *d, struct rdtgroup *rdt
> >   		rdtgroup_free_unassign_cntr(r, d, rdtgrp, mevt);
> >   	}
> >   }
> > +
> > +/*
> > + * rdtgroup_update_cntr_event - Update the counter assignments for the event
> > + *				in a group.
> > + * @r:		Resource to which update needs to be done.
> > + * @rdtgrp:	Resctrl group.
> > + * @evtid:	MBM monitor event.
> > + */
> > +static void rdtgroup_update_cntr_event(struct rdt_resource *r, struct rdtgroup *rdtgrp,
> > +				       enum resctrl_event_id evtid)
> > +{
> > +	struct rdt_mon_domain *d;
> > +	int cntr_id;
> > +
> > +	list_for_each_entry(d, &r->mon_domains, hdr.list) {
> > +		cntr_id = mbm_cntr_get(r, d, rdtgrp, evtid);
> > +		if (cntr_id >= 0)
> > +			resctrl_arch_config_cntr(r, d, evtid, rdtgrp->mon.rmid,
> > +						 rdtgrp->closid, cntr_id, true);
> Should non-arch MBM state be reset here?

Yes. Added it now.

Thanks

Babu