public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Gautham R Shenoy <ego@in.ibm.com>
To: linux-kernel@vger.kernel.org,
	Zdenek Kabelac <zdenek.kabelac@gmail.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Oleg Nesterov <oleg@tv-sign.ru>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	"Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@elte.hu>,
	Srivatsa Vaddagiri <vatsa@in.ibm.com>
Subject: [PATCH 8/8] cpufreq: Nest down_write/read(cpufreq_rwsem) within get_online_cpus()/put_online_cpus()
Date: Tue, 29 Apr 2008 18:35:05 +0530	[thread overview]
Message-ID: <20080429130505.GI23562@in.ibm.com> (raw)
In-Reply-To: <20080429125659.GA23562@in.ibm.com>

cpufreq: Nest down_write/read(cpufreq_rwsem) with get_online_cpus()

From: Gautham R Shenoy <ego@in.ibm.com>

On the CPU-Hotplug callback path, the cpufreq_rwsem has a dependency
with the cpu_hotplug lock. However on the read-path, this dependency is not
honoured resulting in a possible deadlock when one tries to change
the cpufreq governor when a CPU-Hotplug is in progress.

This patch nests the down_write/read(cpufreq_rwsem) with
get_online_cpus()/put_online_cpus() pair.

Also, it introduces a try_get_online_cpus() in the do_dbs_timer() work-item
code, since we cannot do get_online_cpus() from within work items.

Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
---

 drivers/cpufreq/cpufreq.c          |   11 +++++++++--
 drivers/cpufreq/cpufreq_ondemand.c |    9 +++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 35a26a3..087f8fc 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -71,11 +71,12 @@ int lock_policy_rwsem_##mode						\
 {									\
 	int policy_cpu = per_cpu(policy_cpu, cpu);			\
 	BUG_ON(policy_cpu == -1);					\
-	down_##mode(&per_cpu(cpu_policy_rwsem, policy_cpu));		\
+	get_online_cpus();						\
 	if (unlikely(!cpu_online(cpu))) {				\
-		up_##mode(&per_cpu(cpu_policy_rwsem, policy_cpu));	\
+		put_online_cpus();					\
 		return -1;						\
 	}								\
+	down_##mode(&per_cpu(cpu_policy_rwsem, policy_cpu));		\
 									\
 	return 0;							\
 }
@@ -91,6 +92,7 @@ void unlock_policy_rwsem_read(int cpu)
 	int policy_cpu = per_cpu(policy_cpu, cpu);
 	BUG_ON(policy_cpu == -1);
 	up_read(&per_cpu(cpu_policy_rwsem, policy_cpu));
+	put_online_cpus();
 }
 EXPORT_SYMBOL_GPL(unlock_policy_rwsem_read);
 
@@ -99,6 +101,7 @@ void unlock_policy_rwsem_write(int cpu)
 	int policy_cpu = per_cpu(policy_cpu, cpu);
 	BUG_ON(policy_cpu == -1);
 	up_write(&per_cpu(cpu_policy_rwsem, policy_cpu));
+	put_online_cpus();
 }
 EXPORT_SYMBOL_GPL(unlock_policy_rwsem_write);
 
@@ -1818,7 +1821,9 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
 	cpufreq_driver = driver_data;
 	spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
 
+	get_online_cpus();
 	ret = sysdev_driver_register(&cpu_sysdev_class,&cpufreq_sysdev_driver);
+	put_online_cpus();
 
 	if ((!ret) && !(cpufreq_driver->flags & CPUFREQ_STICKY)) {
 		int i;
@@ -1833,8 +1838,10 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
 		if (ret) {
 			dprintk("no CPU initialized for driver %s\n",
 							driver_data->name);
+			get_online_cpus();
 			sysdev_driver_unregister(&cpu_sysdev_class,
 						&cpufreq_sysdev_driver);
+			put_online_cpus();
 
 			spin_lock_irqsave(&cpufreq_driver_lock, flags);
 			cpufreq_driver = NULL;
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index d2af20d..9428f7e 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -446,12 +446,15 @@ static void do_dbs_timer(struct work_struct *work)
 
 	delay -= jiffies % delay;
 
-	if (lock_policy_rwsem_write(cpu) < 0)
+	if (!try_get_online_cpus())
 		return;
 
+	if (lock_policy_rwsem_write(cpu) < 0)
+		goto out;
+
 	if (!dbs_info->enable) {
 		unlock_policy_rwsem_write(cpu);
-		return;
+		goto out;
 	}
 
 	/* Common NORMAL_SAMPLE setup */
@@ -471,6 +474,8 @@ static void do_dbs_timer(struct work_struct *work)
 	}
 	queue_delayed_work_on(cpu, kondemand_wq, &dbs_info->work, delay);
 	unlock_policy_rwsem_write(cpu);
+out:
+	put_online_cpus();
 }
 
 static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info)
-- 
Thanks and Regards
gautham

      parent reply	other threads:[~2008-04-29 13:05 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-29 12:56 [PATCH 0/8] CPU-Hotplug: Fix CPU-Hotplug <--> cpufreq locking dependency Gautham R Shenoy
2008-04-29 12:57 ` [PATCH 1/8] lockdep: fix recursive read lock validation Gautham R Shenoy
2008-04-29 13:16   ` Bart Van Assche
2008-04-29 14:57     ` Peter Zijlstra
2008-04-29 15:03       ` Bart Van Assche
2008-04-29 15:15         ` Peter Zijlstra
2008-04-29 16:03           ` Bart Van Assche
2008-04-29 16:15             ` Peter Zijlstra
2008-04-29 16:29               ` Bart Van Assche
2008-04-29 17:04                 ` Peter Zijlstra
2008-04-29 17:45                   ` Bart Van Assche
2008-04-29 17:58                     ` Peter Zijlstra
2008-04-29 12:58 ` [PATCH 2/8] lockdep: reader-in-writer recursion Gautham R Shenoy
2008-04-29 13:00 ` [PATCH 3/8] lockdep: fix fib_hash softirq inversion Gautham R Shenoy
2008-04-29 14:45   ` Peter Zijlstra
2008-04-29 13:01 ` [PATCH 4/8] net: af_netlink: deadlock Gautham R Shenoy
2008-04-29 13:19   ` Hans Reiser, reiserfs developer linux-os (Dick Johnson)
2008-04-29 13:02 ` [PATCH 5/8] cpu: cpu-hotplug deadlock Gautham R Shenoy
2008-04-29 14:33   ` Oleg Nesterov
2008-04-29 15:09     ` Peter Zijlstra
2008-04-29 16:45       ` Oleg Nesterov
2008-04-29 17:31         ` Peter Zijlstra
2008-04-30  5:37     ` Gautham R Shenoy
2008-04-30 11:43       ` Oleg Nesterov
2008-04-29 13:02 ` [PATCH 6/8] lockdep: annotate cpu_hotplug Gautham R Shenoy
2008-04-29 13:03 ` [PATCH 7/8] cpu_hotplug: Introduce try_get_online_cpus() Gautham R Shenoy
2008-04-29 13:05 ` Gautham R Shenoy [this message]

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=20080429130505.GI23562@in.ibm.com \
    --to=ego@in.ibm.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=oleg@tv-sign.ru \
    --cc=rjw@sisk.pl \
    --cc=vatsa@in.ibm.com \
    --cc=zdenek.kabelac@gmail.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