Re: [PATCH 2/2] cpufreq: make sure frequency transitions are serialized

[Viresh Kumar]

On 23 October 2013 14:43, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> I got another solution which is much simpler, and I really can't believe
> (psychologically) that it will solve all the problems we were talking about..
> Please see if there is any loophole in there..

Ping!!

> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index ec391d7..e4ed89a 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -340,6 +340,13 @@ static void __cpufreq_notify_transition(struct
> cpufreq_policy *policy,
>         }
>  }
>
> +void cpufreq_notify_transition_each_cpu(struct cpufreq_policy *policy,
> +               struct cpufreq_freqs *freqs, unsigned int state)
> +{
> +       for_each_cpu(freqs->cpu, policy->cpus)
> +               __cpufreq_notify_transition(policy, freqs, state);
> +}
> +
>  /**
>   * cpufreq_notify_transition - call notifier chain and adjust_jiffies
>   * on frequency transition.
> @@ -351,8 +358,34 @@ static void __cpufreq_notify_transition(struct
> cpufreq_policy *policy,
>  void cpufreq_notify_transition(struct cpufreq_policy *policy,
>                 struct cpufreq_freqs *freqs, unsigned int state)
>  {
> -       for_each_cpu(freqs->cpu, policy->cpus)
> -               __cpufreq_notify_transition(policy, freqs, state);
> +       if ((state != CPUFREQ_PRECHANGE) && (state != CPUFREQ_POSTCHANGE))
> +               return cpufreq_notify_transition_each_cpu(policy, freqs, state);
> +
> +       /* Serialize pre-post notifications */
> +       mutex_lock(&policy->transition_lock);
> +       if (unlikely(WARN_ON(!policy->transition_ongoing &&
> +                               (state == CPUFREQ_POSTCHANGE)))) {
> +               mutex_unlock(&policy->transition_lock);
> +               return;
> +       }
> +
> +       if (state == CPUFREQ_PRECHANGE) {
> +               while (policy->transition_ongoing) {
> +                       mutex_unlock(&policy->transition_lock);
> +                       cpu_relax();
> +                       mutex_lock(&policy->transition_lock);
> +               }
> +
> +               policy->transition_ongoing = true;
> +               mutex_unlock(&policy->transition_lock);
> +       }
> +
> +       cpufreq_notify_transition_each_cpu(policy, freqs, state);
> +
> +       if (state == CPUFREQ_POSTCHANGE) {
> +               policy->transition_ongoing = false;
> +               mutex_unlock(&policy->transition_lock);
> +       }
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_notify_transition);
>
> @@ -950,6 +983,8 @@ static struct cpufreq_policy *cpufreq_policy_alloc(void)
>                 goto err_free_cpumask;
>
>         INIT_LIST_HEAD(&policy->policy_list);
> +       mutex_init(&policy->transition_lock);
> +
>         return policy;
>
>  err_free_cpumask:
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 0aba2a6c..bb76909 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -85,6 +85,8 @@ struct cpufreq_policy {
>         struct list_head        policy_list;
>         struct kobject          kobj;
>         struct completion       kobj_unregister;
> +       bool                    transition_ongoing; /* Tracks
> transition status */
> +       struct mutex            transition_lock;
>  };
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/