Re: About CPU hot-plug stress test failed in cpufreq driver

["Rafael J. Wysocki" <rafael@kernel.org>]

On Tue, Dec 10, 2019 at 11:54 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 10-12-19, 11:39, Rafael J. Wysocki wrote:
> > Index: linux-pm/kernel/sched/cpufreq.c
> > ===================================================================
> > --- linux-pm.orig/kernel/sched/cpufreq.c
> > +++ linux-pm/kernel/sched/cpufreq.c
> > @@ -5,6 +5,8 @@
> >   * Copyright (C) 2016, Intel Corporation
> >   * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >   */
> > +#include <linux/cpufreq.h>
> > +
> >  #include "sched.h"
> >
> >  DEFINE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
> > @@ -57,3 +59,19 @@ void cpufreq_remove_update_util_hook(int
> >       rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), NULL);
> >  }
> >  EXPORT_SYMBOL_GPL(cpufreq_remove_update_util_hook);
> > +
> > +/**
> > + * cpufreq_this_cpu_can_update - Check if cpufreq policy can be updated.
> > + * @policy: cpufreq policy to check.
> > + *
> > + * Return 'true' if:
> > + * - the local and remote CPUs share @policy,
> > + * - dvfs_possible_from_any_cpu is set in @policy and the local CPU is not going
> > + *   offline (in which it is not expected to run cpufreq updates any more).
> > + */
> > +bool cpufreq_this_cpu_can_update(struct cpufreq_policy *policy)
> > +{
> > +     return cpumask_test_cpu(smp_processor_id(), policy->cpus) ||
> > +             (policy->dvfs_possible_from_any_cpu &&
>
> > +              rcu_dereference_sched(*this_cpu_ptr(&cpufreq_update_util_data)));
>
> I somehow feel that doing this particular check in cpufreq_update_util() maybe
> better. Or maybe we can call cpufreq_this_cpu_can_update() itself right from
> cpufreq_update_util() instead and remove it from multiple places in the
> governors.

First, there are two places actually.

Second, the point is that the presence of the hook only needs to be
checked if dvfs_possible_from_any_cpu is set and checking that in
cpufreq_update_util() would be kind of obnoxious IMO.