From: venkatesh.pallipadi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
To: Dave Jones <davej-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
cpufreq-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
kernel-testers-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>,
"Rafael J. Wysocki" <rjw-KKrjLPT3xs0@public.gmane.org>,
Dave Young
<hidave.darkstar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
Pekka Enberg <penberg-bbCR+/B0CizivPeTLB3BmA@public.gmane.org>,
Mathieu Desnoyers
<mathieu.desnoyers-scC8bbJcJLCw5LPnMra/2Q@public.gmane.org>,
Thomas Renninger <trenn-l3A5Bk7waGM@public.gmane.org>,
Venkatesh Pallipadi
<venkatesh.pallipadi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Subject: [patch 4/4] cpufreq: Cleanup locking in conservative governor
Date: Thu, 02 Jul 2009 17:08:33 -0700 [thread overview]
Message-ID: <20090703000924.273906000@intel.com> (raw)
In-Reply-To: 20090703000829.735976000@intel.com
[-- Attachment #1: 0004-cpufreq-Cleanup-locking-in-conservative-governor.patch --]
[-- Type: text/plain, Size: 4330 bytes --]
Redesign the locking inside conservative driver. Make dbs_mutex handle all the
global state changes inside the driver and invent a new percpu mutex
to serialize percpu timer and frequency limit change.
Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/cpufreq/cpufreq_conservative.c | 34 ++++++++++++-------------------
1 files changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index 58889f2..5749050 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -63,7 +63,12 @@ struct cpu_dbs_info_s {
unsigned int down_skip;
unsigned int requested_freq;
int cpu;
- unsigned int enable:1;
+ /*
+ * percpu mutex that serializes governor limit change with
+ * do_dbs_timer invocation. We do not want do_dbs_timer to run
+ * when user is changing the governor or limits.
+ */
+ struct mutex timer_mutex;
};
static DEFINE_PER_CPU(struct cpu_dbs_info_s, cpu_dbs_info);
@@ -71,9 +76,7 @@ static unsigned int dbs_enable; /* number of CPUs using this policy */
/*
* dbs_mutex protects data in dbs_tuners_ins from concurrent changes on
- * different CPUs. It protects dbs_enable in governor start/stop. It also
- * serializes governor limit_change with do_dbs_timer. We do not want
- * do_dbs_timer to run when user is changing the governor or limits.
+ * different CPUs. It protects dbs_enable in governor start/stop.
*/
static DEFINE_MUTEX(dbs_mutex);
@@ -138,9 +141,6 @@ dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
struct cpufreq_policy *policy;
- if (!this_dbs_info->enable)
- return 0;
-
policy = this_dbs_info->cur_policy;
/*
@@ -483,17 +483,12 @@ static void do_dbs_timer(struct work_struct *work)
delay -= jiffies % delay;
- mutex_lock(&dbs_mutex);
-
- if (!dbs_info->enable) {
- mutex_unlock(&dbs_mutex);
- return;
- }
+ mutex_lock(&dbs_info->timer_mutex);
dbs_check_cpu(dbs_info);
queue_delayed_work_on(cpu, kconservative_wq, &dbs_info->work, delay);
- mutex_unlock(&dbs_mutex);
+ mutex_unlock(&dbs_info->timer_mutex);
}
static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info)
@@ -502,7 +497,6 @@ static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info)
int delay = usecs_to_jiffies(dbs_tuners_ins.sampling_rate);
delay -= jiffies % delay;
- dbs_info->enable = 1;
INIT_DELAYED_WORK_DEFERRABLE(&dbs_info->work, do_dbs_timer);
queue_delayed_work_on(dbs_info->cpu, kconservative_wq, &dbs_info->work,
delay);
@@ -510,7 +504,6 @@ static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info)
static inline void dbs_timer_exit(struct cpu_dbs_info_s *dbs_info)
{
- dbs_info->enable = 0;
cancel_delayed_work_sync(&dbs_info->work);
}
@@ -529,9 +522,6 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
if ((!cpu_online(cpu)) || (!policy->cur))
return -EINVAL;
- if (this_dbs_info->enable) /* Already enabled */
- break;
-
mutex_lock(&dbs_mutex);
rc = sysfs_create_group(&policy->kobj, &dbs_attr_group);
@@ -555,6 +545,7 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
this_dbs_info->down_skip = 0;
this_dbs_info->requested_freq = policy->cur;
+ mutex_init(&this_dbs_info->timer_mutex);
dbs_enable++;
/*
* Start the timerschedule work, when this governor
@@ -596,6 +587,7 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
mutex_lock(&dbs_mutex);
sysfs_remove_group(&policy->kobj, &dbs_attr_group);
dbs_enable--;
+ mutex_destroy(&this_dbs_info->timer_mutex);
/*
* Stop the timerschedule work, when this governor
@@ -611,7 +603,7 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
break;
case CPUFREQ_GOV_LIMITS:
- mutex_lock(&dbs_mutex);
+ mutex_lock(&this_dbs_info->timer_mutex);
if (policy->max < this_dbs_info->cur_policy->cur)
__cpufreq_driver_target(
this_dbs_info->cur_policy,
@@ -620,7 +612,7 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
__cpufreq_driver_target(
this_dbs_info->cur_policy,
policy->min, CPUFREQ_RELATION_L);
- mutex_unlock(&dbs_mutex);
+ mutex_unlock(&this_dbs_info->timer_mutex);
break;
}
--
1.6.0.6
--
WARNING: multiple messages have this Message-ID (diff)
From: venkatesh.pallipadi@intel.com
To: "Dave Jones" <davej@redhat.com>
Cc: linux-kernel@vger.kernel.org, cpufreq@vger.kernel.org,
kernel-testers@vger.kernel.org, "Ingo Molnar" <mingo@elte.hu>,
"Rafael J. Wysocki" <rjw@sisk.pl>,
"Dave Young" <hidave.darkstar@gmail.com>,
"Pekka Enberg" <penberg@cs.helsinki.fi>,
"Mathieu Desnoyers" <mathieu.desnoyers@polymtl.ca>,
"Thomas Renninger" <trenn@suse.de>,
"Venkatesh Pallipadi" <venkatesh.pallipadi@intel.com>
Subject: [patch 4/4] cpufreq: Cleanup locking in conservative governor
Date: Thu, 02 Jul 2009 17:08:33 -0700 [thread overview]
Message-ID: <20090703000924.273906000@intel.com> (raw)
In-Reply-To: 20090703000829.735976000@intel.com
[-- Attachment #1: 0004-cpufreq-Cleanup-locking-in-conservative-governor.patch --]
[-- Type: text/plain, Size: 4301 bytes --]
Redesign the locking inside conservative driver. Make dbs_mutex handle all the
global state changes inside the driver and invent a new percpu mutex
to serialize percpu timer and frequency limit change.
Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
---
drivers/cpufreq/cpufreq_conservative.c | 34 ++++++++++++-------------------
1 files changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index 58889f2..5749050 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -63,7 +63,12 @@ struct cpu_dbs_info_s {
unsigned int down_skip;
unsigned int requested_freq;
int cpu;
- unsigned int enable:1;
+ /*
+ * percpu mutex that serializes governor limit change with
+ * do_dbs_timer invocation. We do not want do_dbs_timer to run
+ * when user is changing the governor or limits.
+ */
+ struct mutex timer_mutex;
};
static DEFINE_PER_CPU(struct cpu_dbs_info_s, cpu_dbs_info);
@@ -71,9 +76,7 @@ static unsigned int dbs_enable; /* number of CPUs using this policy */
/*
* dbs_mutex protects data in dbs_tuners_ins from concurrent changes on
- * different CPUs. It protects dbs_enable in governor start/stop. It also
- * serializes governor limit_change with do_dbs_timer. We do not want
- * do_dbs_timer to run when user is changing the governor or limits.
+ * different CPUs. It protects dbs_enable in governor start/stop.
*/
static DEFINE_MUTEX(dbs_mutex);
@@ -138,9 +141,6 @@ dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
struct cpufreq_policy *policy;
- if (!this_dbs_info->enable)
- return 0;
-
policy = this_dbs_info->cur_policy;
/*
@@ -483,17 +483,12 @@ static void do_dbs_timer(struct work_struct *work)
delay -= jiffies % delay;
- mutex_lock(&dbs_mutex);
-
- if (!dbs_info->enable) {
- mutex_unlock(&dbs_mutex);
- return;
- }
+ mutex_lock(&dbs_info->timer_mutex);
dbs_check_cpu(dbs_info);
queue_delayed_work_on(cpu, kconservative_wq, &dbs_info->work, delay);
- mutex_unlock(&dbs_mutex);
+ mutex_unlock(&dbs_info->timer_mutex);
}
static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info)
@@ -502,7 +497,6 @@ static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info)
int delay = usecs_to_jiffies(dbs_tuners_ins.sampling_rate);
delay -= jiffies % delay;
- dbs_info->enable = 1;
INIT_DELAYED_WORK_DEFERRABLE(&dbs_info->work, do_dbs_timer);
queue_delayed_work_on(dbs_info->cpu, kconservative_wq, &dbs_info->work,
delay);
@@ -510,7 +504,6 @@ static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info)
static inline void dbs_timer_exit(struct cpu_dbs_info_s *dbs_info)
{
- dbs_info->enable = 0;
cancel_delayed_work_sync(&dbs_info->work);
}
@@ -529,9 +522,6 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
if ((!cpu_online(cpu)) || (!policy->cur))
return -EINVAL;
- if (this_dbs_info->enable) /* Already enabled */
- break;
-
mutex_lock(&dbs_mutex);
rc = sysfs_create_group(&policy->kobj, &dbs_attr_group);
@@ -555,6 +545,7 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
this_dbs_info->down_skip = 0;
this_dbs_info->requested_freq = policy->cur;
+ mutex_init(&this_dbs_info->timer_mutex);
dbs_enable++;
/*
* Start the timerschedule work, when this governor
@@ -596,6 +587,7 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
mutex_lock(&dbs_mutex);
sysfs_remove_group(&policy->kobj, &dbs_attr_group);
dbs_enable--;
+ mutex_destroy(&this_dbs_info->timer_mutex);
/*
* Stop the timerschedule work, when this governor
@@ -611,7 +603,7 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
break;
case CPUFREQ_GOV_LIMITS:
- mutex_lock(&dbs_mutex);
+ mutex_lock(&this_dbs_info->timer_mutex);
if (policy->max < this_dbs_info->cur_policy->cur)
__cpufreq_driver_target(
this_dbs_info->cur_policy,
@@ -620,7 +612,7 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
__cpufreq_driver_target(
this_dbs_info->cur_policy,
policy->min, CPUFREQ_RELATION_L);
- mutex_unlock(&dbs_mutex);
+ mutex_unlock(&this_dbs_info->timer_mutex);
break;
}
--
1.6.0.6
--
next prev parent reply other threads:[~2009-07-03 0:08 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-03 0:08 [patch 0/4] Take care of cpufreq lockdep issues (take 2) venkatesh.pallipadi-ral2JQCrhuEAvxtiuMwx3w
2009-07-03 0:08 ` venkatesh.pallipadi
2009-07-03 0:08 ` [patch 1/4] cpufreq: Eliminate the recent lockdep warnings in cpufreq venkatesh.pallipadi-ral2JQCrhuEAvxtiuMwx3w
2009-07-03 0:08 ` venkatesh.pallipadi
[not found] ` <20090703000923.800507000-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2009-07-03 1:06 ` Mathieu Desnoyers
2009-07-03 1:06 ` Mathieu Desnoyers
2009-07-03 2:04 ` Pallipadi, Venkatesh
2009-07-03 2:04 ` Pallipadi, Venkatesh
[not found] ` <7E82351C108FA840AB1866AC776AEC4669BFEF78-osO9UTpF0URqS6EAlXoojrfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2009-07-03 2:25 ` Mathieu Desnoyers
2009-07-03 2:25 ` Mathieu Desnoyers
2009-07-03 11:41 ` Thomas Renninger
2009-07-03 11:41 ` Thomas Renninger
[not found] ` <200907031341.19141.trenn-l3A5Bk7waGM@public.gmane.org>
2009-07-03 14:28 ` Pallipadi, Venkatesh
2009-07-03 14:28 ` Pallipadi, Venkatesh
[not found] ` <7E82351C108FA840AB1866AC776AEC4669BFF050-osO9UTpF0URqS6EAlXoojrfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2009-07-06 11:19 ` Thomas Renninger
2009-07-06 11:19 ` Thomas Renninger
2009-07-03 0:08 ` [patch 2/4] cpufreq: Mark policy_rwsem as going static in cpufreq.c wont be exported venkatesh.pallipadi-ral2JQCrhuEAvxtiuMwx3w
2009-07-03 0:08 ` venkatesh.pallipadi
2009-07-03 0:08 ` [patch 3/4] cpufreq: Cleanup locking in ondemand governor venkatesh.pallipadi-ral2JQCrhuEAvxtiuMwx3w
2009-07-03 0:08 ` venkatesh.pallipadi
2009-07-03 0:08 ` venkatesh.pallipadi-ral2JQCrhuEAvxtiuMwx3w [this message]
2009-07-03 0:08 ` [patch 4/4] cpufreq: Cleanup locking in conservative governor venkatesh.pallipadi
[not found] ` <20090703000829.735976000-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2009-07-03 2:23 ` [PATCH] CPUFREQ: fix (utter) cpufreq_add_dev mess v1 Mathieu Desnoyers
2009-07-03 2:23 ` Mathieu Desnoyers
2009-07-03 6:54 ` [patch 0/4] Take care of cpufreq lockdep issues (take 2) Ingo Molnar
2009-07-03 6:54 ` Ingo Molnar
[not found] ` <20090703065427.GA32687-X9Un+BFzKDI@public.gmane.org>
2009-07-03 14:06 ` Mathieu Desnoyers
2009-07-03 14:06 ` Mathieu Desnoyers
2009-07-03 14:31 ` Pallipadi, Venkatesh
2009-07-03 14:31 ` Pallipadi, Venkatesh
[not found] ` <7E82351C108FA840AB1866AC776AEC4669BFF052-osO9UTpF0URqS6EAlXoojrfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2009-07-03 18:48 ` Ingo Molnar
2009-07-03 18:48 ` Ingo Molnar
2009-07-06 18:52 ` Pallipadi, Venkatesh
2009-07-06 18:52 ` Pallipadi, Venkatesh
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=20090703000924.273906000@intel.com \
--to=venkatesh.pallipadi-ral2jqcrhueavxtiumwx3w@public.gmane.org \
--cc=cpufreq-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=davej-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=hidave.darkstar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=kernel-testers-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=mathieu.desnoyers-scC8bbJcJLCw5LPnMra/2Q@public.gmane.org \
--cc=mingo-X9Un+BFzKDI@public.gmane.org \
--cc=penberg-bbCR+/B0CizivPeTLB3BmA@public.gmane.org \
--cc=rjw-KKrjLPT3xs0@public.gmane.org \
--cc=trenn-l3A5Bk7waGM@public.gmane.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.