Skip to contentSkip to conversation

PM: Add pm_trace switch

PATCH In review Rafael J. Wysocki 3 participants 7 messages Aug 29, 2006 – Aug 30, 2006

Subsystems (2)
Suspend to RAM Hibernation
lore
Share & tools
lore

Rafael J. Wysocki

author
#0 lore raw

PATCH 1/1 PM: Add pm_trace switch

Add the pm_trace attribute in /sys/power which has to be explicitly set to one to really enable the "PM tracing" code compiled in when CONFIG_PM_TRACE is set (which modifies the machine's CMOS clock in unpredictable ways).

Signed-off-by Rafael J. Wysocki <[email protected]>
Acked-by Pavel Machek <[email protected]>
---
 include/linux/resume-trace.h |   24 ++++++++++++++----------
 kernel/power/main.c          |   30 ++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 10 deletions(-)
include/linux/resume-trace.h +14 -10
Index: linux-2.6.18-rc4-mm1/include/linux/resume-trace.h
===================================================================
@@ -3,21 +3,25 @@
 
 #ifdef CONFIG_PM_TRACE
 
+extern int pm_trace_enabled;
+
 struct device;
 extern void set_trace_device(struct device *);
 extern void generate_resume_trace(void *tracedata, unsigned int user);
 
 #define TRACE_DEVICE(dev) set_trace_device(dev)
-#define TRACE_RESUME(user) do {				\
-	void *tracedata;				\
-	asm volatile("movl $1f,%0\n"			\
-		".section .tracedata,\"a\"\n"		\
-		"1:\t.word %c1\n"			\
-		"\t.long %c2\n"				\
-		".previous"				\
-		:"=r" (tracedata)			\
-		: "i" (__LINE__), "i" (__FILE__));	\
-	generate_resume_trace(tracedata, user);		\
+#define TRACE_RESUME(user) do {					\
+	if (pm_trace_enabled) {					\
+		void *tracedata;				\
+		asm volatile("movl $1f,%0\n"			\
+			".section .tracedata,\"a\"\n"		\
+			"1:\t.word %c1\n"			\
+			"\t.long %c2\n"				\
+			".previous"				\
+			:"=r" (tracedata)			\
+			: "i" (__LINE__), "i" (__FILE__));	\
+		generate_resume_trace(tracedata, user);		\
+	}							\
 } while (0)
 
 #else
kernel/power/main.c +30 -0
Index: linux-2.6.18-rc4-mm1/kernel/power/main.c
===================================================================
@@ -17,6 +17,7 @@
 #include <linux/pm.h>
 #include <linux/console.h>
 #include <linux/cpu.h>
+#include <linux/resume-trace.h>
 
 #include "power.h"
 

@@ -285,10 +286,39 @@ static ssize_t state_store(struct subsys
 
 power_attr(state);
 
+#ifdef CONFIG_PM_TRACE
+int pm_trace_enabled;
+
+static ssize_t pm_trace_show(struct subsystem * subsys, char * buf)
+{
+	return sprintf(buf, "%d\n", pm_trace_enabled);
+}
+
+static ssize_t
+pm_trace_store(struct subsystem * subsys, const char * buf, size_t n)
+{
+	int val;
+
+	if (sscanf(buf, "%d", &val) == 1) {
+		pm_trace_enabled = !!val;
+		return n;
+	}
+	return -EINVAL;
+}
+
+power_attr(pm_trace);
+
+static struct attribute * g[] = {
+	&state_attr.attr,
+	&pm_trace_attr.attr,
+	NULL,
+};
+#else
 static struct attribute * g[] = {
 	&state_attr.attr,
 	NULL,
 };
+#endif /* CONFIG_PM_TRACE */
 
 static struct attribute_group attr_group = {
 	.attrs = g,
#1 lore raw
Quoted 10 lines · On Tue, 29 Aug 2006 13:09:57 +0200 "Rafael J. Wysocki" <[email protected]> wrote:
kernel/power/main.c +10 -0
+int pm_trace_enabled;
+
+static ssize_t pm_trace_show(struct subsystem * subsys, char * buf)
+{
+	return sprintf(buf, "%d\n", pm_trace_enabled);
+}
+
+static ssize_t
+pm_trace_store(struct subsystem * subsys, const char * buf, size_t n)
+{

kernel/power/main.c static ssize_t state_store(struct subsys · line 309

Quoted 10 lines
kernel/power/main.c +10 -0
+	int val;
+
+	if (sscanf(buf, "%d", &val) == 1) {
+		pm_trace_enabled = !!val;
+		return n;
+	}
+	return -EINVAL;
+}
+
+power_attr(pm_trace);

<grumbles about documentation>

author
#2 lore raw
Quoted 17 lines · Andrew Morton
Quoted 14 lines · On Tue, 29 Aug 2006 13:09:57 +0200 "Rafael J. Wysocki" <[email protected]> wrote:
(quoted diff) +14 -0
+int pm_trace_enabled;
+
+static ssize_t pm_trace_show(struct subsystem * subsys, char * buf)
+{
+	return sprintf(buf, "%d\n", pm_trace_enabled);
+}
+
+static ssize_t
+pm_trace_store(struct subsystem * subsys, const char * buf, size_t n)
+{
+	int val;
+
+	if (sscanf(buf, "%d", &val) == 1) {
+		pm_trace_enabled = !!val;
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+		return n;
+	}
+	return -EINVAL;
+}
+
+power_attr(pm_trace);

<grumbles about documentation>

Well, this is the most difficult part. ;-)

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 Documentation/power/interface.txt |   15 +++++++++++++++
 1 file changed, 15 insertions(+)
Documentation/power/interface.txt +15 -0
Index: linux-2.6.18-rc4-mm3/Documentation/power/interface.txt
===================================================================
@@ -52,3 +52,18 @@ suspend image will be as small as possib
 
 Reading from this file will display the current image size limit, which
 is set to 500 MB by default.
+
+/sys/power/pm_trace controls the code which saves the last PM event point in
+the RTC across reboots, so that you can debug a machine that just hangs
+during suspend (or more commonly, during resume).  Namely, the RTC is only
+used to save the last PM event point if this file contains '1'.  Initially it
+contains '0' which may be changed to '1' by writing a string representing a
+nonzero integer into it.
+
+To use this debugging feature you should attempt to suspend the machine, then
+reboot it and run
+
+	dmesg -s 1000000 | grep 'hash matches'
+
+CAUTION: Using it will cause your machine's real-time (CMOS) clock to be
+set to a random invalid time after a resume.
#3 lore raw
Quoted 27 lines · Rafael J. Wysocki
Quoted 17 lines · Andrew Morton
Quoted 14 lines · On Tue, 29 Aug 2006 13:09:57 +0200 "Rafael J. Wysocki" <[email protected]> wrote:
(quoted diff) +14 -0
+int pm_trace_enabled;
+
+static ssize_t pm_trace_show(struct subsystem * subsys, char * buf)
+{
+	return sprintf(buf, "%d\n", pm_trace_enabled);
+}
+
+static ssize_t
+pm_trace_store(struct subsystem * subsys, const char * buf, size_t n)
+{
+	int val;
+
+	if (sscanf(buf, "%d", &val) == 1) {
+		pm_trace_enabled = !!val;
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+		return n;
+	}
+	return -EINVAL;
+}
+
+power_attr(pm_trace);

<grumbles about documentation>

Quoted 5 lines

Well, this is the most difficult part. ;-)

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 Documentation/power/interface.txt |   15 +++++++++++++++

Please update Documentation/ABI/ too.

thanks,

greg k-h

author
#4 lore raw
Quoted 28 lines · Greg KH
Quoted 27 lines · Rafael J. Wysocki
Quoted 25 lines · Andrew Morton
Quoted 10 lines · On Tue, 29 Aug 2006 13:09:57 +0200 "Rafael J. Wysocki" <[email protected]> wrote:
(quoted diff) +10 -0
+int pm_trace_enabled;
+
+static ssize_t pm_trace_show(struct subsystem * subsys, char * buf)
+{
+	return sprintf(buf, "%d\n", pm_trace_enabled);
+}
+
+static ssize_t
+pm_trace_store(struct subsystem * subsys, const char * buf, size_t n)
+{

Review

Quoted 10 lines
(quoted diff) +10 -0
+	int val;
+
+	if (sscanf(buf, "%d", &val) == 1) {
+		pm_trace_enabled = !!val;
+		return n;
+	}
+	return -EINVAL;
+}
+
+power_attr(pm_trace);

<grumbles about documentation>

Quoted 7 lines
Quoted 5 lines

Well, this is the most difficult part. ;-)

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 Documentation/power/interface.txt |   15 +++++++++++++++

Please update Documentation/ABI/ too.

I could, but there's nothing related to /sys/power/ in Documentation/ABI/ right now.

Do you mean I should create a file for /sys/power/ in there? If so, what should I put in there? I guess pretty much the same as in Documentation/power/interface.txt ...

Greetings,
Rafael

Signature · 2 lines
You never change things by fighting the existing reality.
		R. Buckminster Fuller
#5 lore raw
Quoted 35 lines · Rafael J. Wysocki
Quoted 34 lines · Greg KH
Quoted 32 lines · Rafael J. Wysocki
Quoted 17 lines · Andrew Morton
Quoted 14 lines · On Tue, 29 Aug 2006 13:09:57 +0200 "Rafael J. Wysocki" <[email protected]> wrote:
(quoted diff) +14 -0
+int pm_trace_enabled;
+
+static ssize_t pm_trace_show(struct subsystem * subsys, char * buf)
+{
+	return sprintf(buf, "%d\n", pm_trace_enabled);
+}
+
+static ssize_t
+pm_trace_store(struct subsystem * subsys, const char * buf, size_t n)
+{
+	int val;
+
+	if (sscanf(buf, "%d", &val) == 1) {
+		pm_trace_enabled = !!val;
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+		return n;
+	}
+	return -EINVAL;
+}
+
+power_attr(pm_trace);

<grumbles about documentation>

Well, this is the most difficult part. ;-)

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 Documentation/power/interface.txt |   15 +++++++++++++++
Quoted 8 lines
Quoted 1 line

Please update Documentation/ABI/ too.

I could, but there's nothing related to /sys/power/ in Documentation/ABI/ right now.

Do you mean I should create a file for /sys/power/ in there? If so, what should I put in there? I guess pretty much the same as in Documentation/power/interface.txt ...

Exactly :)

thanks,

greg k-h

author
#6 lore raw
Quoted 38 lines · Greg KH
Quoted 37 lines · Rafael J. Wysocki
Quoted 35 lines · Greg KH
Quoted 27 lines · Rafael J. Wysocki
Quoted 17 lines · Andrew Morton
Quoted 14 lines · On Tue, 29 Aug 2006 13:09:57 +0200 "Rafael J. Wysocki" <[email protected]> wrote:
(quoted diff) +14 -0
+int pm_trace_enabled;
+
+static ssize_t pm_trace_show(struct subsystem * subsys, char * buf)
+{
+	return sprintf(buf, "%d\n", pm_trace_enabled);
+}
+
+static ssize_t
+pm_trace_store(struct subsystem * subsys, const char * buf, size_t n)
+{
+	int val;
+
+	if (sscanf(buf, "%d", &val) == 1) {
+		pm_trace_enabled = !!val;
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+		return n;
+	}
+	return -EINVAL;
+}
+
+power_attr(pm_trace);

<grumbles about documentation>

Quoted 5 lines

Well, this is the most difficult part. ;-)

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 Documentation/power/interface.txt |   15 +++++++++++++++

Please update Documentation/ABI/ too.

Quoted 8 lines
Quoted 6 lines

I could, but there's nothing related to /sys/power/ in Documentation/ABI/ right now.

Do you mean I should create a file for /sys/power/ in there? If so, what should I put in there? I guess pretty much the same as in Documentation/power/interface.txt ...

Exactly :)

Okay, but I think that should be done in a separate patch.

Greetings,
Rafael

Signature · 2 lines
You never change things by fighting the existing reality.
		R. Buckminster Fuller