Skip to contentSkip to conversation

PM / OPP: add some lockdep annotations

PATCH In review Dmitry Torokhov 2 participants 2 messages Dec 16, 2014

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 PM / OPP: add some lockdep annotations

Certain OPP APIs need to be called under RCU lock; let's add a few rcu_lockdep_assert() calls to warn about potential misuse.

---
 drivers/base/power/opp.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
drivers/base/power/opp.c +20 -0
index d24dd614a..852eebf 100644
@@ -218,6 +218,11 @@ int dev_pm_opp_get_opp_count(struct device *dev)
 	struct dev_pm_opp *temp_opp;
 	int count = 0;
 
+	rcu_lockdep_assert(rcu_read_lock_held() ||
+				lockdep_is_held(&dev_opp_list_lock),
+			   "dev_pm_opp_get_opp_count() needs rcu_read_lock() "
+			   "or dev_opp_list_lock protection");
+
 	dev_opp = find_device_opp(dev);
 	if (IS_ERR(dev_opp)) {
 		int r = PTR_ERR(dev_opp);
@@ -267,6 +272,11 @@ struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
 
+	rcu_lockdep_assert(rcu_read_lock_held() ||
+				lockdep_is_held(&dev_opp_list_lock),
+			   "dev_pm_opp_find_freq_exact() needs rcu_read_lock() "
+			   "or dev_opp_list_lock protection");
+
 	dev_opp = find_device_opp(dev);
 	if (IS_ERR(dev_opp)) {
 		int r = PTR_ERR(dev_opp);
@@ -313,6 +323,11 @@ struct dev_pm_opp *dev_pm_opp_find_freq_ceil(struct device *dev,
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
 
+	rcu_lockdep_assert(rcu_read_lock_held() ||
+				lockdep_is_held(&dev_opp_list_lock),
+			   "dev_pm_opp_find_freq_ceil() needs rcu_read_lock() "
+			   "or dev_opp_list_lock protection");
+
 	if (!dev || !freq) {
 		dev_err(dev, "%s: Invalid argument freq=%p\n", __func__, freq);
 		return ERR_PTR(-EINVAL);
@@ -361,6 +376,11 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev,
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
 
+	rcu_lockdep_assert(rcu_read_lock_held() ||
+				lockdep_is_held(&dev_opp_list_lock),
+			   "dev_pm_opp_find_freq_floor() needs rcu_read_lock() "
+			   "or dev_opp_list_lock protection");
+
 	if (!dev || !freq) {
 		dev_err(dev, "%s: Invalid argument freq=%p\n", __func__, freq);
 		return ERR_PTR(-EINVAL);
Signature · 5 lines
2.2.0.rc0.207.ga3a616c


-- 
Dmitry
#1 lore raw
Quoted 2 lines · Dmitry Torokhov

Certain OPP APIs need to be called under RCU lock; let's add a few rcu_lockdep_assert() calls to warn about potential misuse.

Very nice..

Quoted 46 lines
---
 drivers/base/power/opp.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
drivers/base/power/opp.c +15 -0
index d24dd614a..852eebf 100644
@@ -218,6 +218,11 @@ int dev_pm_opp_get_opp_count(struct device *dev)
        struct dev_pm_opp *temp_opp;
        int count = 0;
 
+       rcu_lockdep_assert(rcu_read_lock_held() ||
+                               lockdep_is_held(&dev_opp_list_lock),
+                          "dev_pm_opp_get_opp_count() needs rcu_read_lock() "
+                          "or dev_opp_list_lock protection");
+
        dev_opp = find_device_opp(dev);
        if (IS_ERR(dev_opp)) {
                int r = PTR_ERR(dev_opp);
@@ -267,6 +272,11 @@ struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
        struct device_opp *dev_opp;
        struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
 
+       rcu_lockdep_assert(rcu_read_lock_held() ||
+                               lockdep_is_held(&dev_opp_list_lock),
+                          "dev_pm_opp_find_freq_exact() needs rcu_read_lock() "
+                          "or dev_opp_list_lock protection");
+
        dev_opp = find_device_opp(dev);
        if (IS_ERR(dev_opp)) {
                int r = PTR_ERR(dev_opp);
@@ -313,6 +323,11 @@ struct dev_pm_opp *dev_pm_opp_find_freq_ceil(struct device *dev,
        struct device_opp *dev_opp;
        struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
 
+       rcu_lockdep_assert(rcu_read_lock_held() ||
+                               lockdep_is_held(&dev_opp_list_lock),
+                          "dev_pm_opp_find_freq_ceil() needs rcu_read_lock() "
+                          "or dev_opp_list_lock protection");
+
        if (!dev || !freq) {
                dev_err(dev, "%s: Invalid argument freq=%p\n", __func__, freq);
                return ERR_PTR(-EINVAL);

drivers/base/power/opp.c struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, · line 383

Quoted 8 lines
drivers/base/power/opp.c +5 -0
@@ -361,6 +376,11 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev,
        struct device_opp *dev_opp;
        struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
 
+       rcu_lockdep_assert(rcu_read_lock_held() ||
+                               lockdep_is_held(&dev_opp_list_lock),
+                          "dev_pm_opp_find_freq_floor() needs rcu_read_lock() "
+                          "or dev_opp_list_lock protection");
+

To get rid of the redundancy a bit, what about something like:

#define opp_rcu_lockdep_assert()        \
        rcu_lockdep_assert(rcu_read_lock_held() || \
                                        lockdep_is_held(&dev_opp_list_lock), \
                                        "Missing rcu_read_lock() or

dev_opp_list_lock protection");