[PATCH] kernel/printk.c lockless access

[PATCH] kernel/printk.c lockless access

[Linas Vepstas]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]


Hi Linus, Andrew,

I was wondering if you could see your way to accepting the attached
patch, which provides access to the syslog buffer pointers.

The basic idea is that if a system has crashed, it can be handy to
be able to view the contents of the syslog buffer.  Unfortunately,
this is currently hard to do.

 --  char __log_buf[] is declared static in printk.c, so it cannot
     be found in the ksyms table.

 -- do_syslog() uses spinlocks to protect the data structure, and
    so will typically deadlock if called.

The 'fix' is to provide a routine that simply returns the pointers
to the log buffer.  

I'd be thrilled to have this patch accepted ... 

--linas

Signed-off-by: Linas Vepstas <linas@linas.org>



[-- Attachment #2: printk.patch --]
[-- Type: text/plain, Size: 940 bytes --]

===== kernel/printk.c 1.47 vs edited =====
--- 1.47/kernel/printk.c	2004-11-19 01:03:10 -06:00
+++ edited/kernel/printk.c	2005-01-06 13:44:36 -06:00
@@ -380,6 +380,23 @@ asmlinkage long sys_syslog(int type, cha
 	return do_syslog(type, buf, len);
 }
 
+#ifdef   CONFIG_DEBUG_KERNEL
+/**
+ * Its very handy to be able to view the syslog buffer during debug.
+ * But do_syslog() uses locks and so it will deadlock if called during 
+ * a debugging session. The routine provides the start and end of the 
+ * physical and logical logs, and is equivalent to do_syslog(3).
+ */
+
+void debugger_syslog_data(char *syslog_data[4])
+{
+	syslog_data[0] = log_buf;
+	syslog_data[1] = log_buf + __LOG_BUF_LEN;
+	syslog_data[2] = log_buf + log_end - (logged_chars < __LOG_BUF_LEN ? logged_chars : __LOG_BUF_LEN);
+	syslog_data[3] = log_buf + log_end;
+}
+#endif   /* CONFIG_DEBUG_KERNEL */
+
 /*
  * Call the console drivers on a range of log_buf
  */

Re: [PATCH] kernel/printk.c lockless access

[Andi Kleen]

Linas Vepstas <linas@austin.ibm.com> writes:

> The 'fix' is to provide a routine that simply returns the pointers
> to the log buffer.  

Better&simpler fix would be to just unstatic __log_buf
You cannot call such functions generally when looking at memory
dumps of some form.

-Andi

Re: [PATCH] kernel/printk.c lockless access

[Andrew Morton]

Linas Vepstas <linas@austin.ibm.com> wrote:
>
> I was wondering if you could see your way to accepting the attached
> patch, which provides access to the syslog buffer pointers.
> 
> The basic idea is that if a system has crashed, it can be handy to
> be able to view the contents of the syslog buffer.  Unfortunately,
> this is currently hard to do.
> 
>  --  char __log_buf[] is declared static in printk.c, so it cannot
>      be found in the ksyms table.
> 
>  -- do_syslog() uses spinlocks to protect the data structure, and
>     so will typically deadlock if called.
> 
> The 'fix' is to provide a routine that simply returns the pointers
> to the log buffer.  
> 
> I'd be thrilled to have this patch accepted ... 

We faced the same problem in the Digeo kernel.  When the kernel oopses we
want to grab the last kilobyte or so of the printk buffer and stash it into
nvram.  We did this via a function which copies the data rather than
exporting all those variables, which I think is a nicer and more
maintainable approach:

/** get_printk_state - read the printk log buffer state
 *
 * @how_far_back: how many bytes back-in-time to look
 *
 * get_printk_state() gives the caller the current printk
 * log buffer head index, for later use by get_printk_buffer()
 * The index is offset by @how_far_back, which allows the caller
 * to look at previously-emitted information.  Negative values of
 * how_far_back don't make sense.
 */
int get_printk_state(int how_far_back)
{
	return (log_end - how_far_back) & LOG_BUF_MASK;
}

/** get_printk_buffer - read bytes from the printk buffer
 *
 * get_printk_buffer() will copy some bytes from the printk
 * ring buffer into the caller's buffer.
 *
 * @buf - the caller's buffer.  Bytes are written here.
 * @len - the amount of space at *@buf
 * @start_at - where in the log buffer to start copying out
 * data.  Presumably from a previous get_printk_state() call.
 *
 * get_printk_buffer() does not place a null at the end of *@buf
 *
 * get_printk_state() returns the number of bytes which it wrote
 * to *@buf.
 */
int get_printk_buffer(char *buf, int len, int start_at)
{
	int count;
	int at = start_at;

	for (count = 0; count < len; count++, at++) {
		if ((at & LOG_BUF_MASK) == (log_end & LOG_BUF_MASK))
			break;
		buf[count] = LOG_BUF(at);
	}
	return count;
}

(all available in a horrendous tarball at
http://www.digeo.com/assets/datasheets/digeo-mc1-2.0.8-mods.tar.bz2)

You want to cook up something like that?  (That's 2.4 code, and might need
work for 2.6 changes).

Re: [PATCH] kernel/printk.c lockless access

[Anton Blanchard]

 
> We faced the same problem in the Digeo kernel.  When the kernel oopses we
> want to grab the last kilobyte or so of the printk buffer and stash it into
> nvram.  We did this via a function which copies the data rather than
> exporting all those variables, which I think is a nicer and more
> maintainable approach:

Actually Id love to do this on ppc64 too. Its always difficult to get a
customer to remember to save away an oops report.

Anton

Re: [PATCH] kernel/printk.c lockless access

[Randy.Dunlap]

Anton Blanchard wrote:
>  
> 
>>We faced the same problem in the Digeo kernel.  When the kernel oopses we
>>want to grab the last kilobyte or so of the printk buffer and stash it into
>>nvram.  We did this via a function which copies the data rather than
>>exporting all those variables, which I think is a nicer and more
>>maintainable approach:
> 
> 
> Actually Id love to do this on ppc64 too. Its always difficult to get a
> customer to remember to save away an oops report.

We need /proc/kallsyms, /proc/modules, etc. also....
can you capture all of that for a complete oops/panic analysis?
(short of kdump, that is)

-- 
~Randy

Re: [PATCH] kernel/printk.c lockless access

[Alan Cox]

On Gwe, 2005-01-07 at 00:25, Randy.Dunlap wrote:
> > Actually Id love to do this on ppc64 too. Its always difficult to get a
> > customer to remember to save away an oops report.
> 
> We need /proc/kallsyms, /proc/modules, etc. also....
> can you capture all of that for a complete oops/panic analysis?
> (short of kdump, that is)

Ditto on x86 - several of us raised the ideal of ACPI actually defining
a "log area" in the E820 map types or some other ACPI resource that
would be a chunk of RAM used for logs that wasn't going to get bios
eaten on a soft reboot but could be reclaimed by the OS but we didn't
get it.

Re: [PATCH] kernel/printk.c lockless access

[Frank van Maarseveen]

On Fri, Jan 07, 2005 at 01:54:41AM +0000, Alan Cox wrote:
> 
> Ditto on x86 - several of us raised the ideal of ACPI actually defining
> a "log area" in the E820 map types or some other ACPI resource that
> would be a chunk of RAM used for logs that wasn't going to get bios
> eaten on a soft reboot but could be reclaimed by the OS but we didn't
> get it.

What about UDP (or just eth) broadcasting the oops and catching it
on another system? That would be useful if one has a lot of systems
(I have about 40) and makes it possible to immediately alert someone
without the need for ping games.

-- 
Frank

Re: [PATCH] kernel/printk.c lockless access

[David Wagner]

Frank van Maarseveen  wrote:
>What about UDP (or just eth) broadcasting the oops and catching it
>on another system?

For security reasons, it better be optional and disabled by default.
(That should be easy enough to ensure, so this is obviously not something
to stand in the way of doing something like this.)

Re: [PATCH] kernel/printk.c lockless access

[Alan Cox]

On Sul, 2005-01-09 at 10:44, Frank van Maarseveen wrote:
> What about UDP (or just eth) broadcasting the oops and catching it
> on another system? That would be useful if one has a lot of systems
> (I have about 40) and makes it possible to immediately alert someone
> without the need for ping games.

netdump and kgdb both can do this. The 2.6.10-tiny1 has some nice kgdb
patches that include using the polling network debug interfaces.

Re: [PATCH] kernel/printk.c lockless access

[Matt Mackall]

On Sun, Jan 09, 2005 at 11:00:48PM +0000, Alan Cox wrote:
> On Sul, 2005-01-09 at 10:44, Frank van Maarseveen wrote:
> > What about UDP (or just eth) broadcasting the oops and catching it
> > on another system? That would be useful if one has a lot of systems
> > (I have about 40) and makes it possible to immediately alert someone
> > without the need for ping games.
> 
> netdump and kgdb both can do this. The 2.6.10-tiny1 has some nice kgdb
> patches that include using the polling network debug interfaces.

Netconsole can sends oopses over UDP right now in mainline (since
2.6.3 or so?). And my kgdb over ethernet bits have been in -mm for
quite some time as well. 

(Though testing of -tiny is still appreciated of course.)

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] kernel/printk.c lockless access

[Keith Owens]

On Thu, 6 Jan 2005 13:58:12 -0600, 
Linas Vepstas <linas@austin.ibm.com> wrote:
>===== kernel/printk.c 1.47 vs edited =====
>--- 1.47/kernel/printk.c	2004-11-19 01:03:10 -06:00
>+++ edited/kernel/printk.c	2005-01-06 13:44:36 -06:00
>@@ -380,6 +380,23 @@ asmlinkage long sys_syslog(int type, cha
> 	return do_syslog(type, buf, len);
> }
> 
>+#ifdef   CONFIG_DEBUG_KERNEL
>+/**
>+ * Its very handy to be able to view the syslog buffer during debug.
>+ * But do_syslog() uses locks and so it will deadlock if called during 
>+ * a debugging session. The routine provides the start and end of the 
>+ * physical and logical logs, and is equivalent to do_syslog(3).
>+ */
>+
>+void debugger_syslog_data(char *syslog_data[4])
>+{
>+	syslog_data[0] = log_buf;
>+	syslog_data[1] = log_buf + __LOG_BUF_LEN;
>+	syslog_data[2] = log_buf + log_end - (logged_chars < __LOG_BUF_LEN ? logged_chars : __LOG_BUF_LEN);
>+	syslog_data[3] = log_buf + log_end;
>+}
>+#endif   /* CONFIG_DEBUG_KERNEL */
>+

Replace __LOG_BUF_LEN with log_buf_len.  __LOG_BUF_LEN is the default
size, log_buf_len is the actual size used, including boot time
overrides with log_buf_len=.

On Thu, 06 Jan 2005 21:50:17 +0100,
Andi Kleen <ak@muc.de> wrote:

AK>Better&simpler fix would be to just unstatic __log_buf

The debug caller needs to know where the ring pointers are as well.
Also __log_buf is not necessarily the current log buffer, boot with
log_buf_len= and you get a different buffer.  The caller needs the 

On Thu, 6 Jan 2005 16:12:41 -0800,
Andrew Morton <akpm@osdl.org> wrote:

AKPM>We faced the same problem in the Digeo kernel.  When the kernel oopses we
AKPM>want to grab the last kilobyte or so of the printk buffer and stash it into
AKPM>nvram.  We did this via a function which copies the data rather than
AKPM>exporting all those variables, which I think is a nicer and more
AKPM>maintainable approach:

That assumes that you have enough free space to copy the log buffer to.
In the general debugging case that is not true, especially if you want
the entire print buffer to be printed.  The only safe way to access the
complete print buffer is in situ.  Which is what the patch above does.