Skip to contentSkip to conversation

PM: Use suspend_console in swsusp and make it configureable

RFC PATCH In review Rafael J. Wysocki 7 participants 16 messages Aug 15, 2006 – Sep 4, 2006

Subsystems (3)
Suspend to RAM Hibernation Console
lore
Share & tools
lore

Rafael J. Wysocki

author
#0 lore raw

RFC PATCH 1/1 PM: Use suspend_console in swsusp and make it configureable

Hi,

The appended patch does the following:

1) Adds suspend_console() and resume_console() to the suspend-to-disk code paths so that people using netconsole are safe with swsusp.

2) Adds a Kconfig option allowing us to disable suspend_/resume_console() if need be.

3) Marks CONFIG_PM_TRACE as dangerous.

Comments welcome.

Greetings,
Rafael

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 include/linux/console.h |    5 +++++
 kernel/power/Kconfig    |   13 ++++++++++++-
 kernel/power/disk.c     |    8 ++++++++
 kernel/power/user.c     |    8 +++++++-
 kernel/printk.c         |    2 ++
 5 files changed, 34 insertions(+), 2 deletions(-)
kernel/power/disk.c +8 -0
Index: linux-2.6.18-rc4-mm1/kernel/power/disk.c
===================================================================
@@ -18,6 +18,7 @@
 #include <linux/fs.h>
 #include <linux/mount.h>
 #include <linux/pm.h>
+#include <linux/console.h>
 #include <linux/cpu.h>
 
 #include "power.h"

@@ -119,8 +120,10 @@ int pm_suspend_disk(void)
 	if (error)
 		return error;
 
+	suspend_console();
 	error = device_suspend(PMSG_FREEZE);
 	if (error) {
+		resume_console();
 		printk("Some devices failed to suspend\n");
 		unprepare_processes();
 		return error;

@@ -133,6 +136,7 @@ int pm_suspend_disk(void)
 
 	if (in_suspend) {
 		device_resume();
+		resume_console();
 		pr_debug("PM: writing image.\n");
 		error = swsusp_write();
 		if (!error)

@@ -148,6 +152,7 @@ int pm_suspend_disk(void)
 	swsusp_free();
  Done:
 	device_resume();
+	resume_console();
 	unprepare_processes();
 	return error;
 }

@@ -212,7 +217,9 @@ static int software_resume(void)
 
 	pr_debug("PM: Preparing devices for restore.\n");
 
+	suspend_console();
 	if ((error = device_suspend(PMSG_PRETHAW))) {
+		resume_console();
 		printk("Some devices failed to suspend\n");
 		swsusp_free();
 		goto Thaw;

@@ -224,6 +231,7 @@ static int software_resume(void)
 	swsusp_resume();
 	pr_debug("PM: Restore failed, recovering.n");
 	device_resume();
+	resume_console();
  Thaw:
 	unprepare_processes();
  Done:
kernel/power/user.c +7 -1
Index: linux-2.6.18-rc4-mm1/kernel/power/user.c
===================================================================
@@ -19,6 +19,7 @@
 #include <linux/swapops.h>
 #include <linux/pm.h>
 #include <linux/fs.h>
+#include <linux/console.h>
 #include <linux/cpu.h>
 
 #include <asm/uaccess.h>

@@ -173,12 +174,14 @@ static int snapshot_ioctl(struct inode *
 		/* Free memory before shutting down devices. */
 		error = swsusp_shrink_memory();
 		if (!error) {
+			suspend_console();
 			error = device_suspend(PMSG_FREEZE);
 			if (!error) {
 				in_suspend = 1;
 				error = swsusp_suspend();
 				device_resume();
 			}
+			resume_console();
 		}
 		up(&pm_sem);
 		if (!error)

@@ -196,11 +199,13 @@ static int snapshot_ioctl(struct inode *
 		}
 		down(&pm_sem);
 		pm_prepare_console();
+		suspend_console();
 		error = device_suspend(PMSG_PRETHAW);
 		if (!error) {
 			error = swsusp_resume();
 			device_resume();
 		}
+		resume_console();
 		pm_restore_console();
 		up(&pm_sem);
 		break;

@@ -289,6 +294,7 @@ static int snapshot_ioctl(struct inode *
 		}
 
 		/* Put devices to sleep */
+		suspend_console();
 		error = device_suspend(PMSG_SUSPEND);
 		if (error) {
 			printk(KERN_ERR "Failed to suspend some devices.\n");

@@ -299,7 +305,7 @@ static int snapshot_ioctl(struct inode *
 			/* Wake up devices */
 			device_resume();
 		}
-
+		resume_console();
 		if (pm_ops->finish)
 			pm_ops->finish(PM_SUSPEND_MEM);
include/linux/console.h +5 -0
Index: linux-2.6.18-rc4-mm1/include/linux/console.h
===================================================================
@@ -120,9 +120,14 @@ extern void console_stop(struct console 
 extern void console_start(struct console *);
 extern int is_console_locked(void);
 
+#ifndef CONFIG_PM_DISABLE_CONSOLE_SUSPEND
 /* Suspend and resume console messages over PM events */
 extern void suspend_console(void);
 extern void resume_console(void);
+#else
+static inline void suspend_console(void) {}
+static inline void resume_console(void) {}
+#endif /* CONFIG_PM_DISABLE_CONSOLE_SUSPEND */
 
 /* Some debug stub to catch some of the obvious races in the VT code */
 #if 1
kernel/power/Kconfig +12 -1
Index: linux-2.6.18-rc4-mm1/kernel/power/Kconfig
===================================================================
@@ -36,8 +36,19 @@ config PM_DEBUG
 	code. This is helpful when debugging and reporting various PM bugs, 
 	like suspend support.
 
+config PM_DISABLE_CONSOLE_SUSPEND
+	bool "Keep console(s) enabled during suspend/resume (DANGEROUS)"
+	depends on PM && PM_DEBUG
+	default n
+	---help---
+	This option turns off the console suspend mechanism that prevents
+	debug messages from reaching the console during the suspend/resume
+	operations.  This may be helpful when debugging device drivers'
+	suspend/resume routines, but may itself lead to problems, for example
+	if netconsole is used.
+
 config PM_TRACE
-	bool "Suspend/resume event tracing"
+	bool "Suspend/resume event tracing (DANGEROUS)"
 	depends on PM && PM_DEBUG && X86_32 && EXPERIMENTAL
 	default n
 	---help---
kernel/printk.c +2 -0
Index: linux-2.6.18-rc4-mm1/kernel/printk.c
===================================================================
@@ -702,6 +702,7 @@ int __init add_preferred_console(char *n
 	return 0;
 }
 
+#ifndef CONFIG_PM_DISABLE_CONSOLE_SUSPEND
 /**
  * suspend_console - suspend the console subsystem
  *

@@ -718,6 +719,7 @@ void resume_console(void)
 	console_suspended = 0;
 	release_console_sem();
 }
+#endif /* CONFIG_PM_DISABLE_CONSOLE_SUSPEND */
 
 /**
  * acquire_console_sem - lock the console system for exclusive use.
#1 lore raw

Hi!

Quoted 7 lines

The appended patch does the following:

1) Adds suspend_console() and resume_console() to the suspend-to-disk code paths so that people using netconsole are safe with swsusp.

2) Adds a Kconfig option allowing us to disable suspend_/resume_console() if need be.

Slightly ugly, but I guess that is the way to go.

Quoted 1 line

3) Marks CONFIG_PM_TRACE as dangerous.

I do not think that is enough. "(WILL TRASH YOUR CMOS)" would be more suitable. Dangerous is "may cause problems to you". This is "will cause problems to you". And for this to be useful, people have to edit sources, anyway.

Pavel

Can we just delete the config option?

Quoted 10 lines

Index: linux-2.6.18-rc4-mm1/kernel/power/Kconfig
===================================================================

kernel/power/Kconfig +1 -1
 config PM_TRACE
-	bool "Suspend/resume event tracing"
+	bool "Suspend/resume event tracing (DANGEROUS)"
 	depends on PM && PM_DEBUG && X86_32 && EXPERIMENTAL
 	default n
 	---help---
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#2 lore raw

Hi,

Quoted 6 lines · Pavel Machek

Hi!

Quoted 4 lines

The appended patch does the following:

1) Adds suspend_console() and resume_console() to the suspend-to-disk code paths so that people using netconsole are safe with swsusp.

So I assume this one is OK.

Quoted 4 lines
Quoted 2 lines

2) Adds a Kconfig option allowing us to disable suspend_/resume_console() if need be.

Slightly ugly, but I guess that is the way to go.

It also seems to be needed to add a 2 sec. dealy in suspend_console() so that eg. the network console can send the messages before we try to suspend the device.

Quoted 10 lines
Quoted 1 line

3) Marks CONFIG_PM_TRACE as dangerous.

I do not think that is enough. "(WILL TRASH YOUR CMOS)" would be more suitable. Dangerous is "may cause problems to you". This is "will cause problems to you". And for this to be useful, people have to edit sources, anyway.

Pavel

Can we just delete the config option?

OK, I'll do that.

I have divided the changes into three individual patches that will follow as replies to this message.

Rafael

Signature · 2 lines
You never change things by fighting the existing reality.
		R. Buckminster Fuller
author
#3 lore raw

RFC PATCH 1/3 swsusp: Use suspend_console

Add suspend_console() and resume_console() to the suspend-to-disk code paths so that the users of netconsole can use swsusp with it.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 kernel/power/disk.c |    8 ++++++++
 kernel/power/user.c |    8 +++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)
kernel/power/disk.c +8 -0
Index: linux-2.6.18-rc4-mm1/kernel/power/disk.c
===================================================================
@@ -18,6 +18,7 @@
 #include <linux/fs.h>
 #include <linux/mount.h>
 #include <linux/pm.h>
+#include <linux/console.h>
 #include <linux/cpu.h>
 
 #include "power.h"

@@ -119,8 +120,10 @@ int pm_suspend_disk(void)
 	if (error)
 		return error;
 
+	suspend_console();
 	error = device_suspend(PMSG_FREEZE);
 	if (error) {
+		resume_console();
 		printk("Some devices failed to suspend\n");
 		unprepare_processes();
 		return error;

@@ -133,6 +136,7 @@ int pm_suspend_disk(void)
 
 	if (in_suspend) {
 		device_resume();
+		resume_console();
 		pr_debug("PM: writing image.\n");
 		error = swsusp_write();
 		if (!error)

@@ -148,6 +152,7 @@ int pm_suspend_disk(void)
 	swsusp_free();
  Done:
 	device_resume();
+	resume_console();
 	unprepare_processes();
 	return error;
 }

@@ -212,7 +217,9 @@ static int software_resume(void)
 
 	pr_debug("PM: Preparing devices for restore.\n");
 
+	suspend_console();
 	if ((error = device_suspend(PMSG_PRETHAW))) {
+		resume_console();
 		printk("Some devices failed to suspend\n");
 		swsusp_free();
 		goto Thaw;

@@ -224,6 +231,7 @@ static int software_resume(void)
 	swsusp_resume();
 	pr_debug("PM: Restore failed, recovering.n");
 	device_resume();
+	resume_console();
  Thaw:
 	unprepare_processes();
  Done:
kernel/power/user.c +7 -1
Index: linux-2.6.18-rc4-mm1/kernel/power/user.c
===================================================================
@@ -19,6 +19,7 @@
 #include <linux/swapops.h>
 #include <linux/pm.h>
 #include <linux/fs.h>
+#include <linux/console.h>
 #include <linux/cpu.h>
 
 #include <asm/uaccess.h>

@@ -173,12 +174,14 @@ static int snapshot_ioctl(struct inode *
 		/* Free memory before shutting down devices. */
 		error = swsusp_shrink_memory();
 		if (!error) {
+			suspend_console();
 			error = device_suspend(PMSG_FREEZE);
 			if (!error) {
 				in_suspend = 1;
 				error = swsusp_suspend();
 				device_resume();
 			}
+			resume_console();
 		}
 		up(&pm_sem);
 		if (!error)

@@ -196,11 +199,13 @@ static int snapshot_ioctl(struct inode *
 		}
 		down(&pm_sem);
 		pm_prepare_console();
+		suspend_console();
 		error = device_suspend(PMSG_PRETHAW);
 		if (!error) {
 			error = swsusp_resume();
 			device_resume();
 		}
+		resume_console();
 		pm_restore_console();
 		up(&pm_sem);
 		break;

@@ -289,6 +294,7 @@ static int snapshot_ioctl(struct inode *
 		}
 
 		/* Put devices to sleep */
+		suspend_console();
 		error = device_suspend(PMSG_SUSPEND);
 		if (error) {
 			printk(KERN_ERR "Failed to suspend some devices.\n");

@@ -299,7 +305,7 @@ static int snapshot_ioctl(struct inode *
 			/* Wake up devices */
 			device_resume();
 		}
-
+		resume_console();
 		if (pm_ops->finish)
 			pm_ops->finish(PM_SUSPEND_MEM);
author
#4 lore raw

RFC PATCH 2/3 PM: Make console suspending configureable

In reply to Rafael J. Wysocki

Change suspend_console() so that it waits for all consoles to flush the remaining messages and make it possible to switch the console suspending off with the help of a Kconfig option.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 include/linux/console.h |    5 +++++
 kernel/power/Kconfig    |   11 +++++++++++
 kernel/printk.c         |    8 ++++++++
 3 files changed, 24 insertions(+)
include/linux/console.h +5 -0
Index: linux-2.6.18-rc4-mm1/include/linux/console.h
===================================================================
@@ -120,9 +120,14 @@ extern void console_stop(struct console 
 extern void console_start(struct console *);
 extern int is_console_locked(void);
 
+#ifndef CONFIG_DISABLE_CONSOLE_SUSPEND
 /* Suspend and resume console messages over PM events */
 extern void suspend_console(void);
 extern void resume_console(void);
+#else
+static inline void suspend_console(void) {}
+static inline void resume_console(void) {}
+#endif /* CONFIG_DISABLE_CONSOLE_SUSPEND */
 
 /* Some debug stub to catch some of the obvious races in the VT code */
 #if 1
kernel/power/Kconfig +11 -0
Index: linux-2.6.18-rc4-mm1/kernel/power/Kconfig
===================================================================
@@ -36,6 +36,17 @@ config PM_DEBUG
 	code. This is helpful when debugging and reporting various PM bugs, 
 	like suspend support.
 
+config PM_DISABLE_CONSOLE_SUSPEND
+	bool "Keep console(s) enabled during suspend/resume (DANGEROUS)"
+	depends on PM && PM_DEBUG
+	default n
+	---help---
+	This option turns off the console suspend mechanism that prevents
+	debug messages from reaching the console during the suspend/resume
+	operations.  This may be helpful when debugging device drivers'
+	suspend/resume routines, but may itself lead to problems, for example
+	if netconsole is used.
+
 config PM_TRACE
 	bool "Suspend/resume event tracing"
 	depends on PM && PM_DEBUG && X86_32 && EXPERIMENTAL
kernel/printk.c +8 -0
Index: linux-2.6.18-rc4-mm1/kernel/printk.c
===================================================================
@@ -702,6 +702,7 @@ int __init add_preferred_console(char *n
 	return 0;
 }
 
+#ifndef CONFIG_DISABLE_CONSOLE_SUSPEND
 /**
  * suspend_console - suspend the console subsystem
  *

@@ -709,8 +710,14 @@ int __init add_preferred_console(char *n
  */
 void suspend_console(void)
 {
+	printk("Suspending console(s)\n");
 	acquire_console_sem();
 	console_suspended = 1;
+	/* This is needed so that all of the messages that have already been
+	 * written to all consoles can be actually transmitted (eg. over a
+	 * network) before we try to suspend the consoles' devices.
+	 */
+	ssleep(2);
 }
 
 void resume_console(void)

@@ -718,6 +725,7 @@ void resume_console(void)
 	console_suspended = 0;
 	release_console_sem();
 }
+#endif /* CONFIG_DISABLE_CONSOLE_SUSPEND */
 
 /**
  * acquire_console_sem - lock the console system for exclusive use.
#5 lore raw

Re: [RFC][PATCH 2/3] PM: Make console suspending configureable

Hi,

sorry, i am only slowly catching up after vacation.

Quoted 12 lines · Rafael J. Wysocki

Change suspend_console() so that it waits for all consoles to flush the remaining messages and make it possible to switch the console suspending off with the help of a Kconfig option.

Signed-off-by Rafael J. Wysocki <[email protected]>
kernel/printk.c +1 -0
+#ifndef CONFIG_DISABLE_CONSOLE_SUSPEND
 /**
  * suspend_console - suspend the console subsystem
  *

@@ -709,8 +710,14 @@ int __init add_preferred_console(char *n
  */

kernel/printk.c int __init add_preferred_console(char *n · line 720

Quoted 10 lines
kernel/printk.c +6 -0
 void suspend_console(void)
 {
+	printk("Suspending console(s)\n");
 	acquire_console_sem();
 	console_suspended = 1;
+	/* This is needed so that all of the messages that have already been
+	 * written to all consoles can be actually transmitted (eg. over a
+	 * network) before we try to suspend the consoles' devices.
+	 */
+	ssleep(2);

Sorry, but no. Suspend and resume is already slow enough, no need to make both of them much slower.
If we can condition this on the netconsole being used, ok, but not for the most common case of "console is on plain VGA".

Signature · 6 lines
Stefan Seyfried                  \ "I didn't want to write for pay. I
QA / R&D Team Mobile Devices      \ wanted to be paid for what I write."
SUSE LINUX Products GmbH, Nürnberg \                    -- Leonard Cohen

-- 
VGER BF report: U 0.49988
author
#6 lore raw

Re: [RFC][PATCH 2/3] PM: Make console suspending configureable

Quoted 24 lines · Stefan Seyfried

Hi,

sorry, i am only slowly catching up after vacation.

Quoted 19 lines · Rafael J. Wysocki

Change suspend_console() so that it waits for all consoles to flush the remaining messages and make it possible to switch the console suspending off with the help of a Kconfig option.

Signed-off-by Rafael J. Wysocki <[email protected]>
kernel/printk.c +4 -0
+#ifndef CONFIG_DISABLE_CONSOLE_SUSPEND
 /**
  * suspend_console - suspend the console subsystem
  *

@@ -709,8 +710,14 @@ int __init add_preferred_console(char *n
  */
 void suspend_console(void)
 {
+	printk("Suspending console(s)\n");
 	acquire_console_sem();
 	console_suspended = 1;
+	/* This is needed so that all of the messages that have already been
+	 * written to all consoles can be actually transmitted (eg. over a
Quoted 8 lines

kernel/printk.c int __init add_preferred_console(char *n · line 720

Quoted 3 lines
kernel/printk.c +3 -0
+	 * network) before we try to suspend the consoles' devices.
+	 */
+	ssleep(2);

Sorry, but no. Suspend and resume is already slow enough, no need to make both of them much slower.
If we can condition this on the netconsole being used, ok, but not for the most common case of "console is on plain VGA".

Hm, it already is in -mm, but of course I can prepare a patch that removes this ssleep().

Pavel, what do you think?

Rafael

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

-- 
VGER BF report: H 4.55007e-15
#7 lore raw

Re: [RFC][PATCH 2/3] PM: Make console suspending configureable

Quoted 34 lines · Rafael J. Wysocki
Quoted 24 lines · Stefan Seyfried

Hi,

sorry, i am only slowly catching up after vacation.

Quoted 19 lines · Rafael J. Wysocki

Change suspend_console() so that it waits for all consoles to flush the remaining messages and make it possible to switch the console suspending off with the help of a Kconfig option.

Signed-off-by Rafael J. Wysocki <[email protected]>
kernel/printk.c +4 -0
+#ifndef CONFIG_DISABLE_CONSOLE_SUSPEND
 /**
  * suspend_console - suspend the console subsystem
  *

@@ -709,8 +710,14 @@ int __init add_preferred_console(char *n
  */
 void suspend_console(void)
 {
+	printk("Suspending console(s)\n");
 	acquire_console_sem();
 	console_suspended = 1;
+	/* This is needed so that all of the messages that have already been
+	 * written to all consoles can be actually transmitted (eg. over a
Quoted 8 lines

kernel/printk.c int __init add_preferred_console(char *n · line 720

Quoted 3 lines
kernel/printk.c +3 -0
+	 * network) before we try to suspend the consoles' devices.
+	 */
+	ssleep(2);

Sorry, but no. Suspend and resume is already slow enough, no need to make both of them much slower.
If we can condition this on the netconsole being used, ok, but not for the most common case of "console is on plain VGA".

Quoted 4 lines

Hm, it already is in -mm, but of course I can prepare a patch that removes this ssleep().

Pavel, what do you think?

Well, in suspend-to-ram case, 2 seconds is quite a lot... like more than rest of suspend, so stefan has some point...

Signature · 5 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

-- 
VGER BF report: H 1.68641e-06
#8 lore raw

Re: [RFC][PATCH 2/3] PM: Make console suspending configureable

Quoted 32 lines · Pavel Machek
Quoted 31 lines · Rafael J. Wysocki
Quoted 30 lines · Stefan Seyfried

Hi,

sorry, i am only slowly catching up after vacation.

Quoted 11 lines · Rafael J. Wysocki

Change suspend_console() so that it waits for all consoles to flush the remaining messages and make it possible to switch the console suspending off with the help of a Kconfig option.

Signed-off-by Rafael J. Wysocki <[email protected]>
kernel/printk.c +1 -0
+#ifndef CONFIG_DISABLE_CONSOLE_SUSPEND
 /**
  * suspend_console - suspend the console subsystem
  *

@@ -709,8 +710,14 @@ int __init add_preferred_console(char *n
  */

kernel/printk.c int __init add_preferred_console(char *n · line 720

Quoted 10 lines
kernel/printk.c +6 -0
 void suspend_console(void)
 {
+	printk("Suspending console(s)\n");
 	acquire_console_sem();
 	console_suspended = 1;
+	/* This is needed so that all of the messages that have already been
+	 * written to all consoles can be actually transmitted (eg. over a
+	 * network) before we try to suspend the consoles' devices.
+	 */
+	ssleep(2);

Sorry, but no. Suspend and resume is already slow enough, no need to make both of them much slower.
If we can condition this on the netconsole being used, ok, but not for the most common case of "console is on plain VGA".

Quoted 7 lines
Quoted 4 lines

Hm, it already is in -mm, but of course I can prepare a patch that removes this ssleep().

Pavel, what do you think?

Well, in suspend-to-ram case, 2 seconds is quite a lot... like more than rest of suspend, so stefan has some point...

Rafael added this "ssleep 2" because of a bug I reported on LKML, see the thread "2.6.18-rc4-mm1: eth0: trigger_send() called with the transmitter busy"
(http://marc.theaimsgroup.com/?l=linux-kernel&m=115565636718377&w=2).

Basically, I was having issues when suspending with netconsole on my Realtek RTL-8029 network card (ne2k-pci).

But further investigations revealed that this driver (ne2k-pci) have issues with suspend/resume even when netconsole is disabled (see http://bugzilla.kernel.org/show_bug.cgi?id=7082). These days, I'm unable to do some tests with latest kernels because my ADSL router is broken.

The point of this mail is that we should get rid of this "ssleep 2" since nobody else reported such an issue with netconsole and the network driver I'm using appears to be not so clean.

~~
laurent

author
#9 lore raw

RFC PATCH 3/3 PM: Remove PM_TRACE from Kconfig

In reply to Rafael J. Wysocki

Remove the CONFIG_PM_TRACE option, which is dangerous and should only be used by people who know exactly what they are doing, from Kconfig.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 kernel/power/Kconfig |   18 ------------------
 1 files changed, 18 deletions(-)
kernel/power/Kconfig +0 -18
Index: linux-2.6.18-rc4-mm1/kernel/power/Kconfig
===================================================================
@@ -47,24 +47,6 @@ config PM_DISABLE_CONSOLE_SUSPEND
 	suspend/resume routines, but may itself lead to problems, for example
 	if netconsole is used.
 
-config PM_TRACE
-	bool "Suspend/resume event tracing"
-	depends on PM && PM_DEBUG && X86_32 && EXPERIMENTAL
-	default n
-	---help---
-	This enables some cheesy code to save 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).
-
-	To use this debugging feature you should attempt to suspend the machine,
-	then reboot it, then run
-
-		dmesg -s 1000000 | grep 'hash matches'
-
-	CAUTION: this option will cause your machine's real-time clock to be
-	set to an invalid time after a resume.
-
-
 config SOFTWARE_SUSPEND
 	bool "Software Suspend"
 	depends on PM && SWAP && (X86 && (!SMP || SUSPEND_SMP)) || ((FRV || PPC32) && !SMP)
#10 lore raw

Re: [linux-pm] [RFC][PATCH 3/3] PM: Remove PM_TRACE from Kconfig

Quoted 2 lines · Rafael J. Wysocki

Remove the CONFIG_PM_TRACE option, which is dangerous and should only be used by people who know exactly what they are doing, from Kconfig.

No, don't remove this, that's not acceptable at all. This is useful for others (and one specifically who will be pissed to see this removed...)

So NAK to this.

thanks,

greg k-h

author
#11 lore raw

Re: [linux-pm] [RFC][PATCH 3/3] PM: Remove PM_TRACE from Kconfig

Quoted 6 lines · Greg KH
Quoted 2 lines · Rafael J. Wysocki

Remove the CONFIG_PM_TRACE option, which is dangerous and should only be used by people who know exactly what they are doing, from Kconfig.

No, don't remove this, that's not acceptable at all. This is useful for others (and one specifically who will be pissed to see this removed...)

Well, this was a Pavel's idea. ;-)

However, it does some harm to users. I was hit by it personally and there's been at least one report related to it on LKML recently.

Still I think we can just mark it as dangerous or something like that, which was my initial idea.

Greetings,
Rafael

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

Re: [linux-pm] [RFC][PATCH 3/3] PM: Remove PM_TRACE from Kconfig

In reply to Greg KH

Hi!

Quoted 5 lines
Quoted 2 lines

Remove the CONFIG_PM_TRACE option, which is dangerous and should only be used by people who know exactly what they are doing, from Kconfig.

No, don't remove this, that's not acceptable at all. This is useful for others (and one specifically who will be pissed to see this removed...)

Yep, while it breaks suspend for every pool soul that enables it by mistake. (And in hard-to-debug way, too).

This option has EXACTLY ONE USER... or more precisely used to have one user when he was debugging his mac mini...
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#13 lore raw

Re: [linux-pm] [RFC][PATCH 3/3] PM: Remove PM_TRACE from Kconfig

Quoted 5 lines · Pavel Machek

Hi!

Quoted 3 lines
Quoted 2 lines

Remove the CONFIG_PM_TRACE option, which is dangerous and should only be used by people who know exactly what they are doing, from Kconfig.

Quoted 8 lines
Quoted 2 lines

No, don't remove this, that's not acceptable at all. This is useful for others (and one specifically who will be pissed to see this removed...)

Yep, while it breaks suspend for every pool soul that enables it by mistake. (And in hard-to-debug way, too).

This option has EXACTLY ONE USER... or more precisely used to have one user when he was debugging his mac mini...

I tried it out for a bit when having problems with resume, which I have yet to resolve properly, so I'll need it again when I get the chance.

And you go tell that "ONE USER" that you disabled his feature :)

thanks,

greg k-h

#14 lore raw
In reply to Rafael J. Wysocki

Hi!

Quoted 23 lines
Quoted 4 lines
Quoted 4 lines

The appended patch does the following:

1) Adds suspend_console() and resume_console() to the suspend-to-disk code paths so that people using netconsole are safe with swsusp.

So I assume this one is OK.

Quoted 4 lines
Quoted 2 lines

2) Adds a Kconfig option allowing us to disable suspend_/resume_console() if need be.

Slightly ugly, but I guess that is the way to go.

It also seems to be needed to add a 2 sec. dealy in suspend_console() so that eg. the network console can send the messages before we try to suspend the device.

Quoted 7 lines
Quoted 1 line

3) Marks CONFIG_PM_TRACE as dangerous.

I do not think that is enough. "(WILL TRASH YOUR CMOS)" would be more suitable. Dangerous is "may cause problems to you". This is "will cause problems to you". And for this to be useful, people have to edit sources, anyway.

Quoted 8 lines
Quoted 3 lines

Pavel

Can we just delete the config option?

OK, I'll do that.

I have divided the changes into three individual patches that will follow as replies to this message.

Ack on all three... and thanks.

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#15 lore raw
In reply to Pavel Machek

Hi.

Quoted 1 line · Pavel Machek

I do not think that is enough. "(WILL TRASH YOUR CMOS)" would be more

But 'trash your cmos' says too much. 'Trash your cmos clock' might be better.

Regards,

Nigel