* [PATCH] PM: s2idle: Introduce syscore callbacks in s2idle flow @ 2020-09-01 7:04 Claude Yen 2020-09-01 11:57 ` Rafael J. Wysocki 0 siblings, 1 reply; 3+ messages in thread From: Claude Yen @ 2020-09-01 7:04 UTC (permalink / raw) To: Rafael J. Wysocki, Daniel Lezcano, Frederic Weisbecker, Thomas Gleixner, Ingo Molnar, Matthias Brugger, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Douglas Anderson Cc: linux-pm, linux-kernel, linux-arm-kernel, linux-mediatek, wsd_upstream This series based on 5.9-rc1 This patch makes s2idle call existing syscore callbacks. Currently, when s2idle is selected as system suspend method, callbacks hooked by register_syscore_ops() will not be triggered. This may induce unexpected results. For example, sched_clock_suspend() was added to s2idle flow in commit 3f2552f7e9c5 ("timers/sched_clock: Prevent generic sched_clock wrap caused by tick_freeze()") to fix clock wrap problem. However, sched_clock_suspend() is originally registered in syscore callback. With this patch, if another syscore callback is needed in s2idle, additional migration effort could be saved. *** BLURB HERE *** claude.yen (1): PM: s2idle: Introduce syscore callbacks in s2idle flow drivers/cpuidle/cpuidle.c | 36 ++++++++++++++++++++++++++++++++---- kernel/cpu_pm.c | 17 +++++++++++++++++ kernel/time/tick-common.c | 17 ++--------------- 3 files changed, 51 insertions(+), 19 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] PM: s2idle: Introduce syscore callbacks in s2idle flow 2020-09-01 7:04 [PATCH] PM: s2idle: Introduce syscore callbacks in s2idle flow Claude Yen @ 2020-09-01 11:57 ` Rafael J. Wysocki [not found] ` <1599099247.4435.4.camel@mtksdccf07> 0 siblings, 1 reply; 3+ messages in thread From: Rafael J. Wysocki @ 2020-09-01 11:57 UTC (permalink / raw) To: Claude Yen Cc: Rafael J. Wysocki, Daniel Lezcano, Frederic Weisbecker, Thomas Gleixner, Ingo Molnar, Matthias Brugger, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Douglas Anderson, Linux PM, Linux Kernel Mailing List, Linux ARM, moderated list:ARM/Mediatek SoC..., wsd_upstream On Tue, Sep 1, 2020 at 9:05 AM Claude Yen <claude.yen@mediatek.com> wrote: > > This series based on 5.9-rc1 > This patch makes s2idle call existing syscore callbacks. Currently, > when s2idle is selected as system suspend method, callbacks hooked > by register_syscore_ops() will not be triggered. This may induce > unexpected results. They are not executed by design. > For example, sched_clock_suspend() was added to s2idle flow in > commit 3f2552f7e9c5 ("timers/sched_clock: Prevent generic sched_clock > wrap caused by tick_freeze()") to fix clock wrap problem. However, > sched_clock_suspend() is originally registered in syscore callback. I'm not sure why this matters here. > With this patch, if another syscore callback is needed in s2idle, > additional migration effort could be saved. s2idle cannot execute syscore callbacks, because it doesn' take non-boot CPUs offline and it won't do that. Thanks! ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <1599099247.4435.4.camel@mtksdccf07>]
* Re: [PATCH] PM: s2idle: Introduce syscore callbacks in s2idle flow [not found] ` <1599099247.4435.4.camel@mtksdccf07> @ 2020-09-10 12:55 ` Rafael J. Wysocki 0 siblings, 0 replies; 3+ messages in thread From: Rafael J. Wysocki @ 2020-09-10 12:55 UTC (permalink / raw) To: Claude. Yen Cc: Rafael J. Wysocki, Daniel Lezcano, Frederic Weisbecker, Thomas Gleixner, Ingo Molnar, Matthias Brugger, Bjorn Andersson, Greg Kroah-Hartman, Stephen Boyd, Douglas Anderson, Linux PM, Linux Kernel Mailing List, Linux ARM, moderated list:ARM/Mediatek SoC..., wsd_upstream On Thursday, September 3, 2020 4:14:07 AM CEST Claude. Yen wrote: > On Tue, 2020-09-01 at 13:57 +0200, Rafael J. Wysocki wrote: > > On Tue, Sep 1, 2020 at 9:05 AM Claude Yen <claude.yen@mediatek.com> wrote: > > > > > > This series based on 5.9-rc1 > > > This patch makes s2idle call existing syscore callbacks. Currently, > > > when s2idle is selected as system suspend method, callbacks hooked > > > by register_syscore_ops() will not be triggered. This may induce > > > unexpected results. > > > > They are not executed by design. > > > > > For example, sched_clock_suspend() was added to s2idle flow in > > > commit 3f2552f7e9c5 ("timers/sched_clock: Prevent generic sched_clock > > > wrap caused by tick_freeze()") to fix clock wrap problem. However, > > > sched_clock_suspend() is originally registered in syscore callback. > > > > I'm not sure why this matters here. > > If functions in syscore callbacks are needed in s2idle, explicit > migration is needed like commit 3f2552f7e9c5 ("timers/sched_clock: > Prevent generic sched_clock wrap caused by tick_freeze()"). > Thus, I am wondering if such effort could be saved. Yes, it could. You can define platform ops for s2idle and invoke what's needed from there. > > > With this patch, if another syscore callback is needed in s2idle, > > > additional migration effort could be saved. > > > > s2idle cannot execute syscore callbacks, because it doesn' take > > non-boot CPUs offline and it won't do that. > > > > Thanks! > > Yes, the current design of syscore callback needs non-boot CPUs offline. > Considering the following case: in s2idle flow, there is a status that > only one CPU is alive and other CPUs have enter deepest idle state. > This situation is similar to getting non-boot CPUs offline, though all > CPUs are online from kernel's perspective. It is only similar AFAICS. You don't migrate interrupts during s2idle, for example. > Reply from Stephen mentioned that if an operation is needed in both > S2R and s2idle, CPU_PM notifier can be utilized. > In my opinion, CPU_PM notifier is particularly for CPU entering idle > state. In contrast, syscore callback is for system going low power > state. There exists semantic difference between these two callbacks. Fair enough. > Could the current design of syscore callback be re-designed as > system-wide suspend callback? No, it couldn't. > Proposed suspend flow in this patch: > > Freeze tasks > | > V > Device suspend callbacks > | > |-------------s2idle---------- > | | > V | > Disable nonboot CPUs Is this CPU last core to enter idle? > | | > V |------------- > syscore callbacks | | > | No Yes > V | | > platform suspend V V > enter idle syscore callback > | > V > enter idle > The primary problem with this is that on some architectures (x86 at least) the syscore things cannot be run during the s2idle flow. Also there is a way to invoke them through the platform ops as I said. Thanks! ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-10 21:54 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-01 7:04 [PATCH] PM: s2idle: Introduce syscore callbacks in s2idle flow Claude Yen 2020-09-01 11:57 ` Rafael J. Wysocki [not found] ` <1599099247.4435.4.camel@mtksdccf07> 2020-09-10 12:55 ` Rafael J. Wysocki
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).