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

[Reinette Chatre <reinette.chatre@intel.com>]

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.

> 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. 

Reinette