[RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Pekka J Enberg]

Hi,

Nigel, can I please get a sign-off for these if you're okay with them. 
Rafael, Pavel, please ACK or NAK. Thanks.

			Pekka

From: Nigel Cunningham <nigel@nigel.suspend2.net>

Adds a new config option CONFIG_PRINTK_NOSAVE which makes the kernel
preserve printk() output from boot after resume.  This is useful for
debugging hibernation.

Cc: Pavel Machek <pavel@ucw.cz>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 include/linux/suspend.h |    6 ++++++
 kernel/power/Kconfig    |   11 +++++++++++
 kernel/printk.c         |    7 ++++---
 3 files changed, 21 insertions(+), 3 deletions(-)

Index: 2.6/include/linux/suspend.h
===================================================================
--- 2.6.orig/include/linux/suspend.h	2007-04-27 14:42:13.000000000 +0300
+++ 2.6/include/linux/suspend.h	2007-04-27 14:43:13.000000000 +0300
@@ -55,4 +55,10 @@ unsigned long get_safe_page(gfp_t gfp_ma
  */
 #define PAGES_FOR_IO	1024
 
+#ifdef CONFIG_PRINTK_NOSAVE
+#define POSS_NOSAVE __nosavedata
+#else
+#define POSS_NOSAVE
+#endif
+
 #endif /* _LINUX_SWSUSP_H */
Index: 2.6/kernel/power/Kconfig
===================================================================
--- 2.6.orig/kernel/power/Kconfig	2007-04-27 14:42:13.000000000 +0300
+++ 2.6/kernel/power/Kconfig	2007-04-27 14:43:13.000000000 +0300
@@ -48,6 +48,17 @@ config DISABLE_CONSOLE_SUSPEND
 	suspend/resume routines, but may itself lead to problems, for example
 	if netconsole is used.
 
+config PRINTK_NOSAVE
+	bool "Preserve printk data from boot kernel when resuming."
+	default n
+	---help---
+	This option gives printk data and the associated variables the
+	attribute __nosave, which means that they will not be saves as
+	part of the image. The net effect is that after resuming, your
+	dmesg will show the messages from prior to the atomic restore,
+	instead of the messages from the resumed kernel. This may be
+	useful for debugging hibernation.
+
 config PM_TRACE
 	bool "Suspend/resume event tracing"
 	depends on PM && PM_DEBUG && X86_32 && EXPERIMENTAL
Index: 2.6/kernel/printk.c
===================================================================
--- 2.6.orig/kernel/printk.c	2007-04-27 14:42:13.000000000 +0300
+++ 2.6/kernel/printk.c	2007-04-27 14:43:13.000000000 +0300
@@ -32,6 +32,7 @@  *	01Mar01 Andrew Morton <andrewm@uow.ed
 #include <linux/bootmem.h>
 #include <linux/syscalls.h>
 #include <linux/jiffies.h>
+#include <linux/suspend.h>
 
 #include <asm/uaccess.h>
 
@@ -92,9 +93,9 @@ static DEFINE_SPINLOCK(logbuf_lock);
  * The indices into log_buf are not constrained to log_buf_len - they
  * must be masked before subscripting
  */
-static unsigned long log_start;	/* Index into log_buf: next char to be read by syslog() */
-static unsigned long con_start;	/* Index into log_buf: next char to be sent to consoles */
-static unsigned long log_end;	/* Index into log_buf: most-recently-written-char + 1 */
+static unsigned long POSS_NOSAVE log_start;	/* Index into log_buf: next char to be read by syslog() */
+static unsigned long POSS_NOSAVE con_start;	/* Index into log_buf: next char to be sent to consoles */
+static unsigned long POSS_NOSAVE log_end;	/* Index into log_buf: most-recently-written-char + 1 */
 
 /*
  *	Array of consoles built from command line options (console=)

Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Pavel Machek]

Hi!

> Nigel, can I please get a sign-off for these if you're okay with them. 
> Rafael, Pavel, please ACK or NAK. Thanks.

> Adds a new config option CONFIG_PRINTK_NOSAVE which makes the kernel
> preserve printk() output from boot after resume.  This is useful for
> debugging hibernation.

1. This should not be a config option


> @@ -92,9 +93,9 @@ static DEFINE_SPINLOCK(logbuf_lock);
>   * The indices into log_buf are not constrained to log_buf_len - they
>   * must be masked before subscripting
>   */
> -static unsigned long log_start;	/* Index into log_buf: next char to be read by syslog() */
> -static unsigned long con_start;	/* Index into log_buf: next char to be sent to consoles */
> -static unsigned long log_end;	/* Index into log_buf: most-recently-written-char + 1 */
> +static unsigned long POSS_NOSAVE log_start;	/* Index into log_buf: next char to be read by syslog() */
> +static unsigned long POSS_NOSAVE con_start;	/* Index into log_buf: next char to be sent to consoles */
> +static unsigned long POSS_NOSAVE log_end;	/* Index into log_buf: most-recently-written-char + 1 */

2. This does not work. pointers are nosave, but logbuf is not. Eh?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Johannes Berg]

[-- Attachment #1.1: Type: text/plain, Size: 266 bytes --]

On Fri, 2007-04-27 at 15:25 +0300, Pekka J Enberg wrote:
>  
> +#ifdef CONFIG_PRINTK_NOSAVE
> +#define POSS_NOSAVE __nosavedata

Interestingly I stumbled across this this morning. What's the purpose of
the POSS naming? I can't make sense of that.

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Pekka J Enberg]

On Fri, 27 Apr 2007, Pavel Machek wrote:
> 1. This should not be a config option

Ok.

On Fri, 27 Apr 2007, Pavel Machek wrote:
> 2. This does not work. pointers are nosave, but logbuf is not. Eh?

My bad, sorry. Thanks for the comments, Pavel!

			Pekka

[PATCH] kernel: preserve boot-time printk output after resume
From: Nigel Cunningham <nigel@nigel.suspend2.net>

This makes the kernel preserve boot-time printk() output after resume.

Cc: Pavel Machek <pavel@ucw.cz>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 kernel/printk.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Index: 2.6/kernel/printk.c
===================================================================
--- 2.6.orig/kernel/printk.c	2007-04-27 15:39:26.000000000 +0300
+++ 2.6/kernel/printk.c	2007-04-27 15:49:25.000000000 +0300
@@ -92,9 +92,9 @@ static DEFINE_SPINLOCK(logbuf_lock);
  * The indices into log_buf are not constrained to log_buf_len - they
  * must be masked before subscripting
  */
-static unsigned long log_start;	/* Index into log_buf: next char to be read by syslog() */
-static unsigned long con_start;	/* Index into log_buf: next char to be sent to consoles */
-static unsigned long log_end;	/* Index into log_buf: most-recently-written-char + 1 */
+static unsigned long __nosavedata log_start;	/* Index into log_buf: next char to be read by syslog() */
+static unsigned long __nosavedata con_start;	/* Index into log_buf: next char to be sent to consoles */
+static unsigned long __nosavedata log_end;	/* Index into log_buf: most-recently-written-char + 1 */
 
 /*
  *	Array of consoles built from command line options (console=)
@@ -117,10 +117,10 @@ static int console_may_schedule;
 
 #ifdef CONFIG_PRINTK
 
-static char __log_buf[__LOG_BUF_LEN];
-static char *log_buf = __log_buf;
-static int log_buf_len = __LOG_BUF_LEN;
-static unsigned long logged_chars; /* Number of chars produced since last read+clear operation */
+static char __nosavedata __log_buf[__LOG_BUF_LEN];
+static char __nosavedata *log_buf = __log_buf;
+static int __nosavedata log_buf_len = __LOG_BUF_LEN;
+static unsigned long __nosavedata logged_chars; /* Number of chars produced since last read+clear operation */
 
 static int __init log_buf_len_setup(char *str)
 {

Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Pavel Machek]

On Fri 2007-04-27 15:52:12, Pekka J Enberg wrote:
> On Fri, 27 Apr 2007, Pavel Machek wrote:
> > 1. This should not be a config option
> 
> Ok.

Unfortunately, we loose dmesg from the state saving this way, so I do
not think it can be hardcoded, either. Best way would be to somehow
preserve both dmesg buffers.

Other way is to just control it by the define in the .c file; person
using this is by definition kernel hacker, anyway. Or this can simply
live like debugging patch, interested parties can apply.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Alan Stern]

On Fri, 27 Apr 2007, Pavel Machek wrote:

> On Fri 2007-04-27 15:52:12, Pekka J Enberg wrote:
> > On Fri, 27 Apr 2007, Pavel Machek wrote:
> > > 1. This should not be a config option
> > 
> > Ok.
> 
> Unfortunately, we loose dmesg from the state saving this way, so I do
> not think it can be hardcoded, either. Best way would be to somehow
> preserve both dmesg buffers.
> 
> Other way is to just control it by the define in the .c file; person
> using this is by definition kernel hacker, anyway. Or this can simply
> live like debugging patch, interested parties can apply.

Consider this: Add a debugfs file containing a binary value.  If the value 
is on then preserve the boot dmesg buffer.  If the value is off, don't 
preserve it -- overwrite the buffer pointers with values saved in the 
image.

Alan Stern

Re: Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Pekka Enberg]

On 4/27/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> Consider this: Add a debugfs file containing a binary value.  If the value
> is on then preserve the boot dmesg buffer.  If the value is off, don't
> preserve it -- overwrite the buffer pointers with values saved in the
> image.

But isn't it impractical to change a debugfs file upon kernel startup
before we hit resume? Or do you mean we always make a copy of the
dmesg buffer before resume starts and afterwards replace the new one
if the debug value is set?

Re: Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Johannes Berg]

[-- Attachment #1.1: Type: text/plain, Size: 428 bytes --]

On Fri, 2007-04-27 at 10:03 -0400, Alan Stern wrote:

> Consider this: Add a debugfs file containing a binary value.  If the value 
> is on then preserve the boot dmesg buffer.  If the value is off, don't 
> preserve it -- overwrite the buffer pointers with values saved in the 
> image.

Or just add a debugfs file containing the printk buffer from before? I'd
think it's only useful for debugging anyway.

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Alan Stern]

On Fri, 27 Apr 2007, Pekka Enberg wrote:

> On 4/27/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> > Consider this: Add a debugfs file containing a binary value.  If the value
> > is on then preserve the boot dmesg buffer.  If the value is off, don't
> > preserve it -- overwrite the buffer pointers with values saved in the
> > image.
> 
> But isn't it impractical to change a debugfs file upon kernel startup
> before we hit resume? Or do you mean we always make a copy of the
> dmesg buffer before resume starts and afterwards replace the new one
> if the debug value is set?

Yes, that's what I meant.  It would make changing the setting a lot easier 
during testing.

Alan Stern

Re: Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Pavel Machek]

Hi!

> > Consider this: Add a debugfs file containing a binary value.  If the value 
> > is on then preserve the boot dmesg buffer.  If the value is off, don't 
> > preserve it -- overwrite the buffer pointers with values saved in the 
> > image.
> 
> Or just add a debugfs file containing the printk buffer from before? I'd
> think it's only useful for debugging anyway.

I like it better than binary option.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Nigel Cunningham]

[-- Attachment #1.1: Type: text/plain, Size: 709 bytes --]

Hi.

On Fri, 2007-04-27 at 14:57 +0200, Pavel Machek wrote:
> On Fri 2007-04-27 15:52:12, Pekka J Enberg wrote:
> > On Fri, 27 Apr 2007, Pavel Machek wrote:
> > > 1. This should not be a config option
> > 
> > Ok.
> 
> Unfortunately, we loose dmesg from the state saving this way, so I do
> not think it can be hardcoded, either. Best way would be to somehow
> preserve both dmesg buffers.
> 
> Other way is to just control it by the define in the .c file; person
> using this is by definition kernel hacker, anyway. Or this can simply
> live like debugging patch, interested parties can apply.

That's why I made it a config option - so you can choose whether you
want it or not.

Nigel

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Pavel Machek]

On Sat 2007-04-28 07:21:59, Nigel Cunningham wrote:
> Hi.
> 
> On Fri, 2007-04-27 at 14:57 +0200, Pavel Machek wrote:
> > On Fri 2007-04-27 15:52:12, Pekka J Enberg wrote:
> > > On Fri, 27 Apr 2007, Pavel Machek wrote:
> > > > 1. This should not be a config option
> > > 
> > > Ok.
> > 
> > Unfortunately, we loose dmesg from the state saving this way, so I do
> > not think it can be hardcoded, either. Best way would be to somehow
> > preserve both dmesg buffers.
> > 
> > Other way is to just control it by the define in the .c file; person
> > using this is by definition kernel hacker, anyway. Or this can simply
> > live like debugging patch, interested parties can apply.
> 
> That's why I made it a config option - so you can choose whether you
> want it or not.

Yes, and having it as a config option is completely wrong. There's no
good value for that config option, how is poor user going to answer
it. That config option is evil.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC/PATCH 1/2] swsusp: preserve boot-time printk output after resume

[Nigel Cunningham]

[-- Attachment #1.1: Type: text/plain, Size: 1272 bytes --]

Hi.

On Sat, 2007-04-28 at 00:05 +0200, Pavel Machek wrote:
> On Sat 2007-04-28 07:21:59, Nigel Cunningham wrote:
> > Hi.
> > 
> > On Fri, 2007-04-27 at 14:57 +0200, Pavel Machek wrote:
> > > On Fri 2007-04-27 15:52:12, Pekka J Enberg wrote:
> > > > On Fri, 27 Apr 2007, Pavel Machek wrote:
> > > > > 1. This should not be a config option
> > > > 
> > > > Ok.
> > > 
> > > Unfortunately, we loose dmesg from the state saving this way, so I do
> > > not think it can be hardcoded, either. Best way would be to somehow
> > > preserve both dmesg buffers.
> > > 
> > > Other way is to just control it by the define in the .c file; person
> > > using this is by definition kernel hacker, anyway. Or this can simply
> > > live like debugging patch, interested parties can apply.
> > 
> > That's why I made it a config option - so you can choose whether you
> > want it or not.
> 
> Yes, and having it as a config option is completely wrong. There's no
> good value for that config option, how is poor user going to answer
> it. That config option is evil.

It's an option for developers, and what's appropriate depends on which
part of the resume you're trying to debug. That said, I do agree that
preserving both buffers would be nicer.

Nigel

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]