| Message ID | 1252019887-4463-1-git-send-email-mike@android.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | Kevin Hilman |
| Headers | show |
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 -- 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
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 > -- 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
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 >> -- 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
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
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; }
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(-)