Re: [PATCH v11 08/23] x86/resctrl: Delete rdt_alloc_capable and rdt_mon_capable

[Luck, Tony]

On Wed, Sep 09, 2026 at 08:57:04PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 8/31/26 10:44 AM, Tony Luck wrote:
> > These globals are no longer needed.
> > 
> > Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
> 
> (missing scissor lines)
> 
> > v11:
> > 	New patch
> > ---
> >  arch/x86/include/asm/resctrl.h        |  2 --
> >  arch/x86/kernel/cpu/resctrl/core.c    | 15 +++++----------
> >  arch/x86/kernel/cpu/resctrl/monitor.c |  6 ------
> >  3 files changed, 5 insertions(+), 18 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> > index fb6cfff82e2f..49b5357e2e5c 100644
> > --- a/arch/x86/include/asm/resctrl.h
> > +++ b/arch/x86/include/asm/resctrl.h
> > @@ -42,8 +42,6 @@ struct resctrl_pqr_state {
> >  
> >  DECLARE_PER_CPU(struct resctrl_pqr_state, pqr_state);
> >  
> > -extern bool rdt_alloc_capable;
> > -extern bool rdt_mon_capable;
> >  extern unsigned int __ro_after_init rdt_l3_mon_scale;
> >  
> >  DECLARE_STATIC_KEY_FALSE(rdt_enable_key);
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> > index 62b97e357ce5..e851da431dd9 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -45,12 +45,6 @@ static DEFINE_MUTEX(domain_list_lock);
> >   */
> >  DEFINE_PER_CPU(struct resctrl_pqr_state, pqr_state);
> >  
> > -/*
> > - * Global boolean for rdt_alloc which is true if any
> > - * resource allocation is enabled.
> > - */
> > -bool rdt_alloc_capable;
> > -
> >  static void mba_wrmsr_intel(struct msr_param *m);
> >  static void cat_wrmsr(struct msr_param *m);
> >  static void mba_wrmsr_amd(struct msr_param *m);
> > @@ -786,7 +780,6 @@ void resctrl_arch_pre_mount(void)
> >  	cpus_read_lock();
> >  	mutex_lock(&domain_list_lock);
> >  	r->mon_capable = true;
> > -	rdt_mon_capable = true;
> >  	for_each_online_cpu(cpu)
> >  		domain_add_cpu_mon(cpu, r);
> >  	mutex_unlock(&domain_list_lock);
> > @@ -1026,10 +1019,12 @@ static __init void check_quirks(void)
> >  
> >  static __init bool get_rdt_resources(void)
> >  {
> > -	rdt_alloc_capable = get_rdt_alloc_resources();
> > -	rdt_mon_capable = get_rdt_mon_resources();
> > +	bool alloc_capable, mon_capable;
> > +
> > +	alloc_capable = get_rdt_alloc_resources();
> > +	mon_capable = get_rdt_mon_resources();
> >  
> > -	return (rdt_mon_capable || rdt_alloc_capable);
> > +	return (mon_capable || alloc_capable);
> 
> Are these new local variables really needed? Looks like this can just be a single
> return statement ... which in turn makes get_rdt_resources() look unnecessary?

I need to call both get_rdt_alloc_resources() and get_rdt_mon_resources()
regardless of the return value of each. Changing this to:

	return get_rdt_mon_resources() || get_rdt_alloc_resources();

would skip the call to get_rdt_alloc_resources() when get_rdt_mon_resources()
returns "true".

Am I missing something?

> 
> >  }
> >  
> >  static __init void rdt_init_res_defs_intel(void)
> > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> > index 0bce199a5f7b..bc2df287ccfa 100644
> > --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> > @@ -26,12 +26,6 @@
> >  
> >  #include "internal.h"
> >  
> > -/*
> > - * Global boolean for rdt_monitor which is true if any
> > - * resource monitoring is enabled.
> > - */
> > -bool rdt_mon_capable;
> > -
> >  /*
> >   * Scale factor to convert L3 monitor events to bytes.
> >   */
> 
> Reinette

-Tony