linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH, 2nd try] make disable_console_suspend runtime configurable
@ 2007-06-13 20:03 Stefan Seyfried
  2007-06-13 21:36 ` Rafael J. Wysocki
  2007-06-13 22:08 ` Pavel Machek
  0 siblings, 2 replies; 13+ messages in thread
From: Stefan Seyfried @ 2007-06-13 20:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: suspend-devel List, Frank Seidel, akpm, nigel, pavel, Rafael J. Wysocki

I hate having to recompile the kernel, just to be able to debug suspend.
Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
/sys/power/disable_console_suspend.

Signed-off-by: Stefan Seyfried <seife@suse.de>
Signed-off-by: Frank Seidel <fseidel@suse.de>
---
Second try, no longer uses a sysctl in /proc, but a tunable in /sys/power.
Thanks Frank for moving it quicker to sysfs than i would have ever been
able to :-)

 Documentation/power/basic-pm-debugging.txt |   16 +++++++++-------
 drivers/serial/serial_core.c               |    8 ++------
 include/linux/console.h                    |    7 ++-----
 kernel/power/Kconfig                       |   11 -----------
 kernel/power/main.c                        |   27 +++++++++++++++++++++++++++
 kernel/printk.c                            |    7 +++++--
 6 files changed, 45 insertions(+), 31 deletions(-)

--- a/Documentation/power/basic-pm-debugging.txt
+++ b/Documentation/power/basic-pm-debugging.txt
@@ -78,13 +78,15 @@ c) Advanced debugging
 In case the STD does not work on your system even in the minimal configuration
 and compiling more drivers as modules is not practical or some modules cannot
 be unloaded, you can use one of the more advanced debugging techniques to find
-the problem.  First, if there is a serial port in your box, you can set the
-CONFIG_DISABLE_CONSOLE_SUSPEND kernel configuration option and try to log kernel
-messages using the serial console.  This may provide you with some information
-about the reasons of the suspend (resume) failure.  Alternatively, it may be
-possible to use a FireWire port for debugging with firescope
-(ftp://ftp.firstfloor.org/pub/ak/firescope/).  On i386 it is also possible to
-use the PM_TRACE mechanism documented in Documentation/s2ram.txt .
+the problem.  First, if there is a serial port in your box, you can
+
+# echo 1 > /sys/power/disable_console_suspend
+
+and try to log kernel messages using the serial console.  This may provide you
+with some information about the reasons of the suspend (resume) failure.
+Alternatively, it may be possible to use a FireWire port for debugging with
+firescope (ftp://ftp.firstfloor.org/pub/ak/firescope/).  On i386 it is also
+possible to use the PM_TRACE mechanism documented in Documentation/s2ram.txt .
 
 2. Testing suspend to RAM (STR)
 
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1934,12 +1934,10 @@ int uart_suspend_port(struct uart_driver
 
 	mutex_lock(&state->mutex);
 
-#ifdef CONFIG_DISABLE_CONSOLE_SUSPEND
-	if (uart_console(port)) {
+	if (disable_console_suspend && uart_console(port)) {
 		mutex_unlock(&state->mutex);
 		return 0;
 	}
-#endif
 
 	if (state->info && state->info->flags & UIF_INITIALIZED) {
 		const struct uart_ops *ops = port->ops;
@@ -1982,12 +1980,10 @@ int uart_resume_port(struct uart_driver 
 
 	mutex_lock(&state->mutex);
 
-#ifdef CONFIG_DISABLE_CONSOLE_SUSPEND
-	if (uart_console(port)) {
+	if (disable_console_suspend && uart_console(port)) {
 		mutex_unlock(&state->mutex);
 		return 0;
 	}
-#endif
 
 	uart_change_pm(state, 0);
 
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -64,6 +64,8 @@ extern const struct consw vga_con;	/* VG
 extern const struct consw newport_con;	/* SGI Newport console  */
 extern const struct consw prom_con;	/* SPARC PROM console */
 
+extern int disable_console_suspend;
+
 int con_is_bound(const struct consw *csw);
 int register_con_driver(const struct consw *csw, int first, int last);
 int unregister_con_driver(const struct consw *csw);
@@ -120,14 +122,9 @@ 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 */
 
 int mda_console_init(void);
 void prom_con_init(void);
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -37,17 +37,6 @@ config PM_DEBUG
 	code. This is helpful when debugging and reporting various PM bugs, 
 	like suspend support.
 
-config 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
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -76,6 +76,7 @@ struct console *console_drivers;
  * locked without the console sempahore held
  */
 static int console_locked, console_suspended;
+int disable_console_suspend;
 
 /*
  * logbuf_lock protects log_buf, log_start, log_end, con_start and logged_chars
@@ -726,7 +727,6 @@ int __init add_preferred_console(char *n
 	return 0;
 }
 
-#ifndef CONFIG_DISABLE_CONSOLE_SUSPEND
 /**
  * suspend_console - suspend the console subsystem
  *
@@ -734,6 +734,8 @@ int __init add_preferred_console(char *n
  */
 void suspend_console(void)
 {
+	if (disable_console_suspend)
+		return;
 	printk("Suspending console(s)\n");
 	acquire_console_sem();
 	console_suspended = 1;
@@ -741,10 +743,11 @@ void suspend_console(void)
 
 void resume_console(void)
 {
+	if (disable_console_suspend)
+		return;
 	console_suspended = 0;
 	release_console_sem();
 }
-#endif /* CONFIG_DISABLE_CONSOLE_SUSPEND */
 
 /**
  * acquire_console_sem - lock the console system for exclusive use.
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -308,6 +308,31 @@ static ssize_t state_store(struct kset *
 
 power_attr(state);
 
+/**
+ * disable_console_suspend - control console suspend
+ *
+ * If set the consoles are not suspended so it is
+ * still possible to see and collect debug info.
+ */
+static ssize_t disable_console_suspend_show(struct kset *kset, char *buf)
+{
+	return sprintf(buf, "%i\n", disable_console_suspend);
+}
+
+static ssize_t disable_console_suspend_store(struct kset *kset, const char *buf, size_t n)
+{
+	int disable_console;
+
+	if (sscanf(buf, "%i", &disable_console) == 1) {
+		disable_console_suspend = disable_console;
+		return n;
+	}
+
+	return -EINVAL;
+}
+
+power_attr(disable_console_suspend);
+
 #ifdef CONFIG_PM_TRACE
 int pm_trace_enabled;
 
@@ -332,12 +357,14 @@ power_attr(pm_trace);
 
 static struct attribute * g[] = {
 	&state_attr.attr,
+	&disable_console_suspend_attr.attr,
 	&pm_trace_attr.attr,
 	NULL,
 };
 #else
 static struct attribute * g[] = {
 	&state_attr.attr,
+	&disable_console_suspend_attr.attr,
 	NULL,
 };
 #endif /* CONFIG_PM_TRACE */

-- 
Stefan Seyfried
QA / R&D Team Mobile Devices        |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg  | "Well, surrounding them's out." 

This footer brought to you by insane German lawmakers:
SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH, 2nd try] make disable_console_suspend runtime configurable
  2007-06-13 20:03 [PATCH, 2nd try] make disable_console_suspend runtime configurable Stefan Seyfried
@ 2007-06-13 21:36 ` Rafael J. Wysocki
  2007-06-13 22:08 ` Pavel Machek
  1 sibling, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2007-06-13 21:36 UTC (permalink / raw)
  To: Stefan Seyfried
  Cc: linux-kernel, suspend-devel List, Frank Seidel, akpm, nigel, pavel

On Wednesday, 13 June 2007 22:03, Stefan Seyfried wrote:
> I hate having to recompile the kernel, just to be able to debug suspend.
> Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
> /sys/power/disable_console_suspend.
> 
> Signed-off-by: Stefan Seyfried <seife@suse.de>
> Signed-off-by: Frank Seidel <fseidel@suse.de>
> ---
> Second try, no longer uses a sysctl in /proc, but a tunable in /sys/power.
> Thanks Frank for moving it quicker to sysfs than i would have ever been
> able to :-)

ACK

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH, 2nd try] make disable_console_suspend runtime configurable
  2007-06-13 20:03 [PATCH, 2nd try] make disable_console_suspend runtime configurable Stefan Seyfried
  2007-06-13 21:36 ` Rafael J. Wysocki
@ 2007-06-13 22:08 ` Pavel Machek
  2007-06-13 22:23   ` Stefan Seyfried
  1 sibling, 1 reply; 13+ messages in thread
From: Pavel Machek @ 2007-06-13 22:08 UTC (permalink / raw)
  To: Stefan Seyfried
  Cc: linux-kernel, suspend-devel List, Frank Seidel, akpm, nigel,
	Rafael J. Wysocki

Hi!

> I hate having to recompile the kernel, just to be able to debug suspend.
> Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
> /sys/power/disable_console_suspend.

> Signed-off-by: Stefan Seyfried <seife@suse.de>
> Signed-off-by: Frank Seidel <fseidel@suse.de>

I wonder if there's a better name?

Or maybe this should not be /sys configurable, but just have value for
each console "this console can work while suspended"?

(serial can, vesafb can, netconsole can't)?

Exporting "crash-me" option to user does not seem that cool to me.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH, 2nd try] make disable_console_suspend runtime configurable
  2007-06-13 22:08 ` Pavel Machek
@ 2007-06-13 22:23   ` Stefan Seyfried
  2007-06-13 23:09     ` Rafael J. Wysocki
  2007-06-14 12:14     ` Rafael J. Wysocki
  0 siblings, 2 replies; 13+ messages in thread
From: Stefan Seyfried @ 2007-06-13 22:23 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-kernel, suspend-devel List, Frank Seidel, akpm, nigel,
	Rafael J. Wysocki

On Thu, Jun 14, 2007 at 12:08:00AM +0200, Pavel Machek wrote:
> Hi!
> 
> > I hate having to recompile the kernel, just to be able to debug suspend.
> > Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
> > /sys/power/disable_console_suspend.
> 
> > Signed-off-by: Stefan Seyfried <seife@suse.de>
> > Signed-off-by: Frank Seidel <fseidel@suse.de>
> 
> I wonder if there's a better name?

Suggest one.

> Or maybe this should not be /sys configurable, but just have value for
> each console "this console can work while suspended"?
> 
> (serial can, vesafb can, netconsole can't)?

Go ahead, submit a patch. It won't be that trivial. And i wonder
if it is actually worth the hassle. This is a debugging facility.

> Exporting "crash-me" option to user does not seem that cool to me.

We have "echo c > /proc/sysrq-trigger" also.
This is a debugging option, and forcing users to recompile the kernel just
to debug suspend problems (not resume problems, the "it does not even go to
sleep" stuff is where this matters most) is IMO a bad idea.

We can also make this a boot parameter, i don't care, but i want to disable
console suspend without recompiling the kernel.
-- 
Stefan Seyfried
QA / R&D Team Mobile Devices        |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg  | "Well, surrounding them's out." 

This footer brought to you by insane German lawmakers:
SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH, 2nd try] make disable_console_suspend runtime configurable
  2007-06-13 22:23   ` Stefan Seyfried
@ 2007-06-13 23:09     ` Rafael J. Wysocki
  2007-06-14 12:14     ` Rafael J. Wysocki
  1 sibling, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2007-06-13 23:09 UTC (permalink / raw)
  To: Stefan Seyfried
  Cc: Pavel Machek, linux-kernel, suspend-devel List, Frank Seidel,
	akpm, nigel

On Thursday, 14 June 2007 00:23, Stefan Seyfried wrote:
> On Thu, Jun 14, 2007 at 12:08:00AM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > I hate having to recompile the kernel, just to be able to debug suspend.
> > > Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
> > > /sys/power/disable_console_suspend.
> > 
> > > Signed-off-by: Stefan Seyfried <seife@suse.de>
> > > Signed-off-by: Frank Seidel <fseidel@suse.de>
> > 
> > I wonder if there's a better name?
> 
> Suggest one.
> 
> > Or maybe this should not be /sys configurable, but just have value for
> > each console "this console can work while suspended"?
> > 
> > (serial can, vesafb can, netconsole can't)?
> 
> Go ahead, submit a patch. It won't be that trivial. And i wonder
> if it is actually worth the hassle. This is a debugging facility.
> 
> > Exporting "crash-me" option to user does not seem that cool to me.
> 
> We have "echo c > /proc/sysrq-trigger" also.
> This is a debugging option, and forcing users to recompile the kernel just
> to debug suspend problems (not resume problems, the "it does not even go to
> sleep" stuff is where this matters most) is IMO a bad idea.
> 
> We can also make this a boot parameter, i don't care, but i want to disable
> console suspend without recompiling the kernel.

It's a bit similar to the pm_trace thing.

As long as there is a sane default (ie. the consoles are disabled), I don't see
a big problem with that.

Moreover, distributions will probably want to have a switch so that they can
tell the user "please do 'echo 1 > ...' and retest" and if they start to add
such things by themselves that wouldn't be nice.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH, 2nd try] make disable_console_suspend runtime configurable
  2007-06-13 22:23   ` Stefan Seyfried
  2007-06-13 23:09     ` Rafael J. Wysocki
@ 2007-06-14 12:14     ` Rafael J. Wysocki
  2007-06-14 13:59       ` [PATCH, 3rd " Frank Seidel
  1 sibling, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2007-06-14 12:14 UTC (permalink / raw)
  To: Stefan Seyfried
  Cc: Pavel Machek, linux-kernel, suspend-devel List, Frank Seidel,
	akpm, nigel

On Thursday, 14 June 2007 00:23, Stefan Seyfried wrote:
> On Thu, Jun 14, 2007 at 12:08:00AM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > I hate having to recompile the kernel, just to be able to debug suspend.
> > > Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
> > > /sys/power/disable_console_suspend.
> > 
> > > Signed-off-by: Stefan Seyfried <seife@suse.de>
> > > Signed-off-by: Frank Seidel <fseidel@suse.de>
> > 
> > I wonder if there's a better name?
> 
> Suggest one.

Hmm, you could call it 'console_suspend' with
'echo disabled > /sys/power/console_suspend' for switching that off
(and analogously with 'enabled') and with 'cat /sys/power/console_suspend'
printing the current status.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH, 3rd try] make disable_console_suspend runtime configurable
  2007-06-14 12:14     ` Rafael J. Wysocki
@ 2007-06-14 13:59       ` Frank Seidel
  2007-06-14 22:20         ` Rafael J. Wysocki
  0 siblings, 1 reply; 13+ messages in thread
From: Frank Seidel @ 2007-06-14 13:59 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Stefan Seyfried, Pavel Machek, linux-kernel, suspend-devel List,
	akpm, nigel

From: Stefan Seyfried <seife@suse.de>

I hate having to recompile the kernel, just to be able to debug suspend.
Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
/sys/power/disable_console_suspend.


Signed-off-by: Stefan Seyfried <seife@suse.de>
Signed-off-by: Frank Seidel <fseidel@suse.de>
---
Third try, renamed sysfs interface to console_suspend 
reporting and expecting either "enabled" or "disabled"

 Documentation/power/basic-pm-debugging.txt |   16 +++++++++-------
 drivers/serial/serial_core.c               |    8 ++------
 include/linux/console.h                    |    7 ++-----
 kernel/power/Kconfig                       |   11 -----------
 kernel/power/main.c                        |   28 ++++++++++++++++++++++++++++
 kernel/printk.c                            |    7 +++++--
 6 files changed, 46 insertions(+), 31 deletions(-)

--- a/Documentation/power/basic-pm-debugging.txt
+++ b/Documentation/power/basic-pm-debugging.txt
@@ -78,13 +78,15 @@ c) Advanced debugging
 In case the STD does not work on your system even in the minimal configuration
 and compiling more drivers as modules is not practical or some modules cannot
 be unloaded, you can use one of the more advanced debugging techniques to find
-the problem.  First, if there is a serial port in your box, you can set the
-CONFIG_DISABLE_CONSOLE_SUSPEND kernel configuration option and try to log kernel
-messages using the serial console.  This may provide you with some information
-about the reasons of the suspend (resume) failure.  Alternatively, it may be
-possible to use a FireWire port for debugging with firescope
-(ftp://ftp.firstfloor.org/pub/ak/firescope/).  On i386 it is also possible to
-use the PM_TRACE mechanism documented in Documentation/s2ram.txt .
+the problem.  First, if there is a serial port in your box, you can
+
+# echo disabled > /sys/power/console_suspend
+
+and try to log kernel messages using the serial console.  This may provide you
+with some information about the reasons of the suspend (resume) failure.
+Alternatively, it may be possible to use a FireWire port for debugging with
+firescope (ftp://ftp.firstfloor.org/pub/ak/firescope/).  On i386 it is also
+possible to use the PM_TRACE mechanism documented in Documentation/s2ram.txt .
 
 2. Testing suspend to RAM (STR)
 
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1934,12 +1934,10 @@ int uart_suspend_port(struct uart_driver
 
 	mutex_lock(&state->mutex);
 
-#ifdef CONFIG_DISABLE_CONSOLE_SUSPEND
-	if (uart_console(port)) {
+	if (disable_console_suspend && uart_console(port)) {
 		mutex_unlock(&state->mutex);
 		return 0;
 	}
-#endif
 
 	if (state->info && state->info->flags & UIF_INITIALIZED) {
 		const struct uart_ops *ops = port->ops;
@@ -1982,12 +1980,10 @@ int uart_resume_port(struct uart_driver 
 
 	mutex_lock(&state->mutex);
 
-#ifdef CONFIG_DISABLE_CONSOLE_SUSPEND
-	if (uart_console(port)) {
+	if (disable_console_suspend && uart_console(port)) {
 		mutex_unlock(&state->mutex);
 		return 0;
 	}
-#endif
 
 	uart_change_pm(state, 0);
 
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -64,6 +64,8 @@ extern const struct consw vga_con;	/* VG
 extern const struct consw newport_con;	/* SGI Newport console  */
 extern const struct consw prom_con;	/* SPARC PROM console */
 
+extern int disable_console_suspend;
+
 int con_is_bound(const struct consw *csw);
 int register_con_driver(const struct consw *csw, int first, int last);
 int unregister_con_driver(const struct consw *csw);
@@ -120,14 +122,9 @@ 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 */
 
 int mda_console_init(void);
 void prom_con_init(void);
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -37,17 +37,6 @@ config PM_DEBUG
 	code. This is helpful when debugging and reporting various PM bugs, 
 	like suspend support.
 
-config 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
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -76,6 +76,7 @@ struct console *console_drivers;
  * locked without the console sempahore held
  */
 static int console_locked, console_suspended;
+int disable_console_suspend;
 
 /*
  * logbuf_lock protects log_buf, log_start, log_end, con_start and logged_chars
@@ -726,7 +727,6 @@ int __init add_preferred_console(char *n
 	return 0;
 }
 
-#ifndef CONFIG_DISABLE_CONSOLE_SUSPEND
 /**
  * suspend_console - suspend the console subsystem
  *
@@ -734,6 +734,8 @@ int __init add_preferred_console(char *n
  */
 void suspend_console(void)
 {
+	if (disable_console_suspend)
+		return;
 	printk("Suspending console(s)\n");
 	acquire_console_sem();
 	console_suspended = 1;
@@ -741,10 +743,11 @@ void suspend_console(void)
 
 void resume_console(void)
 {
+	if (disable_console_suspend)
+		return;
 	console_suspended = 0;
 	release_console_sem();
 }
-#endif /* CONFIG_DISABLE_CONSOLE_SUSPEND */
 
 /**
  * acquire_console_sem - lock the console system for exclusive use.
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -308,6 +308,32 @@ static ssize_t state_store(struct kset *
 
 power_attr(state);
 
+/**
+ * disable_console_suspend - control console suspend
+ *
+ * If set the consoles are not suspended so it is
+ * still possible to see and collect debug info.
+ */
+static ssize_t console_suspend_show(struct kset *kset, char *buf)
+{
+	return sprintf(buf, disable_console_suspend ? "disabled\n" : "enabled\n");
+}
+
+static ssize_t console_suspend_store(struct kset *kset, const char *buf, size_t n)
+{
+	if (strncmp(buf, "disabled", 8) == 0) {
+		disable_console_suspend = 1;
+		return n;
+	} else if (strncmp(buf, "enabled", 7) == 0) {
+		disable_console_suspend = 0;
+		return n;
+	}
+
+	return -EINVAL;
+}
+
+power_attr(console_suspend);
+
 #ifdef CONFIG_PM_TRACE
 int pm_trace_enabled;
 
@@ -332,12 +358,14 @@ power_attr(pm_trace);
 
 static struct attribute * g[] = {
 	&state_attr.attr,
+	&console_suspend_attr.attr,
 	&pm_trace_attr.attr,
 	NULL,
 };
 #else
 static struct attribute * g[] = {
 	&state_attr.attr,
+	&console_suspend_attr.attr,
 	NULL,
 };
 #endif /* CONFIG_PM_TRACE */

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH, 3rd try] make disable_console_suspend runtime configurable
  2007-06-14 13:59       ` [PATCH, 3rd " Frank Seidel
@ 2007-06-14 22:20         ` Rafael J. Wysocki
  2007-06-17 21:35           ` Pavel Machek
  0 siblings, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2007-06-14 22:20 UTC (permalink / raw)
  To: Frank Seidel, Pavel Machek
  Cc: Stefan Seyfried, linux-kernel, suspend-devel List, akpm, nigel

On Thursday, 14 June 2007 15:59, Frank Seidel wrote:
> From: Stefan Seyfried <seife@suse.de>
> 
> I hate having to recompile the kernel, just to be able to debug suspend.
> Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
> /sys/power/disable_console_suspend.
> 
> 
> Signed-off-by: Stefan Seyfried <seife@suse.de>
> Signed-off-by: Frank Seidel <fseidel@suse.de>
> ---
> Third try, renamed sysfs interface to console_suspend 
> reporting and expecting either "enabled" or "disabled"

Thanks a lot for redoing it.

I have no objections.  Pavel?

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH, 3rd try] make disable_console_suspend runtime configurable
  2007-06-14 22:20         ` Rafael J. Wysocki
@ 2007-06-17 21:35           ` Pavel Machek
  2007-06-17 21:49             ` [Suspend-devel] " Pavel Machek
  0 siblings, 1 reply; 13+ messages in thread
From: Pavel Machek @ 2007-06-17 21:35 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Frank Seidel, Stefan Seyfried, linux-kernel, suspend-devel List,
	akpm, nigel

On Fri 2007-06-15 00:20:28, Rafael J. Wysocki wrote:
> On Thursday, 14 June 2007 15:59, Frank Seidel wrote:
> > From: Stefan Seyfried <seife@suse.de>
> > 
> > I hate having to recompile the kernel, just to be able to debug suspend.
> > Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
> > /sys/power/disable_console_suspend.
> > 
> > 
> > Signed-off-by: Stefan Seyfried <seife@suse.de>
> > Signed-off-by: Frank Seidel <fseidel@suse.de>
> > ---
> > Third try, renamed sysfs interface to console_suspend 
> > reporting and expecting either "enabled" or "disabled"
> 
> Thanks a lot for redoing it.
> 
> I have no objections.  Pavel?

I still think that patch is bad. I should have screamed when
CONFIG_DISABLE_CONSOLE_SUSPEND went into kernel. That beast should
_not_ be configurable, it should just do the right thing.

But I realized that too late. And this only makes it works, making
that mistake part of user-kernel interface.

Sorry for not screaming when CONFIG_DISABLE_CONSOLE_SUSPEND went in,
but please lets solve this correctly....

Like every console says if it needs to  be disabled during suspend,
and if at least one active console says so, we disable. Nothing to
configure, we just do the right thing. And it should not be that much
work, either.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Suspend-devel] [PATCH, 3rd try] make disable_console_suspend runtime configurable
  2007-06-17 21:35           ` Pavel Machek
@ 2007-06-17 21:49             ` Pavel Machek
  2007-06-18  5:27               ` Stefan Seyfried
  0 siblings, 1 reply; 13+ messages in thread
From: Pavel Machek @ 2007-06-17 21:49 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: nigel, linux-kernel, suspend-devel List, akpm, Stefan Seyfried

Hi!

> > > I hate having to recompile the kernel, just to be able to debug suspend.
> > > Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
> > > /sys/power/disable_console_suspend.
> > > 
> > > 
> > > Signed-off-by: Stefan Seyfried <seife@suse.de>
> > > Signed-off-by: Frank Seidel <fseidel@suse.de>
> > > ---
> > > Third try, renamed sysfs interface to console_suspend 
> > > reporting and expecting either "enabled" or "disabled"
> > 
> > Thanks a lot for redoing it.
> > 
> > I have no objections.  Pavel?
> 
> I still think that patch is bad. I should have screamed when
> CONFIG_DISABLE_CONSOLE_SUSPEND went into kernel. That beast should
> _not_ be configurable, it should just do the right thing.
> 
> But I realized that too late. And this only makes it works, making
> that mistake part of user-kernel interface.
> 
> Sorry for not screaming when CONFIG_DISABLE_CONSOLE_SUSPEND went in,
> but please lets solve this correctly....

Ouch and sorry for not screaming at "try 1" time. But it still does
not make the patch right, and I believe that even patch authors agree
that "no-config-needed" is superior solution.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Suspend-devel] [PATCH, 3rd try] make disable_console_suspend runtime configurable
  2007-06-17 21:49             ` [Suspend-devel] " Pavel Machek
@ 2007-06-18  5:27               ` Stefan Seyfried
  2007-06-21 13:20                 ` Pavel Machek
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Seyfried @ 2007-06-18  5:27 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Rafael J. Wysocki, nigel, linux-kernel, suspend-devel List, akpm

On Sun, Jun 17, 2007 at 11:49:40PM +0200, Pavel Machek wrote:
> Hi!
> 
> > > > I hate having to recompile the kernel, just to be able to debug suspend.
> > > > Remove CONFIG_DISABLE_CONSOLE_SUSPEND, replace it by a tunable in
> > > > /sys/power/disable_console_suspend.
> > > > 
> > > > 
> > > > Signed-off-by: Stefan Seyfried <seife@suse.de>
> > > > Signed-off-by: Frank Seidel <fseidel@suse.de>
> > > > ---
> > > > Third try, renamed sysfs interface to console_suspend 
> > > > reporting and expecting either "enabled" or "disabled"
> > > 
> > > Thanks a lot for redoing it.
> > > 
> > > I have no objections.  Pavel?
> > 
> > I still think that patch is bad. I should have screamed when
> > CONFIG_DISABLE_CONSOLE_SUSPEND went into kernel. That beast should
> > _not_ be configurable, it should just do the right thing.
> > 
> > But I realized that too late. And this only makes it works, making
> > that mistake part of user-kernel interface.
> > 
> > Sorry for not screaming when CONFIG_DISABLE_CONSOLE_SUSPEND went in,
> > but please lets solve this correctly....
> 
> Ouch and sorry for not screaming at "try 1" time. But it still does
> not make the patch right, and I believe that even patch authors agree
> that "no-config-needed" is superior solution.

No, i don't agree at all.

In this case, "no config needed" == "not possible to debug suspend problems".

IMO this is the same as issue as with "sysrq-C". You can crash the machine by
other means, but it sometimes is just handy to have a mechanism to do it.

I do not understand what's the problem with this option. If you want to avoid
that people use it for something else than debugging, i can add a patch that
crashes the machine ten seconds after resume if this option is set.
-- 
Stefan Seyfried
QA / R&D Team Mobile Devices        |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg  | "Well, surrounding them's out." 

This footer brought to you by insane German lawmakers:
SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Suspend-devel] [PATCH, 3rd try] make disable_console_suspend runtime configurable
  2007-06-18  5:27               ` Stefan Seyfried
@ 2007-06-21 13:20                 ` Pavel Machek
  2007-06-21 21:14                   ` Stefan Seyfried
  0 siblings, 1 reply; 13+ messages in thread
From: Pavel Machek @ 2007-06-21 13:20 UTC (permalink / raw)
  To: Stefan Seyfried
  Cc: Rafael J. Wysocki, nigel, linux-kernel, suspend-devel List, akpm

Hi!

> > > Sorry for not screaming when CONFIG_DISABLE_CONSOLE_SUSPEND went in,
> > > but please lets solve this correctly....
> > 
> > Ouch and sorry for not screaming at "try 1" time. But it still does
> > not make the patch right, and I believe that even patch authors agree
> > that "no-config-needed" is superior solution.
> 
> No, i don't agree at all.
> 
> In this case, "no config needed" == "not possible to debug suspend
> problems".

No, sorry.

My proposed solution is "figure out which console drivers can survive
being on while machines go down, and keep them on".

So, "no config needed" == "kernel always does the right thing, keeping
console during suspend when possible" == "possible to debug suspend
problems without having to change CONFIG_ or /sys/*".

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Suspend-devel] [PATCH, 3rd try] make disable_console_suspend runtime configurable
  2007-06-21 13:20                 ` Pavel Machek
@ 2007-06-21 21:14                   ` Stefan Seyfried
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Seyfried @ 2007-06-21 21:14 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Rafael J. Wysocki, nigel, linux-kernel, suspend-devel List, akpm

On Thu, Jun 21, 2007 at 03:20:08PM +0200, Pavel Machek wrote:
> Hi!
 
> > No, i don't agree at all.
> > 
> > In this case, "no config needed" == "not possible to debug suspend
> > problems".
> 
> No, sorry.
> 
> My proposed solution is "figure out which console drivers can survive
> being on while machines go down, and keep them on".
> 
> So, "no config needed" == "kernel always does the right thing, keeping
> console during suspend when possible" == "possible to debug suspend
> problems without having to change CONFIG_ or /sys/*".

Ok. Deal. Once you fixed all the console drivers, i'll gladly send a patch
that reverts the patch we are discussing now.

Note that this patch actually helps fixing those drivers, since you can
test much easier if a given driver survives suspend ;-)
-- 
Stefan Seyfried
QA / R&D Team Mobile Devices        |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg  | "Well, surrounding them's out." 

This footer brought to you by insane German lawmakers:
SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2007-06-21 21:14 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-06-13 20:03 [PATCH, 2nd try] make disable_console_suspend runtime configurable Stefan Seyfried
2007-06-13 21:36 ` Rafael J. Wysocki
2007-06-13 22:08 ` Pavel Machek
2007-06-13 22:23   ` Stefan Seyfried
2007-06-13 23:09     ` Rafael J. Wysocki
2007-06-14 12:14     ` Rafael J. Wysocki
2007-06-14 13:59       ` [PATCH, 3rd " Frank Seidel
2007-06-14 22:20         ` Rafael J. Wysocki
2007-06-17 21:35           ` Pavel Machek
2007-06-17 21:49             ` [Suspend-devel] " Pavel Machek
2007-06-18  5:27               ` Stefan Seyfried
2007-06-21 13:20                 ` Pavel Machek
2007-06-21 21:14                   ` Stefan Seyfried

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).