* [PATCH] omap: resource: Add per-resource mutex for OMAP resource framework
@ 2010-03-10 0:09 y at google.com
0 siblings, 0 replies; 3+ messages in thread
From: y at google.com @ 2010-03-10 0:09 UTC (permalink / raw)
To: linux-arm-kernel
From: Chunqiu Wang <cqwang@motorola.com>
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 <cqwang@motorola.com>
Signed-off-by: Mike Chan <mike@android.com>
---
arch/arm/plat-omap/include/plat/resource.h | 2 ++
arch/arm/plat-omap/resource.c | 21 ++++++++++-----------
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/resource.h b/arch/arm/plat-omap/include/plat/resource.h
index 9acebcc..b5aff1f 100644
--- a/arch/arm/plat-omap/include/plat/resource.h
+++ b/arch/arm/plat-omap/include/plat/resource.h
@@ -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 {
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index f1cdecf..0a7b79b 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -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);
@@ -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);
@@ -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
@@ -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);
@@ -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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] omap: resource: Add per-resource mutex for OMAP resource framework
[not found] <-1645286213079228429@unknownmsgid>
@ 2010-03-10 0:11 ` Mike Chan
2010-03-10 17:27 ` Kevin Hilman
0 siblings, 1 reply; 3+ messages in thread
From: Mike Chan @ 2010-03-10 0:11 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 9, 2010 at 4:09 PM, <y@google.com> wrote:
> From: Chunqiu Wang <cqwang@motorola.com>
>
> 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 <cqwang@motorola.com>
> Signed-off-by: Mike Chan <mike@android.com>
> ---
> ?arch/arm/plat-omap/include/plat/resource.h | ? ?2 ++
> ?arch/arm/plat-omap/resource.c ? ? ? ? ? ? ?| ? 21 ++++++++++-----------
> ?2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/resource.h b/arch/arm/plat-omap/include/plat/resource.h
> index 9acebcc..b5aff1f 100644
> --- a/arch/arm/plat-omap/include/plat/resource.h
> +++ b/arch/arm/plat-omap/include/plat/resource.h
> @@ -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 {
> diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
> index f1cdecf..0a7b79b 100644
> --- a/arch/arm/plat-omap/resource.c
> +++ b/arch/arm/plat-omap/resource.c
> @@ -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);
> @@ -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);
> @@ -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
> @@ -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);
> @@ -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
>
>
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 :)
-- Mike
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] omap: resource: Add per-resource mutex for OMAP resource framework
2010-03-10 0:11 ` [PATCH] omap: resource: Add per-resource mutex for OMAP resource framework Mike Chan
@ 2010-03-10 17:27 ` Kevin Hilman
0 siblings, 0 replies; 3+ messages in thread
From: Kevin Hilman @ 2010-03-10 17:27 UTC (permalink / raw)
To: linux-arm-kernel
Mike Chan <mike@android.com> writes:
> On Tue, Mar 9, 2010 at 4:09 PM, <y@google.com> wrote:
>> From: Chunqiu Wang <cqwang@motorola.com>
>>
>> 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 <cqwang@motorola.com>
>> Signed-off-by: Mike Chan <mike@android.com>
>> ---
>> ?arch/arm/plat-omap/include/plat/resource.h | ? ?2 ++
>> ?arch/arm/plat-omap/resource.c ? ? ? ? ? ? ?| ? 21 ++++++++++-----------
>> ?2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/include/plat/resource.h b/arch/arm/plat-omap/include/plat/resource.h
>> index 9acebcc..b5aff1f 100644
>> --- a/arch/arm/plat-omap/include/plat/resource.h
>> +++ b/arch/arm/plat-omap/include/plat/resource.h
>> @@ -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 :)
>
FYI... SRF is currently only in the PM branch of the OMAP tree, is
deprecated and is not targeted for mainline.
I'll merge this there, but please note that there is no new
development happening for SRF.
Thanks,
Kevin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-03-10 17:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <-1645286213079228429@unknownmsgid>
2010-03-10 0:11 ` [PATCH] omap: resource: Add per-resource mutex for OMAP resource framework Mike Chan
2010-03-10 17:27 ` Kevin Hilman
2010-03-10 0:09 y at google.com
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).