| Message ID | 1453288059-1988-1-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
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
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(-) -- 1.9.1