[RFC][PATCH] PM: Use suspend_console in swsusp and make it configureable

[RFC][PATCH] PM: Use suspend_console in swsusp and make it configureable

[Rafael J. Wysocki]

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 <rjw@sisk.pl>
---
 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(-)

Index: linux-2.6.18-rc4-mm1/kernel/power/disk.c
===================================================================
--- linux-2.6.18-rc4-mm1.orig/kernel/power/disk.c
+++ 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:
Index: linux-2.6.18-rc4-mm1/kernel/power/user.c
===================================================================
--- linux-2.6.18-rc4-mm1.orig/kernel/power/user.c
+++ 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);
 
Index: linux-2.6.18-rc4-mm1/include/linux/console.h
===================================================================
--- linux-2.6.18-rc4-mm1.orig/include/linux/console.h
+++ 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
Index: linux-2.6.18-rc4-mm1/kernel/power/Kconfig
===================================================================
--- linux-2.6.18-rc4-mm1.orig/kernel/power/Kconfig
+++ 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---
Index: linux-2.6.18-rc4-mm1/kernel/printk.c
===================================================================
--- linux-2.6.18-rc4-mm1.orig/kernel/printk.c
+++ 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.

Re: [RFC][PATCH] PM: Use suspend_console in swsusp and make it configureable

[Pavel Machek]

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.

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

> 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?
> Index: linux-2.6.18-rc4-mm1/kernel/power/Kconfig
> ===================================================================
> --- linux-2.6.18-rc4-mm1.orig/kernel/power/Kconfig
> +++ linux-2.6.18-rc4-mm1/kernel/power/Kconfig
>  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---

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

Re: [RFC][PATCH] PM: Use suspend_console in swsusp and make it configureable

[Rafael J. Wysocki]

Hi,

On Wednesday 16 August 2006 12:41, Pavel Machek wrote:
> 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.

So I assume this one is OK.

> > 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.

> > 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


-- 
You never change things by fighting the existing reality.
		R. Buckminster Fuller

[RFC][PATCH 1/3] swsusp: Use suspend_console

[Rafael J. Wysocki]

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 <rjw@sisk.pl>
---
 kernel/power/disk.c |    8 ++++++++
 kernel/power/user.c |    8 +++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

Index: linux-2.6.18-rc4-mm1/kernel/power/disk.c
===================================================================
--- linux-2.6.18-rc4-mm1.orig/kernel/power/disk.c	2006-08-16 11:51:35.000000000 +0200
+++ linux-2.6.18-rc4-mm1/kernel/power/disk.c	2006-08-16 11:58:01.000000000 +0200
@@ -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:
Index: linux-2.6.18-rc4-mm1/kernel/power/user.c
===================================================================
--- linux-2.6.18-rc4-mm1.orig/kernel/power/user.c	2006-08-16 11:51:35.000000000 +0200
+++ linux-2.6.18-rc4-mm1/kernel/power/user.c	2006-08-16 11:58:01.000000000 +0200
@@ -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);
 

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

[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 <rjw@sisk.pl>
---
 include/linux/console.h |    5 +++++
 kernel/power/Kconfig    |   11 +++++++++++
 kernel/printk.c         |    8 ++++++++
 3 files changed, 24 insertions(+)

Index: linux-2.6.18-rc4-mm1/include/linux/console.h
===================================================================
--- linux-2.6.18-rc4-mm1.orig/include/linux/console.h	2006-08-16 11:59:56.000000000 +0200
+++ linux-2.6.18-rc4-mm1/include/linux/console.h	2006-08-16 12:00:01.000000000 +0200
@@ -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
Index: linux-2.6.18-rc4-mm1/kernel/power/Kconfig
===================================================================
--- linux-2.6.18-rc4-mm1.orig/kernel/power/Kconfig	2006-08-16 11:59:56.000000000 +0200
+++ linux-2.6.18-rc4-mm1/kernel/power/Kconfig	2006-08-16 12:01:26.000000000 +0200
@@ -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
Index: linux-2.6.18-rc4-mm1/kernel/printk.c
===================================================================
--- linux-2.6.18-rc4-mm1.orig/kernel/printk.c	2006-08-16 11:59:56.000000000 +0200
+++ linux-2.6.18-rc4-mm1/kernel/printk.c	2006-08-16 12:08:59.000000000 +0200
@@ -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.

[RFC][PATCH 3/3] PM: Remove PM_TRACE from Kconfig

[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 <rjw@sisk.pl>
---
 kernel/power/Kconfig |   18 ------------------
 1 files changed, 18 deletions(-)

Index: linux-2.6.18-rc4-mm1/kernel/power/Kconfig
===================================================================
--- linux-2.6.18-rc4-mm1.orig/kernel/power/Kconfig
+++ 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)

Re: [RFC][PATCH] PM: Use suspend_console in swsusp and make it configureable

[Pavel Machek]

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.
> 
> So I assume this one is OK.
> 
> > > 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.
> 
> > > 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.

Ack on all three... and thanks.

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

Re: [RFC][PATCH] PM: Use suspend_console in swsusp and make it configureable

[Nigel Cunningham]

Hi.

On Wed, 2006-08-16 at 12:41 +0200, Pavel Machek wrote:
> 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

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

[Greg KH]

On Wed, Aug 16, 2006 at 01:14:11PM +0200, Rafael J. Wysocki wrote:
> 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

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

[Rafael J. Wysocki]

On Thursday 17 August 2006 06:40, Greg KH wrote:
> On Wed, Aug 16, 2006 at 01:14:11PM +0200, Rafael J. Wysocki wrote:
> > 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


-- 
You never change things by fighting the existing reality.
		R. Buckminster Fuller

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

[Pavel Machek]

Hi!

> > 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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[Greg KH]

On Thu, Aug 17, 2006 at 11:12:38AM +0200, Pavel Machek wrote:
> Hi!
> 
> > > 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...

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

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

[Stefan Seyfried]

Hi,

sorry, i am only slowly catching up after vacation.

On Wed, Aug 16, 2006 at 01:09:34PM +0200, Rafael J. Wysocki wrote:
> 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 <rjw@sisk.pl>

> +#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);

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".

-- 
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

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

[Pavel Machek]

On Mon 2006-09-04 13:03:25, Rafael J. Wysocki wrote:
> On Monday, 4 September 2006 11:08, Stefan Seyfried wrote:
> > Hi,
> > 
> > sorry, i am only slowly catching up after vacation.
> > 
> > On Wed, Aug 16, 2006 at 01:09:34PM +0200, Rafael J. Wysocki wrote:
> > > 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 <rjw@sisk.pl>
> > 
> > > +#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);
> > 
> > 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?

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

-- 
(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

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

[Rafael J. Wysocki]

On Monday, 4 September 2006 11:08, Stefan Seyfried wrote:
> Hi,
> 
> sorry, i am only slowly catching up after vacation.
> 
> On Wed, Aug 16, 2006 at 01:09:34PM +0200, Rafael J. Wysocki wrote:
> > 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 <rjw@sisk.pl>
> 
> > +#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);
> 
> 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


-- 
You never change things by fighting the existing reality.
		R. Buckminster Fuller

-- 
VGER BF report: H 4.55007e-15

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

[Laurent Riffard]

Le 04.09.2006 13:02, Pavel Machek a écrit :
> On Mon 2006-09-04 13:03:25, Rafael J. Wysocki wrote:
>> On Monday, 4 September 2006 11:08, Stefan Seyfried wrote:
>>> Hi,
>>>
>>> sorry, i am only slowly catching up after vacation.
>>>
>>> On Wed, Aug 16, 2006 at 01:09:34PM +0200, Rafael J. Wysocki wrote:
>>>> 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 <rjw@sisk.pl>
>>>> +#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);
>>> 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?
> 
> 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