From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
To: rjw@sisk.pl, sboyd@codeaurora.org, viresh.kumar@linaro.org
Cc: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>,
cpufreq@vger.kernel.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [PATCH 2/5] cpufreq: Invoke __cpufreq_remove_dev_finish() after releasing cpu_hotplug.lock
Date: Sat, 07 Sep 2013 01:23:27 +0530 [thread overview]
Message-ID: <20130906195318.26919.88933.stgit@srivatsabhat.in.ibm.com> (raw)
In-Reply-To: <20130906195206.26919.20884.stgit@srivatsabhat.in.ibm.com>
__cpufreq_remove_dev_finish() handles the kobject cleanup for a CPU going
offline. But because we destroy the kobject towards the end of the CPU offline
phase, there are certain race windows where a task can try to write to a
cpufreq sysfs file (eg: using store_scaling_max_freq()) while we are taking
that CPU offline, and this can bump up the kobject refcount, which in turn might
hinder the CPU offline task from running to completion. (It can also cause
other more serious problems such as trying to acquire a destroyed timer-mutex
etc., depending on the exact stage of the cleanup at which the task managed to
take a new refcount).
To fix the race window, we will need to synchronize those store_*() call-sites
with CPU hotplug, using get_online_cpus()/put_online_cpus(). However, that
in turn can cause a total deadlock because it can end up waiting for the
CPU offline task to complete, with incremented refcount!
Write to sysfs CPU offline task
-------------- ----------------
kobj_refcnt++
Acquire cpu_hotplug.lock
get_online_cpus();
Wait for kobj_refcnt to drop to zero
**DEADLOCK**
A simple way to avoid this problem is to perform the kobject cleanup in the
CPU offline path, with the cpu_hotplug.lock *released*. That is, we can
perform the wait-for-kobj-refcnt-to-drop as well as the subsequent cleanup
in the CPU_POST_DEAD stage of CPU offline, which is run with cpu_hotplug.lock
released. Doing this helps us avoid deadlocks due to holding kobject refcounts
and waiting on each other on the cpu_hotplug.lock.
(Note: We can't move all of the cpufreq CPU offline steps to the
CPU_POST_DEAD stage, because certain things such as stopping the governors
have to be done before the outgoing CPU is marked offline. So retain those
parts in the CPU_DOWN_PREPARE stage itself).
Reported-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
drivers/cpufreq/cpufreq.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 3e5aeb6..a6fe3fd 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2085,6 +2085,9 @@ static int cpufreq_cpu_callback(struct notifier_block *nfb,
case CPU_DOWN_PREPARE:
__cpufreq_remove_dev_prepare(dev, NULL, frozen);
+ break;
+
+ case CPU_POST_DEAD:
__cpufreq_remove_dev_finish(dev, NULL, frozen);
break;
next prev parent reply other threads:[~2013-09-06 19:57 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-06 19:52 [PATCH 0/5] Cpufreq fixes related to cpu hotplug/sysfs-writes Srivatsa S. Bhat
2013-09-06 19:53 ` [PATCH 1/5] cpufreq: Split __cpufreq_remove_dev() into 2 parts (kobj cleanup & the rest) Srivatsa S. Bhat
2013-09-06 19:53 ` Srivatsa S. Bhat [this message]
2013-09-06 19:53 ` [PATCH 3/5] cpufreq: Synchronize the cpufreq store_*() routines with CPU hotplug Srivatsa S. Bhat
2013-09-06 19:53 ` [PATCH 4/5] cpufreq: Remove temporary fix for race between CPU hotplug and sysfs-writes Srivatsa S. Bhat
2013-09-10 7:09 ` Viresh Kumar
2013-09-10 8:57 ` Srivatsa S. Bhat
2013-09-06 19:54 ` [PATCH 5/5] cpufreq: Use signed type for 'ret' variable, to store negative error values Srivatsa S. Bhat
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=20130906195318.26919.88933.stgit@srivatsabhat.in.ibm.com \
--to=srivatsa.bhat@linux.vnet.ibm.com \
--cc=cpufreq@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@sisk.pl \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®