Re: [PATCH 1/2] cpuidle: Avoid calls to cpuidle_resume|pause() for s2idle

Rafael J. Wysocki
2021-10-09T15:40:06Z

On Wednesday, September 29, 2021 4:44:50 PM CEST Ulf Hansson wrote:
> In s2idle_enter(), cpuidle_resume|pause() are invoked to re-allow calls to
> the cpuidle callbacks during s2idle operations. This is needed because
> cpuidle is paused in-between in dpm_suspend_noirq() and dpm_resume_noirq().

Well, in fact, doing that last thing for s2idle is pointless, because cpuidle
is going to be resumed eventually anyway in that case and the breakage expected
to be prevented by the pausing will still occur.

So I would rather do something like the patch below (untested).

---
drivers/base/power/main.c | 11 ++++++-----
kernel/power/suspend.c | 8 ++++++--
2 files changed, 12 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -747,8 +747,6 @@ void dpm_resume_noirq(pm_message_t state

resume_device_irqs();
device_wakeup_disarm_wake_irqs();
-
- cpuidle_resume();
}

/**
@@ -881,6 +879,7 @@ void dpm_resume_early(pm_message_t state
void dpm_resume_start(pm_message_t state)
{
dpm_resume_noirq(state);
+ cpuidle_resume();
dpm_resume_early(state);
}
EXPORT_SYMBOL_GPL(dpm_resume_start);
@@ -1336,8 +1335,6 @@ int dpm_suspend_noirq(pm_message_t state
{
int ret;

- cpuidle_pause();
-
device_wakeup_arm_wake_irqs();
suspend_device_irqs();

@@ -1521,9 +1518,13 @@ int dpm_suspend_end(pm_message_t state)
if (error)
goto out;

+ cpuidle_pause();
+
error = dpm_suspend_noirq(state);
- if (error)
+ if (error) {
+ cpuidle_resume();
dpm_resume_early(resume_event(state));
+ }

out:
dpm_show_time(starttime, state, error, "end");
Index: linux-pm/kernel/power/suspend.c
===================================================================
--- linux-pm.orig/kernel/power/suspend.c
+++ linux-pm/kernel/power/suspend.c
@@ -97,7 +97,6 @@ static void s2idle_enter(void)
raw_spin_unlock_irq(&s2idle_lock);

cpus_read_lock();
- cpuidle_resume();

/* Push all the CPUs into the idle loop. */
wake_up_all_idle_cpus();
@@ -105,7 +104,6 @@ static void s2idle_enter(void)
swait_event_exclusive(s2idle_wait_head,
s2idle_state == S2IDLE_STATE_WAKE);

- cpuidle_pause();
cpus_read_unlock();

raw_spin_lock_irq(&s2idle_lock);
@@ -405,6 +403,9 @@ static int suspend_enter(suspend_state_t
if (error)
goto Devices_early_resume;

+ if (state != PM_SUSPEND_TO_IDLE)
+ cpuidle_pause();
+
error = dpm_suspend_noirq(PMSG_SUSPEND);
if (error) {
pr_err("noirq suspend of devices failed\n");
@@ -459,6 +460,9 @@ static int suspend_enter(suspend_state_t
dpm_resume_noirq(PMSG_RESUME);

Platform_early_resume:
+ if (state != PM_SUSPEND_TO_IDLE)
+ cpuidle_resume();
+
platform_resume_early(state);