From: Ulf Hansson <ulf.hansson@linaro.org>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
Ulf Hansson <ulf.hansson@linaro.org>,
linux-pm@vger.kernel.org
Cc: Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
Kevin Hilman <khilman@kernel.org>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Lina Iyer <lina.iyer@linaro.org>,
Jon Hunter <jonathanh@nvidia.com>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Brian Norris <briannorris@chromium.org>
Subject: [PATCH] PM / Domains: Restore lock-less behaviour for the genpd syscore APIs
Date: Tue, 7 Feb 2017 16:53:04 +0100 [thread overview]
Message-ID: <1486482784-31734-1-git-send-email-ulf.hansson@linaro.org> (raw)
The commit ef4f7e2c8335 ("PM / Domains: Fix asynchronous execution of
*noirq() callbacks") went too far, as it not only changed to use locks for
the *noirq callbacks, but also for the genpd syscore APIs.
This cause the following error, reported by Geert Uytterhoeven:
"This causes the following BUG on all my Renesas arm32 boards, where the
system timer is an IRQ safe device:
BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:232
in_atomic(): 0, irqs_disabled(): 128, pid: 1751, name: s2ram
CPU: 0 PID: 1751 Comm: s2ram Not tainted
4.10.0-rc7-koelsch-05643-g27f4c73972a614fe #3354
Hardware name: Generic R8A7791 (Flattened Device Tree)
[<c020e9c4>] (unwind_backtrace) from [<c020a40c>] (show_stack+0x10/0x14)
[<c020a40c>] (show_stack) from [<c03f9818>] (dump_stack+0x7c/0x9c)
[<c03f9818>] (dump_stack) from [<c0240020>] (___might_sleep+0x124/0x160)
[<c0240020>] (___might_sleep) from [<c06fedfc>] (mutex_lock+0x18/0x60)
[<c06fedfc>] (mutex_lock) from [<c04de340>]
(genpd_syscore_switch+0x2c/0x7c)
[<c04de340>] (genpd_syscore_switch) from [<c05ec328>]
(sh_cmt_clock_event_suspend+0x18/0x28)
[<c05ec328>] (sh_cmt_clock_event_suspend) from [<c027f9a4>]
(clockevents_suspend+0x40/0x54)
[<c027f9a4>] (clockevents_suspend) from [<c0276d48>]
(timekeeping_suspend+0x23c/0x278)
[<c0276d48>] (timekeeping_suspend) from [<c04cbb88>]
(syscore_suspend+0x88/0x138)
[<c04cbb88>] (syscore_suspend) from [<c025f618>]
(suspend_devices_and_enter+0x290/0x470)
[<c025f618>] (suspend_devices_and_enter) from [<c025fa20>]
(pm_suspend+0x228/0x280)
[<c025fa20>] (pm_suspend) from [<c025e60c>] (state_store+0xac/0xcc)
[<c025e60c>] (state_store) from [<c0340c4c>] (kernfs_fop_write+0x160/0x19c)
[<c0340c4c>] (kernfs_fop_write) from [<c02e3054>] (__vfs_write+0x20/0x108)
[<c02e3054>] (__vfs_write) from [<c02e4424>] (vfs_write+0xb8/0x144)
[<c02e4424>] (vfs_write) from [<c02e5014>] (SyS_write+0x40/0x80)
[<c02e5014>] (SyS_write) from [<c0206cc0>] (ret_fast_syscall+0x0/0x34)"
To fix this problem, restore the lock-less behaviour. However only for the
genpd syscore APIs.
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Fixes: ef4f7e2c8335 ("PM / Domains: Fix asynchronous execution of *noirq() callbacks")
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/base/power/domain.c | 48 +++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 271e208..3a75fb1 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -729,16 +729,17 @@ static bool genpd_dev_active_wakeup(struct generic_pm_domain *genpd,
/**
* genpd_sync_power_off - Synchronously power off a PM domain and its masters.
* @genpd: PM domain to power off, if possible.
+ * @use_lock: use the lock.
* @depth: nesting count for lockdep.
*
* Check if the given PM domain can be powered off (during system suspend or
* hibernation) and do that if so. Also, in that case propagate to its masters.
*
* This function is only called in "noirq" and "syscore" stages of system power
- * transitions, but since the "noirq" callbacks may be executed asynchronously,
- * the lock must be held.
+ * transitions. The "noirq" callbacks may be executed asynchronously, thus in
+ * these cases the lock must be held.
*/
-static void genpd_sync_power_off(struct generic_pm_domain *genpd,
+static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
unsigned int depth)
{
struct gpd_link *link;
@@ -759,22 +760,27 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd,
list_for_each_entry(link, &genpd->slave_links, slave_node) {
genpd_sd_counter_dec(link->master);
- genpd_lock_nested(link->master, depth + 1);
- genpd_sync_power_off(link->master, depth + 1);
- genpd_unlock(link->master);
+ if (use_lock)
+ genpd_lock_nested(link->master, depth + 1);
+
+ genpd_sync_power_off(link->master, use_lock, depth + 1);
+
+ if (use_lock)
+ genpd_unlock(link->master);
}
}
/**
* genpd_sync_power_on - Synchronously power on a PM domain and its masters.
* @genpd: PM domain to power on.
+ * @use_lock: use the lock.
* @depth: nesting count for lockdep.
*
* This function is only called in "noirq" and "syscore" stages of system power
- * transitions, but since the "noirq" callbacks may be executed asynchronously,
- * the lock must be held.
+ * transitions. The "noirq" callbacks may be executed asynchronously, thus in
+ * these cases the lock must be held.
*/
-static void genpd_sync_power_on(struct generic_pm_domain *genpd,
+static void genpd_sync_power_on(struct generic_pm_domain *genpd, bool use_lock,
unsigned int depth)
{
struct gpd_link *link;
@@ -785,9 +791,13 @@ static void genpd_sync_power_on(struct generic_pm_domain *genpd,
list_for_each_entry(link, &genpd->slave_links, slave_node) {
genpd_sd_counter_inc(link->master);
- genpd_lock_nested(link->master, depth + 1);
- genpd_sync_power_on(link->master, depth + 1);
- genpd_unlock(link->master);
+ if (use_lock)
+ genpd_lock_nested(link->master, depth + 1);
+
+ genpd_sync_power_on(link->master, use_lock, depth + 1);
+
+ if (use_lock)
+ genpd_unlock(link->master);
}
_genpd_power_on(genpd, false);
@@ -898,7 +908,7 @@ static int pm_genpd_suspend_noirq(struct device *dev)
genpd_lock(genpd);
genpd->suspended_count++;
- genpd_sync_power_off(genpd, 0);
+ genpd_sync_power_off(genpd, true, 0);
genpd_unlock(genpd);
return 0;
@@ -925,7 +935,7 @@ static int pm_genpd_resume_noirq(struct device *dev)
return 0;
genpd_lock(genpd);
- genpd_sync_power_on(genpd, 0);
+ genpd_sync_power_on(genpd, true, 0);
genpd->suspended_count--;
genpd_unlock(genpd);
@@ -1016,7 +1026,7 @@ static int pm_genpd_restore_noirq(struct device *dev)
*/
genpd->status = GPD_STATE_POWER_OFF;
- genpd_sync_power_on(genpd, 0);
+ genpd_sync_power_on(genpd, true, 0);
genpd_unlock(genpd);
if (genpd->dev_ops.stop && genpd->dev_ops.start)
@@ -1070,17 +1080,13 @@ static void genpd_syscore_switch(struct device *dev, bool suspend)
if (!pm_genpd_present(genpd))
return;
- genpd_lock(genpd);
-
if (suspend) {
genpd->suspended_count++;
- genpd_sync_power_off(genpd, 0);
+ genpd_sync_power_off(genpd, false, 0);
} else {
- genpd_sync_power_on(genpd, 0);
+ genpd_sync_power_on(genpd, false, 0);
genpd->suspended_count--;
}
-
- genpd_unlock(genpd);
}
void pm_genpd_syscore_poweroff(struct device *dev)
--
1.9.1
next reply other threads:[~2017-02-07 15:53 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-07 15:53 Ulf Hansson [this message]
2017-02-07 18:29 ` [PATCH] PM / Domains: Restore lock-less behaviour for the genpd syscore APIs Kevin Hilman
2017-02-08 12:06 ` Ulf Hansson
2017-02-08 12:06 ` Rafael J. Wysocki
2017-02-08 12:13 ` Ulf Hansson
2017-02-08 12:15 ` Rafael J. Wysocki
2017-02-08 12:35 ` Ulf Hansson
2017-02-07 19:09 ` Geert Uytterhoeven
2017-02-07 21:52 ` Lina Iyer
2017-02-08 12:41 ` Ulf Hansson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1486482784-31734-1-git-send-email-ulf.hansson@linaro.org \
--to=ulf.hansson@linaro.org \
--cc=briannorris@chromium.org \
--cc=geert@linux-m68k.org \
--cc=jonathanh@nvidia.com \
--cc=khilman@kernel.org \
--cc=len.brown@intel.com \
--cc=lina.iyer@linaro.org \
--cc=linux-pm@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=pavel@ucw.cz \
--cc=rjw@rjwysocki.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).