| Message ID | 1453288059-1988-1-git-send-email-ulf.hansson@linaro.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
On Wednesday, January 20, 2016 12:07:39 PM Ulf Hansson wrote: > We must preserve the same order of how we acquire and release the lock for > genpd, as otherwise we may encounter deadlocks. > > The power on phase of a genpd starts by acquiring its lock. Then it walks > the hierarchy of its parent domains to be able to power on these first, as > per design of genpd. > > From a locking perspective this means the locks of the parents becomes > acquired after the lock of the subdomain. > > Let's fix pm_genpd_add|remove_subdomain() to maintain the same order of > acquiring/releasing the genpd lock as being applied in the power on/off > sequence. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Applied, thanks! Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rafael, On Sun, Jan 24, 2016 at 2:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Wednesday, January 20, 2016 12:07:39 PM Ulf Hansson wrote: >> We must preserve the same order of how we acquire and release the lock for >> genpd, as otherwise we may encounter deadlocks. >> >> The power on phase of a genpd starts by acquiring its lock. Then it walks >> the hierarchy of its parent domains to be able to power on these first, as >> per design of genpd. >> >> From a locking perspective this means the locks of the parents becomes >> acquired after the lock of the subdomain. >> >> Let's fix pm_genpd_add|remove_subdomain() to maintain the same order of >> acquiring/releasing the genpd lock as being applied in the power on/off >> sequence. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > Applied, thanks! It looks like you accidentally applied v1, introducing the lockdep warnings fixed in v2? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26 January 2016 at 14:13, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Rafael, > > On Sun, Jan 24, 2016 at 2:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> On Wednesday, January 20, 2016 12:07:39 PM Ulf Hansson wrote: >>> We must preserve the same order of how we acquire and release the lock for >>> genpd, as otherwise we may encounter deadlocks. >>> >>> The power on phase of a genpd starts by acquiring its lock. Then it walks >>> the hierarchy of its parent domains to be able to power on these first, as >>> per design of genpd. >>> >>> From a locking perspective this means the locks of the parents becomes >>> acquired after the lock of the subdomain. >>> >>> Let's fix pm_genpd_add|remove_subdomain() to maintain the same order of >>> acquiring/releasing the genpd lock as being applied in the power on/off >>> sequence. >>> >>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> >> Applied, thanks! > > It looks like you accidentally applied v1, introducing the lockdep warnings > fixed in v2? > Correct, it's the v1. Rafael, if you can't replace the patch I can send an incremental fix on top. Just tell me what way you prefer. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, January 26, 2016 03:59:19 PM Ulf Hansson wrote: > On 26 January 2016 at 14:13, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Rafael, > > > > On Sun, Jan 24, 2016 at 2:20 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > >> On Wednesday, January 20, 2016 12:07:39 PM Ulf Hansson wrote: > >>> We must preserve the same order of how we acquire and release the lock for > >>> genpd, as otherwise we may encounter deadlocks. > >>> > >>> The power on phase of a genpd starts by acquiring its lock. Then it walks > >>> the hierarchy of its parent domains to be able to power on these first, as > >>> per design of genpd. > >>> > >>> From a locking perspective this means the locks of the parents becomes > >>> acquired after the lock of the subdomain. > >>> > >>> Let's fix pm_genpd_add|remove_subdomain() to maintain the same order of > >>> acquiring/releasing the genpd lock as being applied in the power on/off > >>> sequence. > >>> > >>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > >> > >> Applied, thanks! > > > > It looks like you accidentally applied v1, introducing the lockdep warnings > > fixed in v2? > > > > Correct, it's the v1. > > Rafael, if you can't replace the patch I can send an incremental fix > on top. Just tell me what way you prefer. Can you please send the whole patch again? Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" 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/drivers/base/power/domain.c b/drivers/base/power/domain.c index 6ac9a7f..676d762 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1339,8 +1339,8 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, if (!link) return -ENOMEM; - mutex_lock(&genpd->lock); - mutex_lock_nested(&subdomain->lock, SINGLE_DEPTH_NESTING); + mutex_lock(&subdomain->lock); + mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING); if (genpd->status == GPD_STATE_POWER_OFF && subdomain->status != GPD_STATE_POWER_OFF) { @@ -1363,8 +1363,8 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, genpd_sd_counter_inc(genpd); out: - mutex_unlock(&subdomain->lock); mutex_unlock(&genpd->lock); + mutex_unlock(&subdomain->lock); if (ret) kfree(link); return ret; @@ -1385,7 +1385,8 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd, if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(subdomain)) return -EINVAL; - mutex_lock(&genpd->lock); + mutex_lock(&subdomain->lock); + mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING); if (!list_empty(&subdomain->slave_links) || subdomain->device_count) { pr_warn("%s: unable to remove subdomain %s\n", genpd->name, @@ -1398,22 +1399,19 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd, if (link->slave != subdomain) continue; - mutex_lock_nested(&subdomain->lock, SINGLE_DEPTH_NESTING); - list_del(&link->master_node); list_del(&link->slave_node); kfree(link); if (subdomain->status != GPD_STATE_POWER_OFF) genpd_sd_counter_dec(genpd); - mutex_unlock(&subdomain->lock); - ret = 0; break; } out: mutex_unlock(&genpd->lock); + mutex_unlock(&subdomain->lock); return ret; }
We must preserve the same order of how we acquire and release the lock for genpd, as otherwise we may encounter deadlocks. The power on phase of a genpd starts by acquiring its lock. Then it walks the hierarchy of its parent domains to be able to power on these first, as per design of genpd. From a locking perspective this means the locks of the parents becomes acquired after the lock of the subdomain. Let's fix pm_genpd_add|remove_subdomain() to maintain the same order of acquiring/releasing the genpd lock as being applied in the power on/off sequence. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- Changes in v2: Fix lockdep warning. --- drivers/base/power/domain.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)