Skip to contentSkip to conversation

Ignore bogus sleep time if pm_trace is enabled

RFC PATCH Superseded Chen Yu 1 participant 5 messages Nov 8, 2016

Subsystems (6)

v6 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Chen Yu

author
#0 lore raw

v6 cover Ignore bogus sleep time if pm_trace is enabled

This patch set is based on Thomas's previous solution, to bypass the bogus CMOS-RTC sleep time after resumed, thus to avoid damage to timekeeping system. The bogus sleep time comes from pm_trace, which might use CMOS-RTC for debugging purpose.

Meanwhile, a solution to save/restore of CMOS-RTC is also proposed to give better user experience.

Chen Yu (2):
timekeeping: Ignore the bogus sleep time if pm_trace is enabled PM / sleep: save/restore RTC time after resumed if pm_trace enabled

 arch/x86/kernel/rtc.c       |  9 +++++++++
 drivers/base/power/trace.c  | 31 +++++++++++++++++++++++++++++++
 drivers/rtc/rtc-cmos.c      |  7 +++++++
 include/linux/mc146818rtc.h |  1 +
 include/linux/pm-trace.h    |  9 ++++++++-
 5 files changed, 56 insertions(+), 1 deletion(-)
Signature · 11 lines
2.7.4

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.
author
#1 lore raw

RFC PATCH v6 1/2 timekeeping: Ignore the bogus sleep time if pm_trace is enabled

From: Thomas Gleixner <[email protected]>

Previously we encountered some memory overflow issues due to the bogus sleep time brought by inconsistent rtc, which is triggered when pm_trace is enabled, and we have fixed it in recent kernel. However it's improper in the first place to call __timekeeping_inject_sleeptime() in case that pm_trace is enabled simply because that "hash" time value will wreckage the timekeeping subsystem.

This patch is to bypass the bogus rtc interval when pm_trace is enabled.

Cc Thomas Gleixner <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Len Brown <[email protected]>
Cc John Stultz <[email protected]>
Cc Xunlei Pang <[email protected]>
---
 arch/x86/kernel/rtc.c       | 9 +++++++++
 drivers/base/power/trace.c  | 2 ++
 drivers/rtc/rtc-cmos.c      | 7 +++++++
 include/linux/mc146818rtc.h | 1 +
 include/linux/pm-trace.h    | 9 ++++++++-
 5 files changed, 27 insertions(+), 1 deletion(-)
arch/x86/kernel/rtc.c +9 -0
index 79c6311c..898383c 100644
@@ -64,6 +64,15 @@ void mach_get_cmos_time(struct timespec *now)
 	unsigned int status, year, mon, day, hour, min, sec, century = 0;
 	unsigned long flags;
 
+	/*
+	 * If pm trace abused the RTC as storage set the timespec to 0
+	 * which tells the caller that this RTC value is bogus.
+	 */
+	if (!pm_trace_rtc_valid()) {
+		now->tv_sec = now->tv_nsec = 0;
+		return;
+	}
+
 	spin_lock_irqsave(&rtc_lock, flags);
 
 	/*
drivers/base/power/trace.c +2 -0
index efec10b..aa9109a 100644
@@ -74,6 +74,7 @@
 
 #define DEVSEED (7919)
 
+bool pm_trace_rtc_abused __read_mostly;
 static unsigned int dev_hash_value;
 
 static int set_magic_time(unsigned int user, unsigned int file, unsigned int device)
@@ -104,6 +105,7 @@ static int set_magic_time(unsigned int user, unsigned int file, unsigned int dev
 	time.tm_min = (n % 20) * 3;
 	n /= 20;
 	mc146818_set_time(&time);
+	pm_trace_rtc_abused = true;
 	return n ? -1 : 0;
 }
 
drivers/rtc/rtc-cmos.c +7 -0
index dd3d598..3d9aedc 100644
@@ -191,6 +191,13 @@ static inline void cmos_write_bank2(unsigned char val, unsigned char addr)
 
 static int cmos_read_time(struct device *dev, struct rtc_time *t)
 {
+	/*
+	 * If pmtrace abused the RTC for storage tell the caller that it is
+	 * unusable.
+	 */
+	if (!pm_trace_rtc_valid())
+		return -EIO;
+
 	/* REVISIT:  if the clock has a "century" register, use
 	 * that instead of the heuristic in mc146818_get_time().
 	 * That'll make Y3K compatility (year > 2070) easy!
include/linux/mc146818rtc.h +1 -0
index a585b4b..0661af1 100644
@@ -16,6 +16,7 @@
 #include <asm/mc146818rtc.h>		/* register access macros */
 #include <linux/bcd.h>
 #include <linux/delay.h>
+#include <linux/pm-trace.h>
 
 #ifdef __KERNEL__
 #include <linux/spinlock.h>		/* spinlock_t */
include/linux/pm-trace.h +8 -1
index ecbde7a..7b78793 100644
@@ -1,11 +1,17 @@
 #ifndef PM_TRACE_H
 #define PM_TRACE_H
 
+#include <linux/types.h>
 #ifdef CONFIG_PM_TRACE
 #include <asm/pm-trace.h>
-#include <linux/types.h>
 
 extern int pm_trace_enabled;
+extern bool pm_trace_rtc_abused;
+
+static inline bool pm_trace_rtc_valid(void)
+{
+	return !pm_trace_rtc_abused;
+}
 
 static inline int pm_trace_is_enabled(void)
 {
@@ -24,6 +30,7 @@ extern int show_trace_dev_match(char *buf, size_t size);
 
 #else
 
+static inline bool pm_trace_rtc_valid(void) { return true; }
 static inline int pm_trace_is_enabled(void) { return 0; }
 
 #define TRACE_DEVICE(dev) do { } while (0)
Signature · 11 lines
2.7.4

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.
author
#3 lore raw

RFC PATCH v6 2/2 PM / sleep: save/restore RTC time after resumed if pm_trace enabled

In reply to Chen Yu

Previously the bogus CMOS RTC sleep time has been ignored if pm_trace is enabled, however once the system successfully resumed back, any further read to CMOS RTC would return an error. Actually it is more user-friendly to bring the system back to normal after resumed.

This patch has registered an pm notifier to restore the RTC to the value before been overwitten by pm_trace.

Cc "Rafael J. Wysocki" <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Len Brown <[email protected]>
Cc John Stultz <[email protected]>
Cc Xunlei Pang <[email protected]>
---
 drivers/base/power/trace.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
drivers/base/power/trace.c +29 -0
index aa9109a..1e6c611 100644
@@ -10,6 +10,7 @@
 #include <linux/pm-trace.h>
 #include <linux/export.h>
 #include <linux/rtc.h>
+#include <linux/suspend.h>
 
 #include <linux/mc146818rtc.h>
 
@@ -76,6 +77,11 @@
 
 bool pm_trace_rtc_abused __read_mostly;
 static unsigned int dev_hash_value;
+struct rtc_time_save {
+	struct rtc_time time;
+	atomic_t saved;
+};
+static struct rtc_time_save rtc_saved;
 
 static int set_magic_time(unsigned int user, unsigned int file, unsigned int device)
 {
@@ -104,6 +110,8 @@ static int set_magic_time(unsigned int user, unsigned int file, unsigned int dev
 	n /= 24;
 	time.tm_min = (n % 20) * 3;
 	n /= 20;
+	if (!atomic_cmpxchg(&rtc_saved.saved, 0, 1))
+		mc146818_get_time(&rtc_saved.time);
 	mc146818_set_time(&time);
 	pm_trace_rtc_abused = true;
 	return n ? -1 : 0;
@@ -240,10 +248,31 @@ int show_trace_dev_match(char *buf, size_t size)
 	device_pm_unlock();
 	return ret;
 }
+static int pm_trace_notify(struct notifier_block *nb,
+				unsigned long mode, void *_unused)
+{
+	switch (mode) {
+	case PM_POST_HIBERNATION:
+	case PM_POST_SUSPEND:
+		if (atomic_cmpxchg(&rtc_saved.saved, 1, 0)) {
+			mc146818_set_time(&rtc_saved.time);
+			pm_trace_rtc_abused = false;
+		}
+		break;
+	default:
+		break;
+	}
+	return 0;
+}
+
+static struct notifier_block pm_trace_nb = {
+	.notifier_call = pm_trace_notify,
+};
 
 static int early_resume_init(void)
 {
 	hash_value_early_read = read_magic_time();
+	register_pm_notifier(&pm_trace_nb);
 	return 0;
 }
 
Signature · 1 line
2.7.4
author
#4 lore raw

Re: [PATCH 2/2][RFC v6] PM / sleep: save/restore RTC time after resumed if pm_trace enabled

Hi all,

Quoted 8 lines · Chen Yu

Previously the bogus CMOS RTC sleep time has been ignored if pm_trace is enabled, however once the system successfully resumed back, any further read to CMOS RTC would return an error. Actually it is more user-friendly to bring the system back to normal after resumed.

This patch has registered an pm notifier to restore the RTC to the value before been overwitten by pm_trace.

Quoted 7 lines
Cc "Rafael J. Wysocki" <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Len Brown <[email protected]>
Cc John Stultz <[email protected]>
Cc Xunlei Pang <[email protected]>

---

Please ignore this version for now,
the atomic variable below is unnecessary since
pm_trace has disabled the async suspend.

And in order to allow the users to adjust rtc after resume, we can clear the pm_trace_rtc_abused directly and warn users of the broken rtc. (Without save/restore).

Quoted 24 lines
 drivers/base/power/trace.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
drivers/base/power/trace.c +5 -0
index aa9109a..1e6c611 100644
@@ -10,6 +10,7 @@
 #include <linux/pm-trace.h>
 #include <linux/export.h>
 #include <linux/rtc.h>
+#include <linux/suspend.h>
 
 #include <linux/mc146818rtc.h>
 
@@ -76,6 +77,11 @@
 
 bool pm_trace_rtc_abused __read_mostly;
 static unsigned int dev_hash_value;
+struct rtc_time_save {
+	struct rtc_time time;
+	atomic_t saved;
+};
 
author
#2 lore raw

Re: [PATCH 1/2][RFC v6] timekeeping: Ignore the bogus sleep time if pm_trace is enabled

In reply to Chen Yu
Quoted 12 lines · Chen Yu

From: Thomas Gleixner <[email protected]>

Previously we encountered some memory overflow issues due to the bogus sleep time brought by inconsistent rtc, which is triggered when pm_trace is enabled, and we have fixed it in recent kernel. However it's improper in the first place to call __timekeeping_inject_sleeptime() in case that pm_trace is enabled simply because that "hash" time value will wreckage the timekeeping subsystem.

This patch is to bypass the bogus rtc interval when pm_trace is enabled.

Quoted 8 lines
Cc Thomas Gleixner <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Len Brown <[email protected]>
Cc John Stultz <[email protected]>
Cc Xunlei Pang <[email protected]>

---

[cut]
Will resend another version. Thanks.

Signature · 8 lines
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.