PM / suspend: Count extra iterations of s2idle loop as sleep time

Commit Message

Rafael J. Wysocki Sept. 13, 2018, 10:02 a.m. UTC

From: Rafael J. Wysocki <[e-mail]>

There is a difference in behavior between suspend-to-idle and
suspend-to-RAM in the timekeeping handling that leads to functional
issues.  Namely, every iteration of the loop in s2idle_loop()
increases the monotinic clock somewhat, even if timekeeping_suspend()
and timekeeping_resume() are invoked from s2idle_enter(), and if
many of them are carried out in a row, the monotonic clock can grow
significantly while the system is regarded as suspended, which
doesn't happend during suspend-to-RAM and so it is unexpected and
leads to confusion and misbehavior in user space (similar to what
ensued when we tried to combine the boottime and monotonic clocks).

To avoid that, count all iterations of the loop in s2idle_loop()
except for the last one as "sleep time" and adjust the clock for
that on exit from suspend-to-idle.

Fixes: 33e4f80ee69b (ACPI / PM: Ignore spurious SCI wakeups from suspend-to-idle)
Signed-off-by: Rafael J. Wysocki <[e-mail]>
---
 kernel/power/suspend.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

kernel test robot Sept. 16, 2018, 3:48 p.m. UTC | #1

Hi Rafael,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc3 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Rafael-J-Wysocki/PM-suspend-Count-extra-iterations-of-s2idle-loop-as-sleep-time/20180916-163810
config: powerpc-ppc6xx_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   kernel/power/suspend.o: In function `suspend_devices_and_enter':
>> (.text+0x6b4): undefined reference to `timekeeping_inject_sleeptime64'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch

Index: linux-pm/kernel/power/suspend.c
===================================================================
--- linux-pm.orig/kernel/power/suspend.c
+++ linux-pm/kernel/power/suspend.c
@@ -109,11 +109,16 @@  static void s2idle_enter(void)
 
 static void s2idle_loop(void)
 {
+	ktime_t delta = 0;
+
 	pm_pr_dbg("suspend-to-idle\n");
 
 	for (;;) {
+		ktime_t before;
 		int error;
 
+		before = ktime_get();
+
 		dpm_noirq_begin();
 
 		/*
@@ -148,6 +153,23 @@  static void s2idle_loop(void)
 			break;
 
 		pm_wakeup_clear(false);
+
+		/*
+		 * Count all iterations except for the last one as "sleep time".
+		 */
+		delta = ktime_add(delta, ktime_sub(ktime_get(), before));
+	}
+
+	/*
+	 * If the monotonic clock difference between the start of the loop and
+	 * this point is too large, user space may get confused about whether or
+	 * not the system has been suspended and tasks may get killed by
+	 * watchdogs etc., so compensate for that.
+	 */
+	if (ktime_to_ns(delta) > 0) {
+		struct timespec64 timespec64_delta = ktime_to_timespec64(delta);
+
+		timekeeping_inject_sleeptime64(&timespec64_delta);
 	}
 
 	pm_pr_dbg("resume from suspend-to-idle\n");