omap: resource: Add per-resource mutex for OMAP resource framework

From: Chunqiu Wang <[email protected]>

Current OMAP resource fwk uses a global res_mutex for resource_request and resource_release calls for all the available resources.It may cause dead lock if resource_request/resource_release is called recursively.

For current OMAP3 VDD1/VDD2 resource, the change_level implementation is mach-omap2/resource34xx.c/set_opp(), when using resource_release to remove vdd1 constraint, this function may call resource_release again to release Vdd2 constrait if target vdd1 level is less than OPP3. in this scenario, the global res_mutex down operation will be called again, this will cause the second down operation hang there.

To fix the problem, per-resource mutex is added to avoid hangup when resource_request/resource_release is called recursively.

Signed-off-by Chunqiu Wang <[email protected]>

Signed-off-by Mike Chan <[email protected]>

---
?arch/arm/plat-omap/include/plat/resource.h | ? ?2 ++ ?arch/arm/plat-omap/resource.c ? ? ? ? ? ? ?| ? 21 ++++++++++----------- ?2 files changed, 12 insertions(+), 11 deletions(-)

arch/arm/plat-omap/include/plat/resource.h +0 -0

index 9acebcc..b5aff1f 100644
@@ -54,6 +54,8 @@ struct shared_resource {

? ? ? ?/* Shared resource operations */
? ? ? ?struct shared_resource_ops *ops;
? ? ? ?struct list_head node;
+ ? ? ? /* Protect each resource */
+ ? ? ? struct mutex resource_mutex;
?};

?struct shared_resource_ops {

arch/arm/plat-omap/resource.c +1 -0

index f1cdecf..0a7b79b 100644
@@ -277,6 +277,7 @@ int resource_register(struct shared_resource *resp)
 ? ? ? ?}
 
 ? ? ? ?INIT_LIST_HEAD(&resp->users_list);
+ ? ? ? mutex_init(&resp->resource_mutex);
 

? ? ? ?/* Add the resource to the resource list */ ? ? ? ?list_add(&resp->node, &res_list);

arch/arm/plat-omap/resource.c +3 -4

@@ -339,14 +340,13 @@ int resource_request(const char *name, struct device *dev,
 ? ? ? ?struct ?users_list *user;
 ? ? ? ?int ? ? found = 0, ret = 0;
 
- ? ? ? down(&res_mutex);
- ? ? ? resp = _resource_lookup(name);
+ ? ? ? resp = resource_lookup(name);
 ? ? ? ?if (!resp) {
 ? ? ? ? ? ? ? ?printk(KERN_ERR "resource_request: Invalid resource name\n");
- ? ? ? ? ? ? ? ret = -EINVAL;
- ? ? ? ? ? ? ? goto res_unlock;
+ ? ? ? ? ? ? ? return -EINVAL;
 ? ? ? ?}
 
+ ? ? ? mutex_lock(&resp->resource_mutex);

? ? ? ?/* Call the resource specific validate function */ ? ? ? ?if (resp->ops->validate_level) {
? ? ? ? ? ? ? ?ret = resp->ops->validate_level(resp, level);

arch/arm/plat-omap/resource.c +0 -0

@@ -375,7 +375,7 @@ int resource_request(const char *name, struct device *dev,
 ? ? ? ?user->level = level;
 

?res_unlock:
- ? ? ? up(&res_mutex);
+ ? ? ? mutex_unlock(&resp->resource_mutex);
? ? ? ?/*
? ? ? ? * Recompute and set the current level for the resource. ? ? ? ? * NOTE: update_resource level moved out of spin_lock, as it may call

arch/arm/plat-omap/resource.c +3 -4

@@ -406,14 +406,13 @@ int resource_release(const char *name, struct device *dev)
 ? ? ? ?struct users_list *user;
 ? ? ? ?int found = 0, ret = 0;
 
- ? ? ? down(&res_mutex);
- ? ? ? resp = _resource_lookup(name);
+ ? ? ? resp = resource_lookup(name);
 ? ? ? ?if (!resp) {
 ? ? ? ? ? ? ? ?printk(KERN_ERR "resource_release: Invalid resource name\n");
- ? ? ? ? ? ? ? ret = -EINVAL;
- ? ? ? ? ? ? ? goto res_unlock;
+ ? ? ? ? ? ? ? return -EINVAL;
 ? ? ? ?}
 
+ ? ? ? mutex_lock(&resp->resource_mutex);
 ? ? ? ?list_for_each_entry(user, &resp->users_list, node) {
 ? ? ? ? ? ? ? ?if (user->dev == dev) {
 ? ? ? ? ? ? ? ? ? ? ? ?found = 1;

@@ -434,7 +433,7 @@ int resource_release(const char *name, struct device *dev)

? ? ? ?/* Recompute and set the current level for the resource */ ? ? ? ?ret = update_resource_level(resp);
?res_unlock:
- ? ? ? up(&res_mutex);
+ ? ? ? mutex_unlock(&resp->resource_mutex);
? ? ? ?return ret;
?}
?EXPORT_SYMBOL(resource_release);

arch/arm/plat-omap/resource.c +1 -2

@@ -458,7 +457,7 @@ int resource_get_level(const char *name)
 ? ? ? ? ? ? ? ?up(&res_mutex);
 ? ? ? ? ? ? ? ?return -EINVAL;
 ? ? ? ?}
- ? ? ? ret = resp->curr_level;
+ ? ? ? ret = ?resp->curr_level;
 ? ? ? ?up(&res_mutex);
 ? ? ? ?return ret;
 ?}
--

1.6.6.2

Quoted 35 lines · On Tue, Mar 9, 2010 at 4:09 PM, <[email protected]> wrote:

From: Chunqiu Wang <[email protected]>

Current OMAP resource fwk uses a global res_mutex for resource_request and resource_release calls for all the available resources.It may cause dead lock if resource_request/resource_release is called recursively.

For current OMAP3 VDD1/VDD2 resource, the change_level implementation is mach-omap2/resource34xx.c/set_opp(), when using resource_release to remove vdd1 constraint, this function may call resource_release again to release Vdd2 constrait if target vdd1 level is less than OPP3. in this scenario, the global res_mutex down operation will be called again, this will cause the second down operation hang there.

To fix the problem, per-resource mutex is added to avoid hangup when resource_request/resource_release is called recursively.

Signed-off-by Chunqiu Wang <[email protected]>

Signed-off-by Mike Chan <[email protected]>

---
?arch/arm/plat-omap/include/plat/resource.h | ? ?2 ++ ?arch/arm/plat-omap/resource.c ? ? ? ? ? ? ?| ? 21 ++++++++++----------- ?2 files changed, 12 insertions(+), 11 deletions(-)

arch/arm/plat-omap/include/plat/resource.h +0 -0

index 9acebcc..b5aff1f 100644
@@ -54,6 +54,8 @@ struct shared_resource {

? ? ? ?/* Shared resource operations */
? ? ? ?struct shared_resource_ops *ops;

Oops, my git client was slightly screwed up, apologies, I meant to send this from mike at android.com, the patch is still good though :)