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
