* [PATCH V2 1/4] PM / OPP: Propagate error properly from dev_pm_opp_set_sharing_cpus()
[not found] <cover.1446713282.git.viresh.kumar@linaro.org>
@ 2015-11-05 8:51 ` Viresh Kumar
2015-11-05 8:51 ` [PATCH V2 2/4] PM / OPP: Protect updates to list_dev with mutex Viresh Kumar
` (2 subsequent siblings)
3 siblings, 0 replies; 4+ messages in thread
From: Viresh Kumar @ 2015-11-05 8:51 UTC (permalink / raw)
To: Rafael Wysocki, Stephen Boyd
Cc: linaro-kernel, linux-pm, Viresh Kumar, 4 . 3 # 4 . 3,
Dan Carpenter, Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
Len Brown, open list, Pavel Machek
We are returning 0 even in case of errors, fix it.
Cc: 4.3 <stable@vger.kernel.org> # 4.3
Fixes: 8d4d4e98acd6 ("PM / OPP: Add helpers for initializing CPU OPPs")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/power/opp/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c
index c27a1cdffec9..2139c9d4c447 100644
--- a/drivers/base/power/opp/cpu.c
+++ b/drivers/base/power/opp/cpu.c
@@ -156,7 +156,7 @@ int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask)
out_rcu_read_unlock:
rcu_read_unlock();
- return 0;
+ return ret;
}
EXPORT_SYMBOL_GPL(dev_pm_opp_set_sharing_cpus);
--
2.6.2.198.g614a2ac
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH V2 2/4] PM / OPP: Protect updates to list_dev with mutex
[not found] <cover.1446713282.git.viresh.kumar@linaro.org>
2015-11-05 8:51 ` [PATCH V2 1/4] PM / OPP: Propagate error properly from dev_pm_opp_set_sharing_cpus() Viresh Kumar
@ 2015-11-05 8:51 ` Viresh Kumar
2015-11-05 8:51 ` [PATCH V2 3/4] PM / OPP: Hold dev_opp_list_lock for writers Viresh Kumar
2015-11-05 8:51 ` [PATCH V2 4/4] PM / OPP: Add opp_rcu_lockdep_assert() to _find_device_opp() Viresh Kumar
3 siblings, 0 replies; 4+ messages in thread
From: Viresh Kumar @ 2015-11-05 8:51 UTC (permalink / raw)
To: Rafael Wysocki, Stephen Boyd
Cc: linaro-kernel, linux-pm, Viresh Kumar, 4 . 3 # 4 . 3,
Michael Turquette, Bartlomiej Zolnierkiewicz, Dan Carpenter,
Dmitry Torokhov, Greg Kroah-Hartman, Len Brown, open list,
Nishanth Menon, Pavel Machek
dev_opp_list_lock is used everywhere to protect device and OPP lists,
but dev_pm_opp_set_sharing_cpus() is missed somehow. And instead we used
rcu-lock, which wouldn't help here as we are adding a new list_dev.
This also fixes a problem where we have called kzalloc(..., GFP_KERNEL)
from within rcu-lock, which isn't allowed as kzalloc can sleep when
called with GFP_KERNEL.
With CONFIG_DEBUG_ATOMIC_SLEEP set, we get following lockdep-splat:
include/linux/rcupdate.h:578 Illegal context switch in RCU read-side critical section!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 0
5 locks held by swapper/0/1:
#0: (&dev->mutex){......}, at: [<c02f68f4>] __driver_attach+0x48/0x98
#1: (&dev->mutex){......}, at: [<c02f6904>] __driver_attach+0x58/0x98
#2: (cpu_hotplug.lock){++++++}, at: [<c00249d0>] get_online_cpus+0x40/0xb0
#3: (subsys mutex#5){+.+.+.}, at: [<c02f4f8c>] subsys_interface_register+0x44/0xdc
#4: (rcu_read_lock){......}, at: [<c0305c80>] dev_pm_opp_set_sharing_cpus+0x0/0x1e4
stack backtrace:
CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W 4.3.0-rc7-00047-g81f5932958a8 #59
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[<c0016874>] (unwind_backtrace) from [<c001355c>] (show_stack+0x10/0x14)
[<c001355c>] (show_stack) from [<c022553c>] (dump_stack+0x94/0xbc)
[<c022553c>] (dump_stack) from [<c004904c>] (___might_sleep+0x24c/0x298)
[<c004904c>] (___might_sleep) from [<c00f07e4>] (kmem_cache_alloc+0xe8/0x164)
[<c00f07e4>] (kmem_cache_alloc) from [<c0305354>] (_add_list_dev+0x30/0x58)
[<c0305354>] (_add_list_dev) from [<c0305d50>] (dev_pm_opp_set_sharing_cpus+0xd0/0x1e4)
[<c0305d50>] (dev_pm_opp_set_sharing_cpus) from [<c040eda4>] (cpufreq_init+0x4cc/0x62c)
[<c040eda4>] (cpufreq_init) from [<c040a964>] (cpufreq_online+0xbc/0x73c)
[<c040a964>] (cpufreq_online) from [<c02f4fe0>] (subsys_interface_register+0x98/0xdc)
[<c02f4fe0>] (subsys_interface_register) from [<c040a640>] (cpufreq_register_driver+0x110/0x17c)
[<c040a640>] (cpufreq_register_driver) from [<c040ef64>] (dt_cpufreq_probe+0x60/0x8c)
[<c040ef64>] (dt_cpufreq_probe) from [<c02f8084>] (platform_drv_probe+0x44/0xa4)
[<c02f8084>] (platform_drv_probe) from [<c02f67c0>] (driver_probe_device+0x208/0x2f4)
[<c02f67c0>] (driver_probe_device) from [<c02f6940>] (__driver_attach+0x94/0x98)
[<c02f6940>] (__driver_attach) from [<c02f4c1c>] (bus_for_each_dev+0x68/0x9c)
Cc: 4.3 <stable@vger.kernel.org> # 4.3
Reported-by: Michael Turquette <mturquette@baylibre.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/power/opp/core.c | 2 +-
drivers/base/power/opp/cpu.c | 8 ++++----
drivers/base/power/opp/opp.h | 3 +++
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 8fa522f41226..a93a433c9ac5 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -29,7 +29,7 @@
*/
static LIST_HEAD(dev_opp_list);
/* Lock to allow exclusive modification to the device and opp lists */
-static DEFINE_MUTEX(dev_opp_list_lock);
+DEFINE_MUTEX(dev_opp_list_lock);
#define opp_rcu_lockdep_assert() \
do { \
diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c
index 2139c9d4c447..7b445e88a0d5 100644
--- a/drivers/base/power/opp/cpu.c
+++ b/drivers/base/power/opp/cpu.c
@@ -127,12 +127,12 @@ int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask)
struct device *dev;
int cpu, ret = 0;
- rcu_read_lock();
+ mutex_lock(&dev_opp_list_lock);
dev_opp = _find_device_opp(cpu_dev);
if (IS_ERR(dev_opp)) {
ret = -EINVAL;
- goto out_rcu_read_unlock;
+ goto unlock;
}
for_each_cpu(cpu, cpumask) {
@@ -153,8 +153,8 @@ int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask)
continue;
}
}
-out_rcu_read_unlock:
- rcu_read_unlock();
+unlock:
+ mutex_unlock(&dev_opp_list_lock);
return ret;
}
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index dcb38f78dae4..7366b2aa8997 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -21,6 +21,9 @@
#include <linux/rculist.h>
#include <linux/rcupdate.h>
+/* Lock to allow exclusive modification to the device and opp lists */
+extern struct mutex dev_opp_list_lock;
+
/*
* Internal data structure organization with the OPP layer library is as
* follows:
--
2.6.2.198.g614a2ac
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH V2 3/4] PM / OPP: Hold dev_opp_list_lock for writers
[not found] <cover.1446713282.git.viresh.kumar@linaro.org>
2015-11-05 8:51 ` [PATCH V2 1/4] PM / OPP: Propagate error properly from dev_pm_opp_set_sharing_cpus() Viresh Kumar
2015-11-05 8:51 ` [PATCH V2 2/4] PM / OPP: Protect updates to list_dev with mutex Viresh Kumar
@ 2015-11-05 8:51 ` Viresh Kumar
2015-11-05 8:51 ` [PATCH V2 4/4] PM / OPP: Add opp_rcu_lockdep_assert() to _find_device_opp() Viresh Kumar
3 siblings, 0 replies; 4+ messages in thread
From: Viresh Kumar @ 2015-11-05 8:51 UTC (permalink / raw)
To: Rafael Wysocki, Stephen Boyd
Cc: linaro-kernel, linux-pm, Viresh Kumar, 4 . 3 # 4 . 3,
Bartlomiej Zolnierkiewicz, Dmitry Torokhov, Greg Kroah-Hartman,
Len Brown, open list, Nishanth Menon, Pavel Machek
Writers need to update OPP device and their list with dev_opp_list_lock
mutex held, which was missed at few places. Fix it.
Cc: 4.3 <stable@vger.kernel.org> # 4.3
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/power/opp/core.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index a93a433c9ac5..7a395f5ba5f9 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -1175,13 +1175,17 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
struct device_opp *dev_opp;
int ret = 0, count = 0;
+ mutex_lock(&dev_opp_list_lock);
+
dev_opp = _managed_opp(opp_np);
if (dev_opp) {
/* OPPs are already managed */
if (!_add_list_dev(dev, dev_opp))
ret = -ENOMEM;
+ mutex_unlock(&dev_opp_list_lock);
return ret;
}
+ mutex_unlock(&dev_opp_list_lock);
/* We have opp-list node now, iterate over it and add OPPs */
for_each_available_child_of_node(opp_np, np) {
@@ -1199,15 +1203,20 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
if (WARN_ON(!count))
return -ENOENT;
+ mutex_lock(&dev_opp_list_lock);
+
dev_opp = _find_device_opp(dev);
if (WARN_ON(IS_ERR(dev_opp))) {
ret = PTR_ERR(dev_opp);
+ mutex_unlock(&dev_opp_list_lock);
goto free_table;
}
dev_opp->np = opp_np;
dev_opp->shared_opp = of_property_read_bool(opp_np, "opp-shared");
+ mutex_unlock(&dev_opp_list_lock);
+
return 0;
free_table:
--
2.6.2.198.g614a2ac
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH V2 4/4] PM / OPP: Add opp_rcu_lockdep_assert() to _find_device_opp()
[not found] <cover.1446713282.git.viresh.kumar@linaro.org>
` (2 preceding siblings ...)
2015-11-05 8:51 ` [PATCH V2 3/4] PM / OPP: Hold dev_opp_list_lock for writers Viresh Kumar
@ 2015-11-05 8:51 ` Viresh Kumar
3 siblings, 0 replies; 4+ messages in thread
From: Viresh Kumar @ 2015-11-05 8:51 UTC (permalink / raw)
To: Rafael Wysocki, Stephen Boyd
Cc: linaro-kernel, linux-pm, Viresh Kumar, 4 . 3 # 4 . 3,
Bartlomiej Zolnierkiewicz, Dmitry Torokhov, Greg Kroah-Hartman,
Len Brown, open list, Nishanth Menon, Pavel Machek
_find_device_opp() should be called with rcu-read lock or
dev_opp_list_lock held. Add the opp_rcu_lockdep_assert() check to make
sure caller have taken appropriate locks.
Fix comment over the routine as well.
Cc: 4.3 <stable@vger.kernel.org> # 4.3
Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/power/opp/core.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 7a395f5ba5f9..5e4b1d82b2f8 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -81,14 +81,18 @@ static struct device_opp *_managed_opp(const struct device_node *np)
* Return: pointer to 'struct device_opp' if found, otherwise -ENODEV or
* -EINVAL based on type of error.
*
- * Locking: This function must be called under rcu_read_lock(). device_opp
- * is a RCU protected pointer. This means that device_opp is valid as long
- * as we are under RCU lock.
+ * Locking: For readers, this function must be called under rcu_read_lock().
+ * device_opp is a RCU protected pointer, which means that device_opp is valid
+ * as long as we are under RCU lock.
+ *
+ * For Writers, this function must be called with dev_opp_list_lock held.
*/
struct device_opp *_find_device_opp(struct device *dev)
{
struct device_opp *dev_opp;
+ opp_rcu_lockdep_assert();
+
if (IS_ERR_OR_NULL(dev)) {
pr_err("%s: Invalid parameters\n", __func__);
return ERR_PTR(-EINVAL);
--
2.6.2.198.g614a2ac
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-05 8:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <cover.1446713282.git.viresh.kumar@linaro.org>
2015-11-05 8:51 ` [PATCH V2 1/4] PM / OPP: Propagate error properly from dev_pm_opp_set_sharing_cpus() Viresh Kumar
2015-11-05 8:51 ` [PATCH V2 2/4] PM / OPP: Protect updates to list_dev with mutex Viresh Kumar
2015-11-05 8:51 ` [PATCH V2 3/4] PM / OPP: Hold dev_opp_list_lock for writers Viresh Kumar
2015-11-05 8:51 ` [PATCH V2 4/4] PM / OPP: Add opp_rcu_lockdep_assert() to _find_device_opp() Viresh Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).