[patch 2.6.15-current] i386: multi-column stack backtraces

[patch 2.6.15-current] i386: multi-column stack backtraces

[Chuck Ebbert]

Print stack backtraces in multiple columns, saving screen space.
Number of columns is configurable and defaults to one so 
behavior is backwards-compatible.

Also removes the brackets around addresses when printing more
that one entry per line so they print as:
    <address>
instead of:
    [<address>]
This helps multiple entries fit better on one line.

Original idea by Dave Jones, taken from x86_64.

Signed-Off-By: Chuck Ebbert <76306.1226@compuserve.com>

 arch/i386/Kconfig.debug  |    9 +++++++++
 arch/i386/kernel/traps.c |   32 +++++++++++++++++++++++++++-----
 2 files changed, 36 insertions(+), 5 deletions(-)

--- 2.6.15a.orig/arch/i386/kernel/traps.c
+++ 2.6.15a/arch/i386/kernel/traps.c
@@ -112,12 +112,30 @@ static inline int valid_stack_ptr(struct
 		p < (void *)tinfo + THREAD_SIZE - 3;
 }
 
-static inline void print_addr_and_symbol(unsigned long addr, char *log_lvl)
+/*
+ * Print CONFIG_STACK_BACKTRACE_COLS address/symbol entries per line.
+ */
+static inline int print_addr_and_symbol(unsigned long addr, char *log_lvl,
+					int printed)
 {
-	printk(log_lvl);
+	if (!printed)
+		printk(log_lvl);
+
+#if CONFIG_STACK_BACKTRACE_COLS == 1
 	printk(" [<%08lx>] ", addr);
+#else
+	printk(" <%08lx> ", addr);
+#endif
 	print_symbol("%s", addr);
-	printk("\n");
+
+	printed = (printed + 1) % CONFIG_STACK_BACKTRACE_COLS;
+
+	if (printed)
+		printk("  ");
+	else
+		printk("\n");
+
+	return printed;
 }
 
 static inline unsigned long print_context_stack(struct thread_info *tinfo,
@@ -125,20 +143,24 @@ static inline unsigned long print_contex
 				char *log_lvl)
 {
 	unsigned long addr;
+	int printed = 0; /* nr of entries already printed on current line */
 
 #ifdef	CONFIG_FRAME_POINTER
 	while (valid_stack_ptr(tinfo, (void *)ebp)) {
 		addr = *(unsigned long *)(ebp + 4);
-		print_addr_and_symbol(addr, log_lvl);
+		printed = print_addr_and_symbol(addr, log_lvl, printed);
 		ebp = *(unsigned long *)ebp;
 	}
 #else
 	while (valid_stack_ptr(tinfo, stack)) {
 		addr = *stack++;
 		if (__kernel_text_address(addr))
-			print_addr_and_symbol(addr, log_lvl);
+			printed = print_addr_and_symbol(addr, log_lvl, printed);
 	}
 #endif
+	if (printed)
+		printk("\n");
+
 	return ebp;
 }
 
--- 2.6.15a.orig/arch/i386/Kconfig.debug
+++ 2.6.15a/arch/i386/Kconfig.debug
@@ -31,6 +31,15 @@ config DEBUG_STACK_USAGE
 
 	  This option will slow down process creation somewhat.
 
+config STACK_BACKTRACE_COLS
+	int "Stack backtraces per line" if DEBUG_KERNEL
+	range 1 3
+	default 1
+	help
+	  Selects how many stack backtrace entries per line to display.
+
+	  This can save screen space when displaying traces.
+
 comment "Page alloc debug is incompatible with Software Suspend on i386"
 	depends on DEBUG_KERNEL && SOFTWARE_SUSPEND
 
-- 
Chuck
Currently reading: _Einstein's Bridge_ by John Cramer

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Dave Jones]

On Tue, Jan 17, 2006 at 01:23:26AM -0500, Chuck Ebbert wrote:
 > Print stack backtraces in multiple columns, saving screen space.
 > Number of columns is configurable and defaults to one so 
 > behavior is backwards-compatible.

The CONFIG option seems redundant to me, but..

Signed-off-by: Dave Jones <davej@redhat.com>

		Dave

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Andrew Morton]

Chuck Ebbert <76306.1226@compuserve.com> wrote:
>
> Print stack backtraces in multiple columns, saving screen space.
> Number of columns is configurable and defaults to one so 
> behavior is backwards-compatible.
> 
> Also removes the brackets around addresses when printing more
> that one entry per line so they print as:
>     <address>
> instead of:
>     [<address>]
> This helps multiple entries fit better on one line.
> 
> Original idea by Dave Jones, taken from x86_64.
> 

Presumably this is going to bust ksymoops.  Also the various other custom
oops-parsers which people have written themselves.

> +config STACK_BACKTRACE_COLS

It's pretty sad to go and make something like this a config option.  But
given that it is, believe it or not, an exported-to-userspace interface, I
guess there's not much choice.

The patch is a desirable change (I do get seasick reading x86_64 traces,
but I'll get over it), but it'll cause various bits of downstream grief.

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Dave Jones]

On Mon, Jan 16, 2006 at 10:42:34PM -0800, Andrew Morton wrote:

 > Presumably this is going to bust ksymoops.
 
Do people actually still use ksymoops for 2.6 kernels ?

I resorted to it about 6 months ago for the first time in the
better part of 3 years, and it didn't even compile.
(I only wanted to disassemble a Code: line, addr resolution
 works for me [and most other distros afaik] with kksymoops now)

 > Also the various other custom
 > oops-parsers which people have written themselves.

Given we've extended the oops output in several different
ways without thought in the past, it seems a bit late.
We added printing of module list in the middle of the output.
We added various tainting flags over time.

What other tools parse oopses ? ksymoops is the only one I recall.

 > The patch is a desirable change (I do get seasick reading x86_64 traces,
 > but I'll get over it), but it'll cause various bits of downstream grief.

I'd be surprised if anyone noticed.

*shrug*, in an ideal world, no-one would ever see an oops anyway :-P
 
		Dave

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Andrew Morton]

Dave Jones <davej@redhat.com> wrote:
>
> On Mon, Jan 16, 2006 at 10:42:34PM -0800, Andrew Morton wrote:
> 
>  > Presumably this is going to bust ksymoops.
>  
> Do people actually still use ksymoops for 2.6 kernels ?

They do if they've disabled kallsyms...

>
> What other tools parse oopses ? ksymoops is the only one I recall.
> 

Embedded systems dink with the output, log it to nvram, etc.  Custom stuff.

I think we can live with the breakage, but let's get Keith's input.

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Keith Owens]

Andrew Morton (on Mon, 16 Jan 2006 22:42:34 -0800) wrote:
>Chuck Ebbert <76306.1226@compuserve.com> wrote:
>>
>> Print stack backtraces in multiple columns, saving screen space.
>> Number of columns is configurable and defaults to one so 
>> behavior is backwards-compatible.
>> 
>> Also removes the brackets around addresses when printing more
>> that one entry per line so they print as:
>>     <address>
>> instead of:
>>     [<address>]
>> This helps multiple entries fit better on one line.
>> 
>> Original idea by Dave Jones, taken from x86_64.
>> 
>
>Presumably this is going to bust ksymoops.  Also the various other custom
>oops-parsers which people have written themselves.

Should not be a problem for ksymoops.  Most entries use this regex,
where [ ] is optional.

#define BRACKETED_ADDRESS       "\\[*<([0-9a-fA-F]{4,})>\\]* *"

Printing multiple addresses (with our without [ ]) plus their symbols
on the same line will stop ksymoops processing at the first symbol
name, but who cares?  If you can print a symbol then you already have
kallsyms and you do not need ksymoops.  If you do not have kallsyms
then the output is just multiple addresses on one line, which ksymoops
already handles.

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Keith Owens]

Dave Jones (on Tue, 17 Jan 2006 02:58:42 -0500) wrote:
>On Mon, Jan 16, 2006 at 10:42:34PM -0800, Andrew Morton wrote:
>
> > Presumably this is going to bust ksymoops.
> 
>Do people actually still use ksymoops for 2.6 kernels ?
>
>I resorted to it about 6 months ago for the first time in the
>better part of 3 years, and it didn't even compile.

It compiles for me.  gcc version 4.0.2 20050901 (prerelease) (SUSE
Linux).  There are a few warnings about pointer signedness but that is
all.

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Jan Engelhardt]

> > Presumably this is going to bust ksymoops.
> 
>Do people actually still use ksymoops for 2.6 kernels ?

I think it was said often enough that people _should not_ use it for 2.6. 
Unfortunately, there are still some who do.



Jan Engelhardt
-- 

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Sam Ravnborg]

On Tue, Jan 17, 2006 at 09:22:09PM +1100, Keith Owens wrote:
> >
> >Presumably this is going to bust ksymoops.  Also the various other custom
> >oops-parsers which people have written themselves.
> 
> Should not be a problem for ksymoops.  Most entries use this regex,
> where [ ] is optional.

In that case can we then remove the CONFIG option?
If needed to be configurable a commandline option could do it,
so one does not have to rebuild the kernel.

	Sam

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Keith Owens]

Jan Engelhardt (on Tue, 17 Jan 2006 18:12:20 +0100 (MET)) wrote:
>> > Presumably this is going to bust ksymoops.
>> 
>>Do people actually still use ksymoops for 2.6 kernels ?
>
>I think it was said often enough that people _should not_ use it for 2.6. 
>Unfortunately, there are still some who do.

Some people use ksymoops because the kernel oops does not decode the
instructions.  Some people use ksymoops because they run embedded
systems and cannot afford the kallsyms data.  Others just use ksymoops
out of habit.

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Pavel Machek]

Hi!

> > Print stack backtraces in multiple columns, saving screen space.
> > Number of columns is configurable and defaults to one so 
> > behavior is backwards-compatible.
> > 
> > Also removes the brackets around addresses when printing more
> > that one entry per line so they print as:
> >     <address>
> > instead of:
> >     [<address>]
> > This helps multiple entries fit better on one line.
> > 
> > Original idea by Dave Jones, taken from x86_64.
> > 
> 
> Presumably this is going to bust ksymoops.  Also the various other custom
> oops-parsers which people have written themselves.
> 
> > +config STACK_BACKTRACE_COLS
> 
> It's pretty sad to go and make something like this a config option.  But
> given that it is, believe it or not, an exported-to-userspace interface, I
> guess there's not much choice.

I don't think we should call printk()s "userspace interface". Yes,
someone may do dmesg | custom-grep-to-find-all-the-hardware, but
thats clearly stupid. I also believe that oops dump is so closely tied
to kernel that it is fair to change... plus this should better not be
configurable, or userspace will have hard time parsing it.
								Pavel
-- 
Thanks, Sharp!

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Chuck Ebbert]

In-Reply-To: <20060118224732.GA1812@elf.ucw.cz>

On Wed, 18 Jan 2006, Pavel Machek wrote:

> I also believe that oops dump is so closely tied
> to kernel that it is fair to change... plus this should better not be
> configurable, or userspace will have hard time parsing it.

Userspace will still have to cope with the old format anyway.

I'll send a patch to change the default to two columns so the
new format gets some exposure, though.

-- 
Chuck
Currently reading: _Noise_ by Hal Clement

Re: [patch 2.6.15-current] i386: multi-column stack backtraces

[Chuck Ebbert]

In-Reply-To: <20060117181416.GB8047@mars.ravnborg.org>

On Tue, 17 Jan 2006 19:14:16 +0100, Sam Ravnborg wrote:

> On Tue, Jan 17, 2006 at 09:22:09PM +1100, Keith Owens wrote:
> > Should not be a problem for ksymoops.  Most entries use this regex,
> > where [ ] is optional.
> 
> In that case can we then remove the CONFIG option?
> If needed to be configurable a commandline option could do it,
> so one does not have to rebuild the kernel.

I guess it could be a command-line option, but how many times would
you change it?  Only traps.o get rebuilt when you change the config,
and that plus the kernel relink is pretty fast anyway.

-- 
Chuck