Re: [PATCH v10 08/17] x86/resctrl: Enforce system RMID limit on AET event groups

[Luck, Tony]

On Mon, Aug 17, 2026 at 05:58:13PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 7/29/26 10:27 AM, Tony Luck wrote:
> > AET (Application Energy Telemetry) event groups each support a specific
> > number of RMIDs. But that number may be lower than the number supported
> > by the system. Especially true on systems with SNC (Sub-NUMA Cluster)
> > enabled as that reduces the number of supported RMIDs.
> > 
> > Fix get_rdt_mon_resources() to return true when any monitor resource is
> 
> hmmm ... "Fix" makes one look for the accompanying "Fixes:" tag. What is
> the fix here? What is wrong with existing implementation that needs fixing?
> To me this does not look like a fix though (more below).
> 
> > possibly enabled. Call intel_aet_init() to adjust the event_group::num_rmid
> > values to not exceed the system supported maximum.
> 
> Last sentence just documents the code. Please describe why this is needed.
> Is this a separate logical change?
> 
> > 
> > Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
> > ---
> 
> ...
> 
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> > index 092764cf693f..2c938b97b147 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -1019,10 +1019,10 @@ static __init bool get_rdt_mon_resources(void)
> >  	if (rdt_cpu_has(X86_FEATURE_ABMC))
> >  		ret = true;
> >  
> > -	if (!ret)
> > -		return false;
> > +	if (ret)
> > +		rdt_get_l3_mon_config(r);
> >  
> > -	return !rdt_get_l3_mon_config(r);
> > +	return boot_cpu_data.x86_cache_max_rmid > 0;
> >  }
> 
> >From what I can tell this will return true when the system supports monitoring,
> but no resource may actually have monitoring enabled at this point. Specifically,
> no resource has rdt_resource::mon_capable set. 
> 
> The resctrl initialization now proceeds where it used to stop. resctrl_arch_late_init()
> will proceed and initialize the resctrl filesystem, which in turn would allow user space
> to mount it.
> 
> rdt_get_tree() handling the user mount request could thus be run on a system that does
> not have a monitoring or allocation capable resource and then we see in rdt_get_tree():
> 	if (resctrl_arch_alloc_capable() || resctrl_arch_mon_capable())
> 		resctrl_mounted = true;

Should the inverse of that check really be an error condition and result
in failing the mount? There seems no point in a mount with no monitor or
alloc features.

This code appeared as part of James' separating the x86 specific static
branch code out of the filesystem generic mount path in commit
13e5769debf0 ("x86/resctrl: Make resctrl_mounted checks explicit")

My plan is to move this inverted check earlier (right after the call
to resctrl_arch_pre_mount() which could be the decision point on if
any monitor resources are enabled.


rdt_get_tree()
{
	mutex_lock(&resctrl_mount_lock); // NEW (revived from v4 of series)

	check for nested mount -> -EBUSY

	resctrl_arch_pre_mount();

	if (!resctrl_arch_alloc_capable() && !resctrl_arch_mon_capable()) {
		ret = -EINVAL;
		goto out_mount_unlock;
	}

	...

	resctrl_mounted = true;

Choice of error code is still open. I tried -ENODEV, but that results in
an error message to the user saying the resctrl filesystem is not
supported.

> 
> The above flow change would cause resctrl to think the system supports monitoring
> but resctrl_arch_mon_capable() returns false. If there are no allocation features
> then this will result in resctrl fs mounted ... but resctrl_mounted is not set to
> true and thus allow a remount that is not supported.
> 
> Reinette

-Tony