Re: [PATCH v7 06/14] fs/resctrl: Add interface to disable a monitor event

[Luck, Tony]

On Mon, Jun 08, 2026 at 04:18:23PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 6/1/26 12:56 PM, Tony Luck wrote:
> > In preparation for re-running AET enumeration on every mount, AET code must be
> > able to disable events on unmount so the next mount starts from a clean slate.
> > 
> > Add a file system interface for architecture to clear the enabled flag for
> > a given event.
> 
> This just verbatim describes what the patch does. It would be helpful to describe
> how the flag is used by resctrl to support the first paragraph's implicit claim
> that the enabled flag's value is not relevant when resctrl is unmounted.

Revised commit:
---
Subject: fs/resctrl: Add interface to disable a monitor event

In preparation for re-running AET enumeration on every mount, AET code must be
able to disable events on unmount so the next mount starts from a clean slate.

Add a file system interface for architecture to reset architecture
controlled fields of the given event. mon_event::enabled is only used
during mount and at run time to check which events to include in file
system objects. It is not used during unmount, so it is safe to clear
it as part of the unmount flow.

Add kerneldoc comments to describe limitations on when events may be enabled
or disabled.
---

> 
> > diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> > index 9fd901c78dc6..327e7a863614 100644
> > --- a/fs/resctrl/monitor.c
> > +++ b/fs/resctrl/monitor.c
> > @@ -1012,6 +1012,18 @@ bool resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu,
> >  	return true;
> >  }
> >  
> > +void resctrl_disable_mon_event(enum resctrl_event_id eventid)
> > +{
> > +	if (WARN_ON_ONCE(eventid < QOS_FIRST_EVENT || eventid >= QOS_NUM_EVENTS))
> > +		return;
> > +	if (!mon_event_all[eventid].enabled) {
> > +		pr_warn("Repeat disable for event %d\n", eventid);
> > +		return;
> > +	}
> > +
> > +	mon_event_all[eventid].enabled = false;
> > +}
> 
> It seems reasonable for architecture to expect that "disable" of event undoes all
> settings from the earlier "enable" of event. The new function only sets the "enabled"
> flag to false though. This is potentially confusing since it leaves the event with
> some lingering state, some of which is a pointer to state that an architecture may
> be reasonable to remove after disabling this event leaving a dangling pointer. 

Ok. I'll add:

+	mon_event_all[eventid].any_cpu = false;
+	mon_event_all[eventid].binary_bits = 0;
+	mon_event_all[eventid].arch_priv = NULL;

> 
> Reinette

-Tony
>