| Message ID | 20220629011146.299521-3-xiongxin@kylinos.cn (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | PM: suspend: Optimized suspend is fail returned by wakeup | expand |
On Wed, Jun 29, 2022 at 5:35 AM xiongxin <xiongxin@kylinos.cn> wrote: > > pm_wakeup_clear() will clear the wakeup source, which can ensure that it > is not disturbed by useless wakeup signals when doing suspend/hibernate; > > At the beginning of the suspend/hibernate process, the notifier > mechanism is used to notify other device drivers. This action is > time-consuming (second-level time-consuming). If the process fails due > to the received wakeup signal during the execution of these functions, > it can better improve the experience of failing suspend/hibernate > returns; > > Therefore, it is recommended here that for the suspend/hibernate process > normally called from /sys/power/state, the pm_wakeup_clear() function > should be brought before the notifier call; for the freeze_process() > function called from other places, the original logic is kept; > > The pm_suspend_target_state variable is used here to identify whether the > suspend process is going normally. You seem to be arguing that the previous wakeup IRQ should be cleared before invoking PM notifiers. However, it is only set in pm_system_irq_wakeup() which is only called after suspend_device_irqs(), so clearing it anywhere before calling that function in the given cycle should be sufficient. > Signed-off-by: xiongxin <xiongxin@kylinos.cn> > --- > kernel/power/process.c | 5 ++++- > kernel/power/suspend.c | 6 ++++++ > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/kernel/power/process.c b/kernel/power/process.c > index 3068601e585a..3fde0240b3d1 100644 > --- a/kernel/power/process.c > +++ b/kernel/power/process.c > @@ -131,7 +131,10 @@ int freeze_processes(void) > if (!pm_freezing) > atomic_inc(&system_freezing_cnt); > > - pm_wakeup_clear(0); > + if (pm_suspend_target_state != PM_SUSPEND_ON) > + pm_wakeup_clear(1); This doesn't make sense. > + else > + pm_wakeup_clear(0); > pr_info("Freezing user space processes ... "); > pm_freezing = true; > error = try_to_freeze_tasks(true); > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c > index c754b084ec03..f4259f6c1cc2 100644 > --- a/kernel/power/suspend.c > +++ b/kernel/power/suspend.c > @@ -569,6 +569,12 @@ static int enter_state(suspend_state_t state) > * performed from the /sys/power/state entry. > */ > pm_suspend_target_state = state; > + /* > + * Put pm_wakeup_clear() before the notifier notification chain to > + * optimize in the suspend process, the wakeup signal can interrupt > + * the suspend in advance and fail to return. > + */ The comment above is too hard to understand for me, sorry. > + pm_wakeup_clear(0); > > if (sync_on_suspend_enabled) { > trace_suspend_resume(TPS("sync_filesystems"), 0, true); > -- > 2.25.1 > > > No virus found > Checked by Hillstone Network AntiVirus
diff --git a/kernel/power/process.c b/kernel/power/process.c index 3068601e585a..3fde0240b3d1 100644 --- a/kernel/power/process.c +++ b/kernel/power/process.c @@ -131,7 +131,10 @@ int freeze_processes(void) if (!pm_freezing) atomic_inc(&system_freezing_cnt); - pm_wakeup_clear(0); + if (pm_suspend_target_state != PM_SUSPEND_ON) + pm_wakeup_clear(1); + else + pm_wakeup_clear(0); pr_info("Freezing user space processes ... "); pm_freezing = true; error = try_to_freeze_tasks(true); diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index c754b084ec03..f4259f6c1cc2 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -569,6 +569,12 @@ static int enter_state(suspend_state_t state) * performed from the /sys/power/state entry. */ pm_suspend_target_state = state; + /* + * Put pm_wakeup_clear() before the notifier notification chain to + * optimize in the suspend process, the wakeup signal can interrupt + * the suspend in advance and fail to return. + */ + pm_wakeup_clear(0); if (sync_on_suspend_enabled) { trace_suspend_resume(TPS("sync_filesystems"), 0, true);
pm_wakeup_clear() will clear the wakeup source, which can ensure that it is not disturbed by useless wakeup signals when doing suspend/hibernate; At the beginning of the suspend/hibernate process, the notifier mechanism is used to notify other device drivers. This action is time-consuming (second-level time-consuming). If the process fails due to the received wakeup signal during the execution of these functions, it can better improve the experience of failing suspend/hibernate returns; Therefore, it is recommended here that for the suspend/hibernate process normally called from /sys/power/state, the pm_wakeup_clear() function should be brought before the notifier call; for the freeze_process() function called from other places, the original logic is kept; The pm_suspend_target_state variable is used here to identify whether the suspend process is going normally. Signed-off-by: xiongxin <xiongxin@kylinos.cn> --- kernel/power/process.c | 5 ++++- kernel/power/suspend.c | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-)