From: Prarit Bhargava <prarit@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: robert.schoene@tu-dresden.de, sboyd@codeaurora.org,
Prarit Bhargava <prarit@redhat.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Viresh Kumar <viresh.kumar@linaro.org>,
linux-pm@vger.kernel.org
Subject: [PATCH 3/5] cpufreq, dbs_data->usage count must be atomic
Date: Wed, 5 Nov 2014 09:53:57 -0500 [thread overview]
Message-ID: <1415199239-19019-4-git-send-email-prarit@redhat.com> (raw)
In-Reply-To: <1415199239-19019-1-git-send-email-prarit@redhat.com>
The usage_count value can be modified from several cpus concurrently if
!CPUFREQ_HAVE_GOVERNOR_PER_POLICY. This leads to a variety of panics in
which the usage_count is negative or exceeds the number of cpus in the
system. It must be switched to atomic_t and protected with a mutex to make sure
that future read/writes obtain the correct data.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
drivers/cpufreq/cpufreq_governor.c | 16 ++++++++++++----
drivers/cpufreq/cpufreq_governor.h | 3 ++-
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index 1b44496..32ad9db 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -265,7 +265,9 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
if (have_governor_per_policy()) {
WARN_ON(dbs_data);
} else if (dbs_data) {
- dbs_data->usage_count++;
+ mutex_lock(&dbs_data->usage_count_mutex);
+ atomic_inc(&dbs_data->usage_count);
+ mutex_unlock(&dbs_data->usage_count_mutex);
policy->governor_data = dbs_data;
return 0;
}
@@ -277,7 +279,10 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
}
dbs_data->cdata = cdata;
- dbs_data->usage_count = 1;
+ mutex_init(&dbs_data->usage_count_mutex);
+ mutex_lock(&dbs_data->usage_count_mutex);
+ atomic_set(&dbs_data->usage_count, 1);
+ mutex_unlock(&dbs_data->usage_count_mutex);
rc = cdata->init(dbs_data);
if (rc) {
pr_err("%s: POLICY_INIT: init() failed\n", __func__);
@@ -322,7 +327,8 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
return 0;
case CPUFREQ_GOV_POLICY_EXIT:
- if (!--dbs_data->usage_count) {
+ mutex_lock(&dbs_data->usage_count_mutex);
+ if (atomic_dec_and_test(&dbs_data->usage_count)) {
sysfs_remove_group(get_governor_parent_kobj(policy),
get_sysfs_attr(dbs_data));
@@ -338,9 +344,11 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
}
cdata->exit(dbs_data);
+ mutex_unlock(&dbs_data->usage_count_mutex);
kfree(dbs_data);
cdata->gdbs_data = NULL;
- }
+ } else
+ mutex_unlock(&dbs_data->usage_count_mutex);
policy->governor_data = NULL;
return 0;
diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
index cc401d1..53ca449 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -219,7 +219,8 @@ struct common_dbs_data {
struct dbs_data {
struct common_dbs_data *cdata;
unsigned int min_sampling_rate;
- int usage_count;
+ struct mutex usage_count_mutex;
+ atomic_t usage_count;
void *tuners;
/* dbs_mutex protects dbs_enable in governor start/stop */
--
1.7.9.3
next prev parent reply other threads:[~2014-11-05 14:53 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-05 14:53 [PATCH 0/5] cpufreq, fix locking and data issues Prarit Bhargava
2014-11-05 14:53 ` [PATCH 1/5] cpufreq, do not return stale data to userspace Prarit Bhargava
2014-11-05 14:53 ` [PATCH 2/5] cpufreq, fix locking around CPUFREQ_GOV_POLICY_EXIT calls Prarit Bhargava
2014-11-10 10:44 ` Viresh Kumar
2014-11-10 12:26 ` Prarit Bhargava
2014-11-11 3:37 ` Viresh Kumar
2014-11-11 12:15 ` Prarit Bhargava
2014-11-11 13:07 ` Viresh Kumar
2014-11-13 21:58 ` Saravana Kannan
2014-11-05 14:53 ` Prarit Bhargava [this message]
2014-11-08 1:57 ` [PATCH 3/5] cpufreq, dbs_data->usage count must be atomic Rafael J. Wysocki
2014-11-11 3:40 ` Viresh Kumar
2014-11-05 14:53 ` [PATCH 4/5] cpufreq, policy->initialized " Prarit Bhargava
2014-11-08 1:59 ` Rafael J. Wysocki
2014-11-11 3:55 ` Viresh Kumar
2014-11-05 14:53 ` [PATCH 5/5] cpufreq, add BUG() messages in critical paths to aid debugging failures Prarit Bhargava
2014-11-08 2:00 ` Rafael J. Wysocki
2014-11-08 13:33 ` Prarit Bhargava
2014-11-08 21:46 ` Rafael J. Wysocki
2014-11-09 14:12 ` Prarit Bhargava
2014-11-11 4:23 ` Viresh Kumar
2014-11-11 12:18 ` Prarit Bhargava
2014-11-11 13:11 ` 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=1415199239-19019-4-git-send-email-prarit@redhat.com \
--to=prarit@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=robert.schoene@tu-dresden.de \
--cc=sboyd@codeaurora.org \
--cc=viresh.kumar@linaro.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.