Re: [PATCH V2 2/2] arm64: topology: Reorder init_amu_fie() a bit

[Viresh Kumar]

On 14-12-20, 14:00, Ionela Voinescu wrote:
> I think there could be a potential problem here (it would be unlikely
> but why not fix it :) ). It was in the code before your changes.
> 
> When we enable amu_fie_key here, topology_scale_freq_tick() could be
> called for AMU CPUs, which will compute and set a scale factor. Later
> on, if we happen to find the system not invariant, we disable counter
> based invariance, but a scale factor might have been set already for a
> CPU, which would and should have returned 1024 otherwise (the
> initialisation value of freq_scale).
> 
> 
> Therefore, while here, you could instead do the following:
> 
> cpufreq_inv = cpufreq_supports_freq_invariance();
> 
> if (!cpufreq_inv &&
>     !cpumask_subset(cpu_online_mask, amu_fie_cpus))
>     goto free_valid_mask;
> 
> static_branch_enable(&amu_fie_key);
> 
> pr_info(..);
> 
> if (!cpufreq_inv)
>     rebuild_sched_domains_energy();
> 
> What do you think?

I already had a patch for this, but for a different reason, i.e. to
avoid the enable/disable dance.

       /* We aren't fully invariant yet */
       if (!prev && !cpumask_equal(amu_fie_cpus, cpu_present_mask))
               return;

And this is quite similar to what you have here.

-- 
viresh