Skip to contentSkip to conversation

PM-Trace: add pm-trace support for suspending phase

PATCH Fu, Zhonghui 2 participants 2 messages Mar 9, 2015 – Mar 18, 2015 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

Any comments about this patch?

Thanks,
Zhonghui

Quoted 225 lines · Zhonghui

Occasionally, the system can't come back up after suspend/resume due to problems of device suspending phase. This patch make PM_TRACE infrastructure cover device suspending phase of suspend/resume process, and the information in RTC can tell developers which device suspending function make system hang.

---
Changes in v3:
- Remove duplicate macro definition in arch/x86/include/asm/pm-trace.h.

 .../x86/include/asm/{resume-trace.h => pm-trace.h} |   10 ++++++----
 drivers/base/power/main.c                          |   20 ++++++++++++++++----
 drivers/base/power/trace.c                         |    6 +++---
 include/linux/{resume-trace.h => pm-trace.h}       |    9 +++++----
 kernel/power/main.c                                |    2 +-
 5 files changed, 31 insertions(+), 16 deletions(-)
 rename arch/x86/include/asm/{resume-trace.h => pm-trace.h} (68%)
 rename include/linux/{resume-trace.h => pm-trace.h} (75%)
arch/x86/include/asm/pm-trace.h renamed from arch/x86/include/asm/resume-trace.h +6 -4
similarity index 68%
rename from arch/x86/include/asm/resume-trace.h
rename to arch/x86/include/asm/pm-trace.h
index 3ff1c2c..7b7ac42 100644
@@ -1,5 +1,5 @@
-#ifndef _ASM_X86_RESUME_TRACE_H
-#define _ASM_X86_RESUME_TRACE_H
+#ifndef _ASM_X86_PM_TRACE_H
+#define _ASM_X86_PM_TRACE_H
 
 #include <asm/asm.h>
 
@@ -14,8 +14,10 @@ do {								\
 			     ".previous"			\
 			     :"=r" (tracedata)			\
 			     : "i" (__LINE__), "i" (__FILE__));	\
-		generate_resume_trace(tracedata, user);		\
+		generate_pm_trace(tracedata, user);		\
 	}							\
 } while (0)
 
-#endif /* _ASM_X86_RESUME_TRACE_H */
+#define TRACE_SUSPEND(user)	TRACE_RESUME(user)
+
+#endif /* _ASM_X86_PM_TRACE_H */
drivers/base/power/main.c +16 -4
index 9717d5f..3d874ec 100644
@@ -23,7 +23,7 @@
 #include <linux/mutex.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
-#include <linux/resume-trace.h>
+#include <linux/pm-trace.h>
 #include <linux/interrupt.h>
 #include <linux/sched.h>
 #include <linux/async.h>
@@ -1017,6 +1017,9 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 	char *info = NULL;
 	int error = 0;
 
+	TRACE_DEVICE(dev);
+	TRACE_SUSPEND(0);
+
 	if (async_error)
 		goto Complete;
 
@@ -1057,6 +1060,7 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 
 Complete:
 	complete_all(&dev->power.completion);
+	TRACE_SUSPEND(error);
 	return error;
 }
 
@@ -1078,7 +1082,7 @@ static int device_suspend_noirq(struct device *dev)
 {
 	reinit_completion(&dev->power.completion);
 
-	if (pm_async_enabled && dev->power.async_suspend) {
+	if (is_async(dev)) {
 		get_device(dev);
 		async_schedule(async_suspend_noirq, dev);
 		return 0;
@@ -1157,6 +1161,9 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
 	char *info = NULL;
 	int error = 0;
 
+	TRACE_DEVICE(dev);
+	TRACE_SUSPEND(0);
+
 	__pm_runtime_disable(dev, false);
 
 	if (async_error)
@@ -1198,6 +1205,7 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
 		async_error = error;
 
 Complete:
+	TRACE_SUSPEND(error);
 	complete_all(&dev->power.completion);
 	return error;
 }
@@ -1219,7 +1227,7 @@ static int device_suspend_late(struct device *dev)
 {
 	reinit_completion(&dev->power.completion);
 
-	if (pm_async_enabled && dev->power.async_suspend) {
+	if (is_async(dev)) {
 		get_device(dev);
 		async_schedule(async_suspend_late, dev);
 		return 0;
@@ -1338,6 +1346,9 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async)
 	int error = 0;
 	DECLARE_DPM_WATCHDOG_ON_STACK(wd);
 
+	TRACE_DEVICE(dev);
+	TRACE_SUSPEND(0);
+
 	dpm_wait_for_children(dev, async);
 
 	if (async_error)
@@ -1444,6 +1455,7 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async)
 	if (error)
 		async_error = error;
 
+	TRACE_SUSPEND(error);
 	return error;
 }
 
@@ -1465,7 +1477,7 @@ static int device_suspend(struct device *dev)
 {
 	reinit_completion(&dev->power.completion);
 
-	if (pm_async_enabled && dev->power.async_suspend) {
+	if (is_async(dev)) {
 		get_device(dev);
 		async_schedule(async_suspend, dev);
 		return 0;
drivers/base/power/trace.c +3 -3
index d94a1f5..a311cfa 100644
@@ -7,7 +7,7 @@
  * devices may be working.
  */
 
-#include <linux/resume-trace.h>
+#include <linux/pm-trace.h>
 #include <linux/export.h>
 #include <linux/rtc.h>
 
@@ -154,7 +154,7 @@ EXPORT_SYMBOL(set_trace_device);
  * it's not any guarantee, but it's a high _likelihood_ that
  * the match is valid).
  */
-void generate_resume_trace(const void *tracedata, unsigned int user)
+void generate_pm_trace(const void *tracedata, unsigned int user)
 {
 	unsigned short lineno = *(unsigned short *)tracedata;
 	const char *file = *(const char **)(tracedata + 2);
@@ -164,7 +164,7 @@ void generate_resume_trace(const void *tracedata, unsigned int user)
 	file_hash_value = hash_string(lineno, file, FILEHASH);
 	set_magic_time(user_hash_value, file_hash_value, dev_hash_value);
 }
-EXPORT_SYMBOL(generate_resume_trace);
+EXPORT_SYMBOL(generate_pm_trace);
 
 extern char __tracedata_start, __tracedata_end;
 static int show_file_hash(unsigned int value)
include/linux/pm-trace.h renamed from include/linux/resume-trace.h +5 -4
similarity index 75%
rename from include/linux/resume-trace.h
rename to include/linux/pm-trace.h
index f31db23..ecbde7a 100644
@@ -1,8 +1,8 @@
-#ifndef RESUME_TRACE_H
-#define RESUME_TRACE_H
+#ifndef PM_TRACE_H
+#define PM_TRACE_H
 
 #ifdef CONFIG_PM_TRACE
-#include <asm/resume-trace.h>
+#include <asm/pm-trace.h>
 #include <linux/types.h>
 
 extern int pm_trace_enabled;
@@ -14,7 +14,7 @@ static inline int pm_trace_is_enabled(void)
 
 struct device;
 extern void set_trace_device(struct device *);
-extern void generate_resume_trace(const void *tracedata, unsigned int user);
+extern void generate_pm_trace(const void *tracedata, unsigned int user);
 extern int show_trace_dev_match(char *buf, size_t size);
 
 #define TRACE_DEVICE(dev) do { \
@@ -28,6 +28,7 @@ static inline int pm_trace_is_enabled(void) { return 0; }
 
 #define TRACE_DEVICE(dev) do { } while (0)
 #define TRACE_RESUME(dev) do { } while (0)
+#define TRACE_SUSPEND(dev) do { } while (0)
 
 #endif
 
kernel/power/main.c +1 -2
index 9a59d04..86e8157 100644
@@ -11,7 +11,7 @@
 #include <linux/export.h>
 #include <linux/kobject.h>
 #include <linux/string.h>
-#include <linux/resume-trace.h>
+#include <linux/pm-trace.h>
 #include <linux/workqueue.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
-- 1.7.1
#2 lore raw
In reply to a message not in the archive
Quoted 7 lines · Zhonghui

Occasionally, the system can't come back up after suspend/resume due to problems of device suspending phase. This patch make PM_TRACE infrastructure cover device suspending phase of suspend/resume process, and the information in RTC can tell developers which device suspending function make system hang.

OK, queued up for 4.1.

Next time please send patches as plain text rather than as multipart MIME.

Quoted 217 lines

---
Changes in v3:
- Remove duplicate macro definition in arch/x86/include/asm/pm-trace.h.

 .../x86/include/asm/{resume-trace.h => pm-trace.h} |   10 ++++++----
 drivers/base/power/main.c                          |   20 ++++++++++++++++----
 drivers/base/power/trace.c                         |    6 +++---
 include/linux/{resume-trace.h => pm-trace.h}       |    9 +++++----
 kernel/power/main.c                                |    2 +-
 5 files changed, 31 insertions(+), 16 deletions(-)
 rename arch/x86/include/asm/{resume-trace.h => pm-trace.h} (68%)
 rename include/linux/{resume-trace.h => pm-trace.h} (75%)
arch/x86/include/asm/pm-trace.h renamed from arch/x86/include/asm/resume-trace.h +6 -4
similarity index 68%
rename from arch/x86/include/asm/resume-trace.h
rename to arch/x86/include/asm/pm-trace.h
index 3ff1c2c..7b7ac42 100644
@@ -1,5 +1,5 @@
-#ifndef _ASM_X86_RESUME_TRACE_H
-#define _ASM_X86_RESUME_TRACE_H
+#ifndef _ASM_X86_PM_TRACE_H
+#define _ASM_X86_PM_TRACE_H
 
 #include <asm/asm.h>
 
@@ -14,8 +14,10 @@ do {								\
 			     ".previous"			\
 			     :"=r" (tracedata)			\
 			     : "i" (__LINE__), "i" (__FILE__));	\
-		generate_resume_trace(tracedata, user);		\
+		generate_pm_trace(tracedata, user);		\
 	}							\
 } while (0)
 
-#endif /* _ASM_X86_RESUME_TRACE_H */
+#define TRACE_SUSPEND(user)	TRACE_RESUME(user)
+
+#endif /* _ASM_X86_PM_TRACE_H */
drivers/base/power/main.c +16 -4
index 9717d5f..3d874ec 100644
@@ -23,7 +23,7 @@
 #include <linux/mutex.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
-#include <linux/resume-trace.h>
+#include <linux/pm-trace.h>
 #include <linux/interrupt.h>
 #include <linux/sched.h>
 #include <linux/async.h>
@@ -1017,6 +1017,9 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 	char *info = NULL;
 	int error = 0;
 
+	TRACE_DEVICE(dev);
+	TRACE_SUSPEND(0);
+
 	if (async_error)
 		goto Complete;
 
@@ -1057,6 +1060,7 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
 
 Complete:
 	complete_all(&dev->power.completion);
+	TRACE_SUSPEND(error);
 	return error;
 }
 
@@ -1078,7 +1082,7 @@ static int device_suspend_noirq(struct device *dev)
 {
 	reinit_completion(&dev->power.completion);
 
-	if (pm_async_enabled && dev->power.async_suspend) {
+	if (is_async(dev)) {
 		get_device(dev);
 		async_schedule(async_suspend_noirq, dev);
 		return 0;
@@ -1157,6 +1161,9 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
 	char *info = NULL;
 	int error = 0;
 
+	TRACE_DEVICE(dev);
+	TRACE_SUSPEND(0);
+
 	__pm_runtime_disable(dev, false);
 
 	if (async_error)
@@ -1198,6 +1205,7 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
 		async_error = error;
 
 Complete:
+	TRACE_SUSPEND(error);
 	complete_all(&dev->power.completion);
 	return error;
 }
@@ -1219,7 +1227,7 @@ static int device_suspend_late(struct device *dev)
 {
 	reinit_completion(&dev->power.completion);
 
-	if (pm_async_enabled && dev->power.async_suspend) {
+	if (is_async(dev)) {
 		get_device(dev);
 		async_schedule(async_suspend_late, dev);
 		return 0;
@@ -1338,6 +1346,9 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async)
 	int error = 0;
 	DECLARE_DPM_WATCHDOG_ON_STACK(wd);
 
+	TRACE_DEVICE(dev);
+	TRACE_SUSPEND(0);
+
 	dpm_wait_for_children(dev, async);
 
 	if (async_error)
@@ -1444,6 +1455,7 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async)
 	if (error)
 		async_error = error;
 
+	TRACE_SUSPEND(error);
 	return error;
 }
 
@@ -1465,7 +1477,7 @@ static int device_suspend(struct device *dev)
 {
 	reinit_completion(&dev->power.completion);
 
-	if (pm_async_enabled && dev->power.async_suspend) {
+	if (is_async(dev)) {
 		get_device(dev);
 		async_schedule(async_suspend, dev);
 		return 0;
drivers/base/power/trace.c +3 -3
index d94a1f5..a311cfa 100644
@@ -7,7 +7,7 @@
  * devices may be working.
  */
 
-#include <linux/resume-trace.h>
+#include <linux/pm-trace.h>
 #include <linux/export.h>
 #include <linux/rtc.h>
 
@@ -154,7 +154,7 @@ EXPORT_SYMBOL(set_trace_device);
  * it's not any guarantee, but it's a high _likelihood_ that
  * the match is valid).
  */
-void generate_resume_trace(const void *tracedata, unsigned int user)
+void generate_pm_trace(const void *tracedata, unsigned int user)
 {
 	unsigned short lineno = *(unsigned short *)tracedata;
 	const char *file = *(const char **)(tracedata + 2);
@@ -164,7 +164,7 @@ void generate_resume_trace(const void *tracedata, unsigned int user)
 	file_hash_value = hash_string(lineno, file, FILEHASH);
 	set_magic_time(user_hash_value, file_hash_value, dev_hash_value);
 }
-EXPORT_SYMBOL(generate_resume_trace);
+EXPORT_SYMBOL(generate_pm_trace);
 
 extern char __tracedata_start, __tracedata_end;
 static int show_file_hash(unsigned int value)
include/linux/pm-trace.h renamed from include/linux/resume-trace.h +5 -4
similarity index 75%
rename from include/linux/resume-trace.h
rename to include/linux/pm-trace.h
index f31db23..ecbde7a 100644
@@ -1,8 +1,8 @@
-#ifndef RESUME_TRACE_H
-#define RESUME_TRACE_H
+#ifndef PM_TRACE_H
+#define PM_TRACE_H
 
 #ifdef CONFIG_PM_TRACE
-#include <asm/resume-trace.h>
+#include <asm/pm-trace.h>
 #include <linux/types.h>
 
 extern int pm_trace_enabled;
@@ -14,7 +14,7 @@ static inline int pm_trace_is_enabled(void)
 
 struct device;
 extern void set_trace_device(struct device *);
-extern void generate_resume_trace(const void *tracedata, unsigned int user);
+extern void generate_pm_trace(const void *tracedata, unsigned int user);
 extern int show_trace_dev_match(char *buf, size_t size);
 
 #define TRACE_DEVICE(dev) do { \
@@ -28,6 +28,7 @@ static inline int pm_trace_is_enabled(void) { return 0; }
 
 #define TRACE_DEVICE(dev) do { } while (0)
 #define TRACE_RESUME(dev) do { } while (0)
+#define TRACE_SUSPEND(dev) do { } while (0)
 
 #endif
 
kernel/power/main.c +1 -2
index 9a59d04..86e8157 100644
@@ -11,7 +11,7 @@
 #include <linux/export.h>
 #include <linux/kobject.h>
 #include <linux/string.h>
-#include <linux/resume-trace.h>
+#include <linux/pm-trace.h>
 #include <linux/workqueue.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
-- 1.7.1
Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.