[PATCH -next 0/2] PM: suspend: Optimized suspend is fail returned by wakeup

When the suspend process is executed from the /sys/power/state entry,
the pm_wakeup_clear() signal is cleared in advance, and the wakeup
signal can be captured to fail the suspend process when the suspend
process is notified by the notifier;

Expanding the scope of the pm_suspend_target_state variable also allows
the device driver to know that the system has entered the suspend
process earlier.

xiongxin (2):
  PM: suspend: expand the assignment scope of the
    pm_suspend_target_state
  PM: suspend: advanced pm_wakeup_clear() for normal suspend/hibernate

 kernel/power/process.c |  5 ++++-
 kernel/power/suspend.c | 13 +++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

The pm_suspend_target_state variable can be used as a suspend state
identifier and given to the specific device driver as a code judgment;

Because the suspend_prepare() function is time-consuming for the
operation of freezing processes, and this stage is actually in the suspend
stage, it is necessary to expand the scope of the pm_suspend_target_state
variable to be assigned to the suspend state at enter_state() function;

Another reason is that the specific device driver can locate whether it is
in the suspend state based on this variable, so as to determine the
validity of its wake-up source.

Signed-off-by: xiongxin <redacted>
---
 kernel/power/suspend.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 827075944d28..c754b084ec03 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c

@@ -564,6 +564,12 @@ static int enter_state(suspend_state_t state)
 	if (state == PM_SUSPEND_TO_IDLE)
 		s2idle_begin();
 
+	/*
+	 * Expand the scope of suspend state for suspend operations
+	 * performed from the /sys/power/state entry.
+	 */
+	pm_suspend_target_state = state;
+
 	if (sync_on_suspend_enabled) {
 		trace_suspend_resume(TPS("sync_filesystems"), 0, true);
 		ksys_sync_helper();

@@ -590,6 +596,7 @@ static int enter_state(suspend_state_t state)
 	pm_pr_dbg("Finishing wakeup.\n");
 	suspend_finish();
  Unlock:
+	pm_suspend_target_state = PM_SUSPEND_ON;
 	mutex_unlock(&system_transition_mutex);
 	return error;
 }

-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

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 <redacted>
---
 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);
+	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);

-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

On Wed, Jun 29, 2022 at 5:35 AM xiongxin [off-list ref] wrote:

The pm_suspend_target_state variable can be used as a suspend state
identifier and given to the specific device driver as a code judgment;

Because the suspend_prepare() function is time-consuming for the
operation of freezing processes, and this stage is actually in the suspend
stage, it is necessary to expand the scope of the pm_suspend_target_state
variable to be assigned to the suspend state at enter_state() function;

I don't see why it is necessary, sorry.

Another reason is that the specific device driver can locate whether it is
in the suspend state based on this variable, so as to determine the
validity of its wake-up source.

I think you have a specific use case in mind, so please explain why
and how this change helps in your specific use case.

So far I don't see a reason for applying this patch.

On Wed, Jun 29, 2022 at 5:35 AM xiongxin [off-list ref] 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.

This doesn't make sense.

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