linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Viresh Kumar <viresh.kumar@linaro.org>
To: Rafael Wysocki <rjw@rjwysocki.net>
Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org,
	ashwin.chaugule@linaro.org,
	Viresh Kumar <viresh.kumar@linaro.org>,
	linux-kernel@vger.kernel.org (open list)
Subject: [PATCH V2 1/6] cpufreq: ondemand: Update sampling rate only for concerned policies
Date: Thu,  3 Dec 2015 09:37:49 +0530	[thread overview]
Message-ID: <c33ac949188af81735956c6c9e43204646b9f49a.1449115453.git.viresh.kumar@linaro.org> (raw)
In-Reply-To: <cover.1449115453.git.viresh.kumar@linaro.org>
In-Reply-To: <cover.1449115453.git.viresh.kumar@linaro.org>

We are comparing policy->governor against cpufreq_gov_ondemand to make
sure that we update sampling rate only for the concerned CPUs. But that
isn't enough.

In case of governor_per_policy, there can be multiple instances of
ondemand governor and we will always end up updating all of them with
current code. What we rather need to do, is to compare dbs_data with
poilcy->governor_data, which will match only for the policies governed
by dbs_data.

This code is also racy as the governor might be getting stopped at that
time and we may end up scheduling work for a policy, which we have just
disabled.

Fix that by protecting the entire function with &od_dbs_cdata.mutex,
which will prevent against races with policy START/STOP/etc.

After these locks are in place, we can safely get the policy via per-cpu
dbs_info.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq_ondemand.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 03ac6ce54042..089ca6a6ca02 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -252,20 +252,39 @@ static void update_sampling_rate(struct dbs_data *dbs_data,
 	od_tuners->sampling_rate = new_rate = max(new_rate,
 			dbs_data->min_sampling_rate);
 
+	/*
+	 * Lock governor so that governor start/stop can't execute in parallel.
+	 */
+	mutex_lock(&od_dbs_cdata.mutex);
+
 	for_each_online_cpu(cpu) {
 		struct cpufreq_policy *policy;
 		struct od_cpu_dbs_info_s *dbs_info;
+		struct cpu_dbs_info *cdbs;
+		struct cpu_common_dbs_info *shared;
 		unsigned long next_sampling, appointed_at;
 
-		policy = cpufreq_cpu_get(cpu);
-		if (!policy)
+		dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
+		cdbs = &dbs_info->cdbs;
+		shared = cdbs->shared;
+
+		/*
+		 * A valid shared and shared->policy means governor hasn't
+		 * stopped or exited yet.
+		 */
+		if (!shared || !shared->policy)
 			continue;
-		if (policy->governor != &cpufreq_gov_ondemand) {
-			cpufreq_cpu_put(policy);
+
+		policy = shared->policy;
+
+		/*
+		 * Update sampling rate for CPUs whose policy is governed by
+		 * dbs_data. In case of governor_per_policy, only a single
+		 * policy will be governed by dbs_data, otherwise there can be
+		 * multiple policies that are governed by the same dbs_data.
+		 */
+		if (dbs_data != policy->governor_data)
 			continue;
-		}
-		dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
-		cpufreq_cpu_put(policy);
 
 		if (!delayed_work_pending(&dbs_info->cdbs.dwork))
 			continue;
@@ -281,6 +300,8 @@ static void update_sampling_rate(struct dbs_data *dbs_data,
 
 		}
 	}
+
+	mutex_unlock(&od_dbs_cdata.mutex);
 }
 
 static ssize_t store_sampling_rate(struct dbs_data *dbs_data, const char *buf,
-- 
2.6.2.198.g614a2ac


       reply	other threads:[~2015-12-03  4:08 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1449115453.git.viresh.kumar@linaro.org>
2015-12-03  4:07 ` Viresh Kumar [this message]
2015-12-03  4:07 ` [PATCH V2 2/6] cpufreq: ondemand: Work is guaranteed to be pending Viresh Kumar
2015-12-03  4:07 ` [PATCH V2 3/6] cpufreq: governor: Pass policy as argument to ->gov_dbs_timer() Viresh Kumar
2015-12-03  4:07 ` [PATCH V2 4/6] cpufreq: governor: initialize/destroy timer_mutex with 'shared' Viresh Kumar
2015-12-03  4:07 ` [PATCH V2 5/6] cpufreq: governor: replace per-cpu delayed work with timers Viresh Kumar
2015-12-04  1:18   ` Rafael J. Wysocki
2015-12-04  6:11     ` Viresh Kumar
2015-12-05  2:14       ` Rafael J. Wysocki
2015-12-05  4:10         ` Viresh Kumar
2015-12-07  1:28           ` Rafael J. Wysocki
2015-12-07  7:50             ` Viresh Kumar
2015-12-07 22:43               ` Rafael J. Wysocki
2015-12-07 23:17                 ` Rafael J. Wysocki
2015-12-08  0:39                   ` [PATCH][experimantal] cpufreq: governor: Use an atomic variable for synchronization Rafael J. Wysocki
2015-12-08  6:59                     ` Viresh Kumar
2015-12-08 13:30                       ` Rafael J. Wysocki
2015-12-08 13:36                         ` Viresh Kumar
2015-12-08 14:19                           ` Rafael J. Wysocki
2015-12-08 13:55                             ` Viresh Kumar
2015-12-08 14:30                               ` Rafael J. Wysocki
2015-12-08 14:56                                 ` Viresh Kumar
2015-12-08 16:42                                   ` Rafael J. Wysocki
2015-12-08 16:34                                     ` Viresh Kumar
2015-12-08  6:46                   ` [PATCH V2 5/6] cpufreq: governor: replace per-cpu delayed work with timers Viresh Kumar
2015-12-08  6:56                 ` Viresh Kumar
2015-12-08 13:18                   ` Rafael J. Wysocki
2015-12-08 13:30                     ` Viresh Kumar
2015-12-08 14:04                       ` Rafael J. Wysocki
2015-12-04  6:13   ` [PATCH V3 " Viresh Kumar
2015-12-09  2:04     ` [PATCH V4 " Viresh Kumar
2015-12-09 22:06       ` Rafael J. Wysocki
2015-12-10  2:36         ` Viresh Kumar
2015-12-10 22:17           ` Rafael J. Wysocki
2015-12-11  1:42             ` Viresh Kumar
2015-12-03  4:07 ` [PATCH V2 6/6] cpufreq: ondemand: update update_sampling_rate() to make it more efficient Viresh Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c33ac949188af81735956c6c9e43204646b9f49a.1449115453.git.viresh.kumar@linaro.org \
    --to=viresh.kumar@linaro.org \
    --cc=ashwin.chaugule@linaro.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).