Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1334180

Re: [PATCH 2/9] cpufreq: governor: Avoid atomic operations in hot paths

Show all headers | View raw

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/9] cpufreq governor improvements "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-15 02:30 +0100
  [PATCH 1/9] cpufreq: governor: Simplify gov_cancel_work() slightly "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-15 02:30 +0100
    Re: [PATCH 1/9] cpufreq: governor: Simplify gov_cancel_work()  slightly Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-15 06:50 +0100
  [PATCH 2/9] cpufreq: governor: Avoid atomic operations in hot paths "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-15 02:30 +0100
    Re: [PATCH 2/9] cpufreq: governor: Avoid atomic operations in hot  paths Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-15 07:20 +0100
    Re: [PATCH 2/9] cpufreq: governor: Avoid atomic operations in hot  paths Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-15 09:30 +0100
  [PATCH 7/9] cpufreq: governor: Move rate_mult to struct policy_dbs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-15 02:30 +0100
    Re: [PATCH 7/9] cpufreq: governor: Move rate_mult to struct  policy_dbs Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-15 10:00 +0100
  [PATCH 3/9] cpufreq: governor: Fix nice contribution computation in dbs_check_cpu() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-15 02:30 +0100
    Re: [PATCH 3/9] cpufreq: governor: Fix nice contribution computation  in dbs_check_cpu() Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-15 09:30 +0100
  [PATCH 9/9] cpufreq: governor: Use microseconds in sample delay computations "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-15 02:30 +0100
    Re: [PATCH 9/9] cpufreq: governor: Use microseconds in sample delay  computations Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-15 10:00 +0100
  [PATCH 8/9] cpufreq: ondemand: Simplify conditionals in od_dbs_timer() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-15 02:30 +0100
    Re: [PATCH 8/9] cpufreq: ondemand: Simplify conditionals in  od_dbs_timer() Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-15 10:00 +0100

csiph-web