On 15-02-16, 02:13, Rafael J. Wysocki wrote:
> static void dbs_irq_work(struct irq_work *irq_work)
> @@ -357,6 +360,7 @@ static void dbs_update_util_handler(stru
> {
> struct cpu_dbs_info *cdbs = container_of(data, struct cpu_dbs_info,
> update_util);
> struct policy_dbs_info *policy_dbs = cdbs->policy_dbs;
> + u64 delta_ns;
>
> /*
> * The work may not be allowed to be queued up right now.
> @@ -364,17 +368,30 @@ static void dbs_update_util_handler(stru
> * - Work has already been queued up or is in progress.
> * - It is too early (too little time from the previous sample).
> */
> - if (atomic_inc_return(&policy_dbs->work_count) == 1) {
> - u64 delta_ns;
> + if (policy_dbs->work_in_progress)
> + return;
>
> - delta_ns = time - policy_dbs->last_sample_time;
> - if ((s64)delta_ns >= policy_dbs->sample_delay_ns) {
> - policy_dbs->last_sample_time = time;
> - gov_queue_irq_work(policy_dbs);
> - return;
> - }
> - }
> - atomic_dec(&policy_dbs->work_count);
> + /*
> + * If the reads below are reordered before the check above, the value
> + * of sample_delay_ns used in the computation may be stale.
> + */
> + smp_rmb();
> + delta_ns = time - policy_dbs->last_sample_time;
> + if ((s64)delta_ns < policy_dbs->sample_delay_ns)
> + return;
> +
> + /*
> + * If the policy is not shared, the irq_work may be queued up right away
> + * at this point. Otherwise, we need to ensure that only one of the
> + * CPUs sharing the policy will do that.
> + */
> + if (policy_dbs->is_shared &&
> + !atomic_add_unless(&policy_dbs->work_count, 1, 1))
> + return;
> +
> + policy_dbs->last_sample_time = time;
> + policy_dbs->work_in_progress = true;
> + gov_queue_irq_work(policy_dbs);
> }
>
> static struct policy_dbs_info *alloc_policy_dbs_info(struct cpufreq_policy
> *policy,
> @@ -551,6 +568,8 @@ static int cpufreq_governor_start(struct
> if (!policy->cur)
> return -EINVAL;
>
> + policy_dbs->is_shared = policy_is_shared(policy);
> +
> sampling_rate = dbs_data->sampling_rate;
> ignore_nice = dbs_data->ignore_nice_load;
>
> Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
> +++ linux-pm/drivers/cpufreq/cpufreq_governor.h
> @@ -130,6 +130,9 @@ struct policy_dbs_info {
> /* dbs_data may be shared between multiple policy objects */
> struct dbs_data *dbs_data;
> struct list_head list;
> + /* Status indicators */
> + bool is_shared; /* This object is used by multiple CPUs */
> + bool work_in_progress; /* Work is being queued up or in progress */
> };
>
> static inline void gov_update_sample_delay(struct policy_dbs_info
> *policy_dbs,
--
viresh
