RE: [External] Re: [PATCH v2] arm64: topology: add source check in arch_cpu_idle_enter()

On Mon, Aug 10, 2026 at 11:05 PM, Sumit Gupta wrote: 
> 
> > diff --git a/arch/arm64/kernel/topology.c
> > b/arch/arm64/kernel/topology.c index b32f13358fbb..cae5da68d5ec
> 100644
> > --- a/arch/arm64/kernel/topology.c
> > +++ b/arch/arm64/kernel/topology.c
> > @@ -173,6 +173,9 @@ void arch_cpu_idle_enter(void)
> >          if (!amu_fie_cpu_supported(cpu))
> >                  return;
> >
> > +       if (!topology_scale_freq_source(SCALE_FREQ_SOURCE_ARCH,
> cpu))
> > +               return;
> > +
> >          /* Kick in AMU update but only if one has not happened already
> */
> >          if (housekeeping_cpu(cpu, HK_TYPE_TICK) &&
> >
> > time_is_before_jiffies(per_cpu(cpu_amu_samples.last_scale_update,
> > cpu)))
> 
> How about moving it into the condition below, so we only take the RCU read
> side when an update is due rather than on every idle entry?
> 
>      if (housekeeping_cpu(cpu, HK_TYPE_TICK) &&
> time_is_before_jiffies(per_cpu(cpu_amu_samples.last_scale_update, cpu))
> &&
>          topology_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, cpu))
>          amu_scale_freq_tick();
> 

I agree with you. It's better to move it into the condition below.

> > diff --git a/drivers/base/arch_topology.c
> > b/drivers/base/arch_topology.c index 8c5e47c28d9a..dfc2574a5588
> 100644
> > --- a/drivers/base/arch_topology.c
> > +++ b/drivers/base/arch_topology.c
> > @@ -127,6 +127,20 @@ void topology_clear_scale_freq_source(enum
> scale_freq_source source,
> >   }
> >   EXPORT_SYMBOL_GPL(topology_clear_scale_freq_source);
> >
> > +bool topology_scale_freq_source(enum scale_freq_source source,
> > +unsigned int cpu)
> 
> %s/topology_scale_freq_source/topology_is_scale_freq_source/ ?

Agreed, the `is_` prefix is clearer for a boolean check

> > +{
> > +       struct scale_freq_data *sfd;
> > +       bool ret;
> > +
> > +       rcu_read_lock_sched();
> > +       sfd = rcu_dereference_sched(*per_cpu_ptr(&sft_data, cpu));
> > +       ret = (sfd && sfd->source == source);
> > +       rcu_read_unlock_sched();
> > +
> > +       return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(topology_scale_freq_source);
> > +
> 
> Do we need to export it?

No, the only caller is in arch/arm64/kernel/topology.c which is
built-in. I will remove the EXPORT_SYMBOL_GPL.

I will send v3 with these changes. Thanks for the review.