* [PATCH V2 0/4] PM / OPP: Fixes
@ 2015-11-05 8:51 Viresh Kumar
2015-11-05 8:51 ` Viresh Kumar
` (4 more replies)
0 siblings, 5 replies; 10+ 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
Hi Rafael,
V1 only contained a single patch, i.e. 2/4 from this series. And then
there were few suggestions to incorporate.
All are marked for stable 4.3, as this got changed recently.
I have tested it on exynos (Yeah, I was able to reproduce the lockdep
with few config changes).
Viresh Kumar (4):
PM / OPP: Propagate error properly from dev_pm_opp_set_sharing_cpus()
PM / OPP: Protect updates to list_dev with mutex
PM / OPP: Hold dev_opp_list_lock for writers
PM / OPP: Add opp_rcu_lockdep_assert() to _find_device_opp()
drivers/base/power/opp/core.c | 21 +++++++++++++++++----
drivers/base/power/opp/cpu.c | 10 +++++-----
drivers/base/power/opp/opp.h | 3 +++
3 files changed, 25 insertions(+), 9 deletions(-)
--
2.6.2.198.g614a2ac
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH V2 1/4] PM / OPP: Propagate error properly from dev_pm_opp_set_sharing_cpus()
2015-11-05 8:51 [PATCH V2 0/4] PM / OPP: Fixes Viresh Kumar
@ 2015-11-05 8:51 ` Viresh Kumar
2015-11-05 8:51 ` Viresh Kumar
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ 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] 10+ messages in thread
* [PATCH V2 1/4] PM / OPP: Propagate error properly from dev_pm_opp_set_sharing_cpus()
@ 2015-11-05 8:51 ` Viresh Kumar
0 siblings, 0 replies; 10+ 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] 10+ messages in thread
* [PATCH V2 2/4] PM / OPP: Protect updates to list_dev with mutex
2015-11-05 8:51 [PATCH V2 0/4] PM / OPP: Fixes Viresh Kumar
@ 2015-11-05 8:51 ` Viresh Kumar
2015-11-05 8:51 ` Viresh Kumar
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ 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] 10+ messages in thread
* [PATCH V2 2/4] PM / OPP: Protect updates to list_dev with mutex
@ 2015-11-05 8:51 ` Viresh Kumar
0 siblings, 0 replies; 10+ 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] 10+ messages in thread
* [PATCH V2 3/4] PM / OPP: Hold dev_opp_list_lock for writers
2015-11-05 8:51 [PATCH V2 0/4] PM / OPP: Fixes Viresh Kumar
@ 2015-11-05 8:51 ` Viresh Kumar
2015-11-05 8:51 ` Viresh Kumar
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ 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] 10+ messages in thread
* [PATCH V2 3/4] PM / OPP: Hold dev_opp_list_lock for writers
@ 2015-11-05 8:51 ` Viresh Kumar
0 siblings, 0 replies; 10+ 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] 10+ messages in thread
* [PATCH V2 4/4] PM / OPP: Add opp_rcu_lockdep_assert() to _find_device_opp()
2015-11-05 8:51 [PATCH V2 0/4] PM / OPP: Fixes Viresh Kumar
@ 2015-11-05 8:51 ` Viresh Kumar
2015-11-05 8:51 ` Viresh Kumar
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ 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] 10+ messages in thread
* [PATCH V2 4/4] PM / OPP: Add opp_rcu_lockdep_assert() to _find_device_opp()
@ 2015-11-05 8:51 ` Viresh Kumar
0 siblings, 0 replies; 10+ 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] 10+ messages in thread
* Re: [PATCH V2 0/4] PM / OPP: Fixes
2015-11-05 8:51 [PATCH V2 0/4] PM / OPP: Fixes Viresh Kumar
` (3 preceding siblings ...)
2015-11-05 8:51 ` Viresh Kumar
@ 2015-11-07 1:06 ` Rafael J. Wysocki
4 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2015-11-07 1:06 UTC (permalink / raw)
To: Viresh Kumar; +Cc: Stephen Boyd, linaro-kernel, linux-pm
On Thursday, November 05, 2015 02:21:17 PM Viresh Kumar wrote:
> Hi Rafael,
>
> V1 only contained a single patch, i.e. 2/4 from this series. And then
> there were few suggestions to incorporate.
>
> All are marked for stable 4.3, as this got changed recently.
>
> I have tested it on exynos (Yeah, I was able to reproduce the lockdep
> with few config changes).
>
> Viresh Kumar (4):
> PM / OPP: Propagate error properly from dev_pm_opp_set_sharing_cpus()
> PM / OPP: Protect updates to list_dev with mutex
> PM / OPP: Hold dev_opp_list_lock for writers
> PM / OPP: Add opp_rcu_lockdep_assert() to _find_device_opp()
>
> drivers/base/power/opp/core.c | 21 +++++++++++++++++----
> drivers/base/power/opp/cpu.c | 10 +++++-----
> drivers/base/power/opp/opp.h | 3 +++
> 3 files changed, 25 insertions(+), 9 deletions(-)
Applied, but I removed the stable tag from the last patch as it really
is not a bug fix.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-11-07 0:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-05 8:51 [PATCH V2 0/4] PM / OPP: Fixes Viresh Kumar
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 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 3/4] PM / OPP: Hold dev_opp_list_lock for writers 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
2015-11-05 8:51 ` Viresh Kumar
2015-11-07 1:06 ` [PATCH V2 0/4] PM / OPP: Fixes Rafael J. Wysocki
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.