On Friday 08 Jan 2021 at 16:46:53 (+0530), Viresh Kumar wrote:
> The AMU counters won't get used today if the cpufreq driver is built as
> a module as the amu core requires everything to be ready by late init.
>
> Fix that properly by registering for cpufreq policy notifier. Note that
> the amu core don't have any cpufreq dependency after the first time
> CPUFREQ_CREATE_POLICY notifier is called for all the CPUs. And so we
> don't need to do anything on the CPUFREQ_REMOVE_POLICY notifier. And for
> the same reason we check if the CPUs are already parsed in the beginning
> of amu_fie_setup() and skip if that is true. Alternatively we can shoot
> a work from there to unregister the notifier instead, but that seemed
> too much instead of this simple check.
>
> While at it, convert the print message to pr_debug instead of pr_info.
>
> Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>
>
> ---
> arch/arm64/kernel/topology.c | 92 +++++++++++++++++++-----------------
> 1 file changed, 48 insertions(+), 44 deletions(-)
>
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 57267d694495..e08a4126453a 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -199,69 +199,38 @@ static int freq_inv_set_max_ratio(int cpu, u64
> max_rate, u64 ref_rate)
> return 0;
> }
>
> -static inline void
> -enable_policy_freq_counters(int cpu, cpumask_var_t valid_cpus)
> -{
> - struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> -
> - if (!policy) {
> - pr_debug("CPU%d: No cpufreq policy found.\n", cpu);
> - return;
> - }
> -
> - if (cpumask_subset(policy->related_cpus, valid_cpus))
> - cpumask_or(amu_fie_cpus, policy->related_cpus,
> - amu_fie_cpus);
> -
> - cpufreq_cpu_put(policy);
> -}
> -
> static DEFINE_STATIC_KEY_FALSE(amu_fie_key);
> #define amu_freq_invariant() static_branch_unlikely(&amu_fie_key)
>
> -static int __init init_amu_fie(void)
> +static void amu_fie_setup(const struct cpumask *cpus)
> {
> - cpumask_var_t valid_cpus;
> bool invariant;
> - int ret = 0;
> int cpu;
>
> - if (!zalloc_cpumask_var(&valid_cpus, GFP_KERNEL))
> - return -ENOMEM;
> -
> - if (!zalloc_cpumask_var(&amu_fie_cpus, GFP_KERNEL)) {
> - ret = -ENOMEM;
> - goto free_valid_mask;
> - }
> + /* We are already set since the last insmod of cpufreq driver */
> + if (unlikely(cpumask_subset(cpus, amu_fie_cpus)))
> + return;
>
> - for_each_present_cpu(cpu) {
> + for_each_cpu(cpu, cpus) {
> if (!freq_counters_valid(cpu) ||
> freq_inv_set_max_ratio(cpu,
> cpufreq_get_hw_max_freq(cpu) * 1000,
> arch_timer_get_rate()))
> - continue;
> -
> - cpumask_set_cpu(cpu, valid_cpus);
> - enable_policy_freq_counters(cpu, valid_cpus);
> + return;
> }
>
> - /* Overwrite amu_fie_cpus if all CPUs support AMU */
> - if (cpumask_equal(valid_cpus, cpu_present_mask))
> - cpumask_copy(amu_fie_cpus, cpu_present_mask);
> -
> - if (cpumask_empty(amu_fie_cpus))
> - goto free_valid_mask;
> + cpumask_or(amu_fie_cpus, amu_fie_cpus, cpus);
>
> invariant = topology_scale_freq_invariant();
>
> /* We aren't fully invariant yet */
> if (!invariant && !cpumask_equal(amu_fie_cpus, cpu_present_mask))
> - goto free_valid_mask;
> + return;
>
> static_branch_enable(&amu_fie_key);
>
> - pr_info("CPUs[%*pbl]: counters will be used for FIE.",
> - cpumask_pr_args(amu_fie_cpus));
> + pr_debug("CPUs[%*pbl]: counters will be used for FIE.",
> + cpumask_pr_args(cpus));
>
> /*
> * Task scheduler behavior depends on frequency invariance support,
> @@ -271,13 +240,48 @@ static int __init init_amu_fie(void)
> */
> if (!invariant)
> rebuild_sched_domains_energy();
> +}
> +
> +static int init_amu_fie_callback(struct notifier_block *nb, unsigned long
> val,
> + void *data)
> +{
> + struct cpufreq_policy *policy = data;
> +
> + if (val == CPUFREQ_CREATE_POLICY)
> + amu_fie_setup(policy->related_cpus);
> +
> + /*
> + * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU
> + * counters don't have any dependency on cpufreq driver once we have
> + * initialized AMU support and enabled invariance. The AMU counters will
> + * keep on working just fine in the absence of the cpufreq driver, and
> + * for the CPUs for which there are no counters available, the last set
> + * value of freq_scale will remain valid as that is the frequency those
> + * CPUs are running at.
> + */
> +
> + return 0;
> +}
> +
> +static struct notifier_block init_amu_fie_notifier = {
> + .notifier_call = init_amu_fie_callback,
> +};
> +
> +static int __init init_amu_fie(void)
> +{
> + int ret;
> +
> + if (!zalloc_cpumask_var(&amu_fie_cpus, GFP_KERNEL))
> + return -ENOMEM;
>
> -free_valid_mask:
> - free_cpumask_var(valid_cpus);
> + ret = cpufreq_register_notifier(&init_amu_fie_notifier,
> + CPUFREQ_POLICY_NOTIFIER);
> + if (ret)
> + free_cpumask_var(amu_fie_cpus);
>
> return ret;
> }
> -late_initcall_sync(init_amu_fie);
> +core_initcall(init_amu_fie);
>
> bool arch_freq_counters_available(const struct cpumask *cpus)
> {
> --
> 2.25.0.rc1.19.g042ed3e048af
>
Reviewed-by: Ionela Voinescu <ionela.voine...@arm.com>
Thanks,
Ionela.
