public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: linux-kernel@vger.kernel.org,
	Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>,
	Dave Jones <davej@redhat.com>, Thomas Renninger <trenn@suse.de>,
	cpufreq@vger.kernel.org, kernel-testers@vger.kernel.org,
	Ingo Molnar <mingo@elte.hu>,
	rjw@sisk.pl, Dave Young <hidave.darkstar@gmail.com>,
	Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>,
	Shaohua Li <shaohua.li@intel.com>,
	Rusty Russell <rusty@rustcorp.com.au>,
	sven.wegener@stealer.net
Subject: [patch 2.6.30 3/4] cpufreq add gov mutex
Date: Fri, 03 Jul 2009 11:25:17 -0400	[thread overview]
Message-ID: <20090703152714.953585501@polymtl.ca> (raw)
In-Reply-To: 20090703152514.903448283@polymtl.ca

[-- Attachment #1: cpufreq-add-gov-mutex.patch --]
[-- Type: text/plain, Size: 5771 bytes --]

Using the cpufreq_gov_mutex to protect internal governor data structures. The
policy rwsem write lock nests inside this mutex.  The policy rwsem is taken in
the timer handler, and therefore cannot be held while doing a sync teardown of
the timer.  This cpufreq_gov_mutex lock protects init/teardown of governor
timers. This is why this lock, although it protects a data structure located
within the governors, is located in cpufreq.c.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
CC: rjw@sisk.pl
CC: mingo@elte.hu
CC: Shaohua Li <shaohua.li@intel.com>
CC: Pekka Enberg <penberg@cs.helsinki.fi>
CC: Dave Young <hidave.darkstar@gmail.com>
CC: "Rafael J. Wysocki" <rjw@sisk.pl>
CC: Rusty Russell <rusty@rustcorp.com.au>
CC: sven.wegener@stealer.net
CC: cpufreq@vger.kernel.org
CC: Thomas Renninger <trenn@suse.de>
---
 drivers/cpufreq/cpufreq.c |   38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

Index: linux-2.6-lttng/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-2.6-lttng.orig/drivers/cpufreq/cpufreq.c	2009-07-03 09:48:35.000000000 -0400
+++ linux-2.6-lttng/drivers/cpufreq/cpufreq.c	2009-07-03 10:12:44.000000000 -0400
@@ -133,6 +133,17 @@ pure_initcall(init_cpufreq_transition_no
 static LIST_HEAD(cpufreq_governor_list);
 static DEFINE_MUTEX(cpufreq_governor_mutex);
 
+/*
+ * Using the cpufreq_gov_mutex to protect internal governor
+ * data structures. The policy rwsem write lock nests inside this mutex.
+ * The policy rwsem is taken in the timer handler, and therefore cannot be
+ * held while doing a sync teardown of the timer.
+ * This cpufreq_gov_mutex lock protects init/teardown of governor timers.
+ * This is why this lock, although it protects a data structure located within
+ * the governors, is here.
+ */
+static DEFINE_MUTEX(cpufreq_gov_mutex);
+
 struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu)
 {
 	struct cpufreq_policy *data;
@@ -725,6 +736,7 @@ static ssize_t store(struct kobject *kob
 	if (!policy)
 		goto no_policy;
 
+	mutex_lock(&cpufreq_gov_mutex);
 	if (lock_policy_rwsem_write(policy->cpu) < 0)
 		goto fail;
 
@@ -735,6 +747,7 @@ static ssize_t store(struct kobject *kob
 
 	unlock_policy_rwsem_write(policy->cpu);
 fail:
+	mutex_unlock(&cpufreq_gov_mutex);
 	cpufreq_cpu_put(policy);
 no_policy:
 	return ret;
@@ -823,6 +836,7 @@ static int cpufreq_add_dev(struct sys_de
 
 	/* Initially set CPU itself as the policy_cpu */
 	per_cpu(policy_cpu, cpu) = cpu;
+	mutex_lock(&cpufreq_gov_mutex);
 	ret = (lock_policy_rwsem_write(cpu) < 0);
 	WARN_ON(ret);
 
@@ -875,7 +889,7 @@ static int cpufreq_add_dev(struct sys_de
 					cpufreq_driver->exit(policy);
 				ret = -EBUSY;
 				cpufreq_cpu_put(managed_policy);
-				goto err_free_cpumask;
+				goto err_unlock_gov_mutex;
 			}
 
 			spin_lock_irqsave(&cpufreq_driver_lock, flags);
@@ -964,6 +978,7 @@ static int cpufreq_add_dev(struct sys_de
 	}
 
 	unlock_policy_rwsem_write(cpu);
+	mutex_unlock(&cpufreq_gov_mutex);
 
 	kobject_uevent(&policy->kobj, KOBJ_ADD);
 	module_put(cpufreq_driver->owner);
@@ -989,6 +1004,8 @@ out_driver_exit:
 
 err_unlock_policy:
 	unlock_policy_rwsem_write(cpu);
+err_unlock_gov_mutex:
+	mutex_unlock(&cpufreq_gov_mutex);
 err_free_cpumask:
 	free_cpumask_var(policy->cpus);
 err_free_policy:
@@ -1028,6 +1045,7 @@ static int __cpufreq_remove_dev(struct s
 		spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
 		cpufreq_debug_enable_ratelimit();
 		unlock_policy_rwsem_write(cpu);
+		mutex_unlock(&cpufreq_gov_mutex);
 		return -EINVAL;
 	}
 	per_cpu(cpufreq_cpu_data, cpu) = NULL;
@@ -1045,6 +1063,7 @@ static int __cpufreq_remove_dev(struct s
 		cpufreq_cpu_put(data);
 		cpufreq_debug_enable_ratelimit();
 		unlock_policy_rwsem_write(cpu);
+		mutex_unlock(&cpufreq_gov_mutex);
 		return 0;
 	}
 #endif
@@ -1088,9 +1107,13 @@ static int __cpufreq_remove_dev(struct s
 #endif
 
 	unlock_policy_rwsem_write(cpu);
-
+	/*
+	 * Calling only with the cpufreq_gov_mutex held because
+	 * sync timer teardown has locking dependency with policy rwsem.
+	 */
 	if (cpufreq_driver->target)
 		__cpufreq_governor(data, CPUFREQ_GOV_STOP);
+	mutex_unlock(&cpufreq_gov_mutex);
 
 	kobject_put(&data->kobj);
 
@@ -1123,6 +1146,7 @@ static int cpufreq_remove_dev(struct sys
 	if (cpu_is_offline(cpu))
 		return 0;
 
+	mutex_lock(&cpufreq_gov_mutex);
 	if (unlikely(lock_policy_rwsem_write(cpu)))
 		BUG();
 
@@ -1506,12 +1530,16 @@ int cpufreq_driver_target(struct cpufreq
 	if (!policy)
 		goto no_policy;
 
-	if (unlikely(lock_policy_rwsem_write(policy->cpu)))
+	mutex_lock(&cpufreq_gov_mutex);
+	if (unlikely(lock_policy_rwsem_write(policy->cpu))) {
+		mutex_unlock(&cpufreq_gov_mutex);
 		goto fail;
+	}
 
 	ret = __cpufreq_driver_target(policy, target_freq, relation);
 
 	unlock_policy_rwsem_write(policy->cpu);
+	mutex_unlock(&cpufreq_gov_mutex);
 
 fail:
 	cpufreq_cpu_put(policy);
@@ -1769,7 +1797,9 @@ int cpufreq_update_policy(unsigned int c
 		goto no_policy;
 	}
 
+	mutex_lock(&cpufreq_gov_mutex);
 	if (unlikely(lock_policy_rwsem_write(cpu))) {
+		mutex_unlock(&cpufreq_gov_mutex);
 		ret = -EINVAL;
 		goto fail;
 	}
@@ -1798,6 +1828,7 @@ int cpufreq_update_policy(unsigned int c
 	ret = __cpufreq_set_policy(data, &policy);
 
 	unlock_policy_rwsem_write(cpu);
+	mutex_unlock(&cpufreq_gov_mutex);
 
 fail:
 	cpufreq_cpu_put(data);
@@ -1821,6 +1852,7 @@ static int __cpuinit cpufreq_cpu_callbac
 			break;
 		case CPU_DOWN_PREPARE:
 		case CPU_DOWN_PREPARE_FROZEN:
+			mutex_lock(&cpufreq_gov_mutex);
 			if (unlikely(lock_policy_rwsem_write(cpu)))
 				BUG();
 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

  parent reply	other threads:[~2009-07-03 15:57 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-03 15:25 [patch 2.6.30 0/4] Fix cpufreq locking dependency (resend) Mathieu Desnoyers
2009-07-03 15:25 ` [patch 2.6.30 1/4] remove rwsem lock from CPUFREQ_GOV_STOP call (second call site) Mathieu Desnoyers
2009-07-03 15:25 ` [patch 2.6.30 2/4] CPUFREQ: fix (utter) cpufreq_add_dev mess Mathieu Desnoyers
2009-07-03 19:24   ` Pallipadi, Venkatesh
2009-07-03 19:41     ` Mathieu Desnoyers
2009-07-06 18:02       ` Pallipadi, Venkatesh
2009-07-03 15:25 ` Mathieu Desnoyers [this message]
2009-07-03 19:11   ` [patch 2.6.30 3/4] cpufreq add gov mutex Pallipadi, Venkatesh
2009-07-03 19:36     ` Mathieu Desnoyers
2009-07-06 18:50       ` Pallipadi, Venkatesh
2009-07-07 22:59         ` Mathieu Desnoyers
2009-07-03 15:25 ` [patch 2.6.30 4/4] cpufreq ondemand and conservative remove dbs_mutex Mathieu Desnoyers

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=20090703152714.953585501@polymtl.ca \
    --to=mathieu.desnoyers@polymtl.ca \
    --cc=cpufreq@vger.kernel.org \
    --cc=davej@redhat.com \
    --cc=hidave.darkstar@gmail.com \
    --cc=kernel-testers@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=penberg@cs.helsinki.fi \
    --cc=rjw@sisk.pl \
    --cc=rusty@rustcorp.com.au \
    --cc=shaohua.li@intel.com \
    --cc=sven.wegener@stealer.net \
    --cc=trenn@suse.de \
    --cc=venkatesh.pallipadi@intel.com \
    /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