* [PATCH] [ARM] omap: resource: Make resource_refresh() thread safe.
@ 2009-09-03 23:18 Mike Chan
2009-09-08 22:45 ` Kevin Hilman
0 siblings, 1 reply; 5+ messages in thread
From: Mike Chan @ 2009-09-03 23:18 UTC (permalink / raw)
To: linux-omap; +Cc: khilman, Mike Chan
Need to lock the res_mutex when traversing the res_list.
Signed-off-by: Mike Chan <mike@android.com>
---
arch/arm/plat-omap/resource.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
index 25072cd..4631912 100644
--- a/arch/arm/plat-omap/resource.c
+++ b/arch/arm/plat-omap/resource.c
@@ -234,11 +234,13 @@ int resource_refresh(void)
struct shared_resource *resp = NULL;
int ret = 0;
+ down(&res_mutex);
list_for_each_entry(resp, &res_list, node) {
ret = update_resource_level(resp);
if (ret)
break;
}
+ up(&res_mutex);
return ret;
}
--
1.5.4.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] [ARM] omap: resource: Make resource_refresh() thread safe.
2009-09-03 23:18 [PATCH] [ARM] omap: resource: Make resource_refresh() thread safe Mike Chan
@ 2009-09-08 22:45 ` Kevin Hilman
2009-09-11 4:12 ` Mike Turquette
0 siblings, 1 reply; 5+ messages in thread
From: Kevin Hilman @ 2009-09-08 22:45 UTC (permalink / raw)
To: Mike Chan; +Cc: linux-omap
Mike Chan <mike@android.com> writes:
> Need to lock the res_mutex when traversing the res_list.
>
> Signed-off-by: Mike Chan <mike@android.com>
Looks good, thanks.
Pushed to PM branch and pm-2.6.29.
Kevin
> ---
> arch/arm/plat-omap/resource.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
> index 25072cd..4631912 100644
> --- a/arch/arm/plat-omap/resource.c
> +++ b/arch/arm/plat-omap/resource.c
> @@ -234,11 +234,13 @@ int resource_refresh(void)
> struct shared_resource *resp = NULL;
> int ret = 0;
>
> + down(&res_mutex);
> list_for_each_entry(resp, &res_list, node) {
> ret = update_resource_level(resp);
> if (ret)
> break;
> }
> + up(&res_mutex);
> return ret;
> }
>
> --
> 1.5.4.5
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [ARM] omap: resource: Make resource_refresh() thread safe.
2009-09-08 22:45 ` Kevin Hilman
@ 2009-09-11 4:12 ` Mike Turquette
2009-09-11 21:10 ` Kevin Hilman
0 siblings, 1 reply; 5+ messages in thread
From: Mike Turquette @ 2009-09-11 4:12 UTC (permalink / raw)
To: Kevin Hilman; +Cc: Mike Chan, linux-omap
Kevin Hilman wrote:
> Mike Chan <mike@android.com> writes:
>
>> Need to lock the res_mutex when traversing the res_list.
>>
>> Signed-off-by: Mike Chan <mike@android.com>
>
> Looks good, thanks.
This patch causes a hang for me when transitioning to OFF mode. This
was tested on the Android 2.6.29 tree and is 100% reproducible. The
moment a user runs 'echo 1 > /sys/power/enable_off_mode' the board hangs
without any further output.
Reverting the patch allows me to hit OFF mode again. I haven't yet
tested this on vanilla 2.6.29 or latest L-O.
Mike
> Pushed to PM branch and pm-2.6.29.
>
> Kevin
>
>> ---
>> arch/arm/plat-omap/resource.c | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
>> index 25072cd..4631912 100644
>> --- a/arch/arm/plat-omap/resource.c
>> +++ b/arch/arm/plat-omap/resource.c
>> @@ -234,11 +234,13 @@ int resource_refresh(void)
>> struct shared_resource *resp = NULL;
>> int ret = 0;
>>
>> + down(&res_mutex);
>> list_for_each_entry(resp, &res_list, node) {
>> ret = update_resource_level(resp);
>> if (ret)
>> break;
>> }
>> + up(&res_mutex);
>> return ret;
>> }
>>
>> --
>> 1.5.4.5
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [ARM] omap: resource: Make resource_refresh() thread safe.
2009-09-11 4:12 ` Mike Turquette
@ 2009-09-11 21:10 ` Kevin Hilman
2009-09-11 22:50 ` Mike Chan
0 siblings, 1 reply; 5+ messages in thread
From: Kevin Hilman @ 2009-09-11 21:10 UTC (permalink / raw)
To: Mike Turquette; +Cc: Mike Chan, linux-omap
Mike Turquette <mturquette@ti.com> writes:
> Kevin Hilman wrote:
>> Mike Chan <mike@android.com> writes:
>>
>>> Need to lock the res_mutex when traversing the res_list.
>>>
>>> Signed-off-by: Mike Chan <mike@android.com>
>>
>> Looks good, thanks.
>
> This patch causes a hang for me when transitioning to OFF mode. This
> was tested on the Android 2.6.29 tree and is 100% reproducible. The
> moment a user runs 'echo 1 > /sys/power/enable_off_mode' the board
> hangs without any further output.
>
> Reverting the patch allows me to hit OFF mode again. I haven't yet
> tested this on vanilla 2.6.29 or latest L-O.
>
OK, reverting this in both PM branches. Looks like a deadlock to me,
as update_resource_level() must cause a call to something else that
takes the mutex.
Kevin
>
>> Pushed to PM branch and pm-2.6.29.
>>
>> Kevin
>>
>>> ---
>>> arch/arm/plat-omap/resource.c | 2 ++
>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
>>> index 25072cd..4631912 100644
>>> --- a/arch/arm/plat-omap/resource.c
>>> +++ b/arch/arm/plat-omap/resource.c
>>> @@ -234,11 +234,13 @@ int resource_refresh(void)
>>> struct shared_resource *resp = NULL;
>>> int ret = 0;
>>> + down(&res_mutex);
>>> list_for_each_entry(resp, &res_list, node) {
>>> ret = update_resource_level(resp);
>>> if (ret)
>>> break;
>>> }
>>> + up(&res_mutex);
>>> return ret;
>>> }
>>> --
>>> 1.5.4.5
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [ARM] omap: resource: Make resource_refresh() thread safe.
2009-09-11 21:10 ` Kevin Hilman
@ 2009-09-11 22:50 ` Mike Chan
0 siblings, 0 replies; 5+ messages in thread
From: Mike Chan @ 2009-09-11 22:50 UTC (permalink / raw)
To: Kevin Hilman; +Cc: Mike Turquette, linux-omap
On Fri, Sep 11, 2009 at 2:10 PM, Kevin
Hilman<khilman@deeprootsystems.com> wrote:
> Mike Turquette <mturquette@ti.com> writes:
>
>> Kevin Hilman wrote:
>>> Mike Chan <mike@android.com> writes:
>>>
>>>> Need to lock the res_mutex when traversing the res_list.
>>>>
>>>> Signed-off-by: Mike Chan <mike@android.com>
>>>
>>> Looks good, thanks.
>>
>> This patch causes a hang for me when transitioning to OFF mode. This
>> was tested on the Android 2.6.29 tree and is 100% reproducible. The
>> moment a user runs 'echo 1 > /sys/power/enable_off_mode' the board
>> hangs without any further output.
>>
>> Reverting the patch allows me to hit OFF mode again. I haven't yet
>> tested this on vanilla 2.6.29 or latest L-O.
>>
>
> OK, reverting this in both PM branches. Looks like a deadlock to me,
> as update_resource_level() must cause a call to something else that
> takes the mutex.
>
Good catch, I see the deadlock are in the change_level function
pointers. Where set_xxx will call resource_request() or
resource_release() => resource_lookup() which grabs the list mutex we
grabbed in resource_refresh().
Thoughts on changing the mutex to a RW semaphore?
-- Mike
> Kevin
>
>>
>>> Pushed to PM branch and pm-2.6.29.
>>>
>>> Kevin
>>>
>>>> ---
>>>> arch/arm/plat-omap/resource.c | 2 ++
>>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/arm/plat-omap/resource.c b/arch/arm/plat-omap/resource.c
>>>> index 25072cd..4631912 100644
>>>> --- a/arch/arm/plat-omap/resource.c
>>>> +++ b/arch/arm/plat-omap/resource.c
>>>> @@ -234,11 +234,13 @@ int resource_refresh(void)
>>>> struct shared_resource *resp = NULL;
>>>> int ret = 0;
>>>> + down(&res_mutex);
>>>> list_for_each_entry(resp, &res_list, node) {
>>>> ret = update_resource_level(resp);
>>>> if (ret)
>>>> break;
>>>> }
>>>> + up(&res_mutex);
>>>> return ret;
>>>> }
>>>> --
>>>> 1.5.4.5
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-09-11 22:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-03 23:18 [PATCH] [ARM] omap: resource: Make resource_refresh() thread safe Mike Chan
2009-09-08 22:45 ` Kevin Hilman
2009-09-11 4:12 ` Mike Turquette
2009-09-11 21:10 ` Kevin Hilman
2009-09-11 22:50 ` Mike Chan
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.