Re: [PATCH v9 05/12] x86/resctrl: Drop global 'rdt_mon_capable' flag

[Reinette Chatre]

Hi Tony,

On 7/1/26 2:35 PM, Tony Luck wrote:
> File system code calls resctrl_arch_mon_capable() to determine if the
> system supports any monitoring features.
> 
> x86 architecture code sets a global flag to support implementation of
> resctrl_arch_mon_capable() but the upcoming change to enumerate AET
> (Application Energy Telemetry) features on each mount introduces a new
> corner case.
> 
> Specifically when AET is the only monitoring feature, loading and
> unloading the pmt_telemetry module between resctrl mounts may result
> in monitoring support enabled on some mounts, but not on others.
> 
> Replace the global flag with a scan of resources to check if any are
> marked mon_capable.
> 
> Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
> ---

...

> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> index 575f8408a9e7..a9f481b7a8ed 100644
> --- a/arch/x86/include/asm/resctrl.h
> +++ b/arch/x86/include/asm/resctrl.h

...

> @@ -66,10 +65,7 @@ static inline void resctrl_arch_disable_alloc(void)
>  	static_branch_dec_cpuslocked(&rdt_enable_key);
>  }
>  
> -static inline bool resctrl_arch_mon_capable(void)
> -{
> -	return rdt_mon_capable;
> -}
> +bool resctrl_arch_mon_capable(void);
>  
>  static inline void resctrl_arch_enable_mon(void)
>  {
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 9b9495174041..60d50ac79e7b 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -51,6 +51,20 @@ DEFINE_PER_CPU(struct resctrl_pqr_state, pqr_state);
>   */
>  bool rdt_alloc_capable;
>  
> +/*
> + * Need to re-evaluate on each mount whether any mon_capable resources
> + * are enabled.
> + */

This series introduces a significant change in behavior of this call and I
find that having the declaration in asm header and this significant comment in
arch code to essentially bury very important information on this
call's usage. Could the declaration be elevated to include/linux/resctrl.h
where its function comments can provide the details on how resctrl fs
uses this call and what its expectations from the architecture are? 
Basically this series causes resctrl_arch_mon_capable() to possibly return
different value depending on when it is called so there needs to be
a contract between resctrl fs and architecture on when resctrl fs can safely
call this and expect consistent results.

> +bool resctrl_arch_mon_capable(void)
> +{
> +	struct rdt_resource *r;
> +
> +	for_each_mon_capable_rdt_resource(r)
> +		return true;
> +
> +	return false;
> +}
> +

Reinette