lkml.org 
[lkml]   [2022]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH -next 2/2] PM: suspend: advanced pm_wakeup_clear() for normal suspend/hibernate
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

\
 
 \ /
  Last update: 2022-07-12 21:16    [W:0.051 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site