[PATCH 0/3] changes about Call Trace:

[PATCH 0/3] changes about Call Trace:

[Akinobu Mita]

Hello,

I realized two things when I was porting my small script oops2line
to x86-64. (read call trace, find correspondance modules, calculate addr
and addr2line)

If I'm missing something, please let me know.

a) On x86-64 we get different Call Trace format than other architectures
   when we get oops or press SysRq-t:

   <ffffffffa008ef6c>{:jbd:kjournald+1030}

   There is a architecture independent function print_symbol().
   How about using it on x86-64? But it changes to:

   [<ffffffffa008ef6c>] kjournald+0x406/0x578 [jbd]

b) I can't find useful usage for the symbol size in print_symbol().
   And symbolsize seems to be fixed when vmlinux or modules are compiled.
   So we can calculate it from vmlinux or modules.
   How about removing the field of symbolsize in print_symbol()?

   [<ffffffffa008ef6c>] kjournald+0x406 [jbd]

[PATCH 1/3] makes print_symbol() return int

[Akinobu Mita]

This patch makes print_symbol() return the number of characters printed.

Signed-off-by: Akinobu Mita <mita@miraclelinux.com>
----
 include/linux/kallsyms.h |   17 ++++++++++-------
 kernel/kallsyms.c        |    4 ++--
 2 files changed, 12 insertions(+), 9 deletions(-)

--- 2.6-git.orig/include/linux/kallsyms.h	2006-01-03 12:21:10.000000000 +0900
+++ 2.6-git/include/linux/kallsyms.h	2006-01-11 14:02:57.578291640 +0900
@@ -20,7 +20,7 @@ const char *kallsyms_lookup(unsigned lon
 			    char **modname, char *namebuf);
 
 /* Replace "%s" in format with address, if found */
-extern void __print_symbol(const char *fmt, unsigned long address);
+extern int __print_symbol(const char *fmt, unsigned long address);
 
 #else /* !CONFIG_KALLSYMS */
 
@@ -38,7 +38,10 @@ static inline const char *kallsyms_looku
 }
 
 /* Stupid that this does nothing, but I didn't create this mess. */
-#define __print_symbol(fmt, addr)
+static inline int __print_symbol(const char *fmt, unsigned long addr)
+{
+	return 0;
+}
 #endif /*CONFIG_KALLSYMS*/
 
 /* This macro allows us to keep printk typechecking */
@@ -58,10 +61,10 @@ do {						\
 #define print_fn_descriptor_symbol(fmt, addr) print_symbol(fmt, addr)
 #endif
 
-#define print_symbol(fmt, addr)			\
-do {						\
-	__check_printsym_format(fmt, "");	\
-	__print_symbol(fmt, addr);		\
-} while(0)
+static inline int print_symbol(const char *fmt, unsigned long addr)
+{
+	__check_printsym_format(fmt, "");
+	return __print_symbol(fmt, addr);
+}
 
 #endif /*_LINUX_KALLSYMS_H*/
--- 2.6-git.orig/kernel/kallsyms.c	2006-01-03 12:21:10.000000000 +0900
+++ 2.6-git/kernel/kallsyms.c	2006-01-11 13:45:13.056123608 +0900
@@ -231,7 +231,7 @@ const char *kallsyms_lookup(unsigned lon
 }
 
 /* Replace "%s" in format with address, or returns -errno. */
-void __print_symbol(const char *fmt, unsigned long address)
+int __print_symbol(const char *fmt, unsigned long address)
 {
 	char *modname;
 	const char *name;
@@ -251,7 +251,7 @@ void __print_symbol(const char *fmt, uns
 		else
 			sprintf(buffer, "%s+%#lx/%#lx", name, offset, size);
 	}
-	printk(fmt, buffer);
+	return printk(fmt, buffer);
 }
 
 /* To avoid using get_symbol_offset for every symbol, we carry prefix along. */

Re: [PATCH 1/3] makes print_symbol() return int

[Jesper Juhl]

On 1/16/06, Akinobu Mita <mita@miraclelinux.com> wrote:
> This patch makes print_symbol() return the number of characters printed.
>
Why?
Who are the users of this?
If there are users who can bennefit, then where's the patch to make
them use this new return value?

--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [PATCH 1/3] makes print_symbol() return int

[Akinobu Mita]

On Mon, Jan 16, 2006 at 03:16:36PM +0100, Jesper Juhl wrote:
> On 1/16/06, Akinobu Mita <mita@miraclelinux.com> wrote:
> > This patch makes print_symbol() return the number of characters printed.
> >
> Why?
> Who are the users of this?
> If there are users who can bennefit, then where's the patch to make
> them use this new return value?

Please see 2/3

Re: [PATCH 1/3] makes print_symbol() return int

[Jesper Juhl]

On 1/16/06, Akinobu Mita <mita@miraclelinux.com> wrote:
> On Mon, Jan 16, 2006 at 03:16:36PM +0100, Jesper Juhl wrote:
> > On 1/16/06, Akinobu Mita <mita@miraclelinux.com> wrote:
> > > This patch makes print_symbol() return the number of characters printed.
> > >
> > Why?
> > Who are the users of this?
> > If there are users who can bennefit, then where's the patch to make
> > them use this new return value?
>
> Please see 2/3
>
Ahh, for some reason I have not recieved that patch via LKML, but I
found it in the archives.
Thanks.

--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

[PATCH 2/3] use usual call trace format on x86-64

[Akinobu Mita]

Use print_symbol() to dump call trace.

Signed-off-by: Akinobu Mita <mita@miraclelinux.com>
----
 traps.c |   29 +++++++----------------------
 1 files changed, 7 insertions(+), 22 deletions(-)

--- 2.6-mm/arch/x86_64/kernel/traps.c.orig	2006-01-08 00:49:46.000000000 +0900
+++ 2.6-mm/arch/x86_64/kernel/traps.c	2006-01-08 00:54:07.000000000 +0900
@@ -30,6 +30,7 @@
 #include <linux/moduleparam.h>
 #include <linux/nmi.h>
 #include <linux/kprobes.h>
+#include <linux/kallsyms.h> 
 
 #include <asm/system.h>
 #include <asm/uaccess.h>
@@ -93,30 +94,14 @@ static inline void conditional_sti(struc
 
 static int kstack_depth_to_print = 10;
 
-#ifdef CONFIG_KALLSYMS
-#include <linux/kallsyms.h> 
-int printk_address(unsigned long address)
-{ 
-	unsigned long offset = 0, symsize;
-	const char *symname;
-	char *modname;
-	char *delim = ":"; 
-	char namebuf[128];
-
-	symname = kallsyms_lookup(address, &symsize, &offset, &modname, namebuf); 
-	if (!symname) 
-		return printk("[<%016lx>]", address);
-	if (!modname) 
-		modname = delim = ""; 		
-        return printk("<%016lx>{%s%s%s%s%+ld}",
-		      address,delim,modname,delim,symname,offset); 
-} 
-#else
 int printk_address(unsigned long address)
 { 
-	return printk("[<%016lx>]", address);
-} 
-#endif
+	int len;
+
+	len = printk("[<%016lx>]", address);
+	len += print_symbol(" %s", address);
+	return len;
+}
 
 static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
 					unsigned *usedp, const char **idp)

Re: [PATCH 2/3] use usual call trace format on x86-64

[Andi Kleen]

On Monday 16 January 2006 13:18, Akinobu Mita wrote:
> Use print_symbol() to dump call trace.

Rejected.

-Andi

Re: [PATCH 2/3] use usual call trace format on x86-64

[Christoph Hellwig]

On Mon, Jan 16, 2006 at 01:22:41PM +0100, Andi Kleen wrote:
> On Monday 16 January 2006 13:18, Akinobu Mita wrote:
> > Use print_symbol() to dump call trace.
> 
> Rejected.

Why?  This is a lot more readable than what's there, and similar to other
architectures.

Re: [PATCH 2/3] use usual call trace format on x86-64

[Andi Kleen]

On Monday 16 January 2006 13:32, Christoph Hellwig wrote:
> On Mon, Jan 16, 2006 at 01:22:41PM +0100, Andi Kleen wrote:
> > On Monday 16 January 2006 13:18, Akinobu Mita wrote:
> > > Use print_symbol() to dump call trace.
> > 
> > Rejected.
> 
> Why?  This is a lot more readable than what's there, and similar to other
> architectures.

See my reply to 0/0

-Andi

[PATCH 3/3] omit symbol size field in print_symbol()

[Akinobu Mita]

I can't find useful usage for the symbol size in print_symbol().
And symbolsize seems to be fixed when vmlinux or modules are compiled.
So we can calculate it from vmlinux or modules.

Signed-off-by: Akinobu Mita <mita@miraclelinux.com>
----
 kallsyms.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

--- 2.6-git/kernel/kallsyms.c.orig	2006-01-11 14:50:37.000000000 +0900
+++ 2.6-git/kernel/kallsyms.c	2006-01-11 14:52:23.000000000 +0900
@@ -246,10 +246,9 @@ int __print_symbol(const char *fmt, unsi
 		sprintf(buffer, "0x%lx", address);
 	else {
 		if (modname)
-			sprintf(buffer, "%s+%#lx/%#lx [%s]", name, offset,
-				size, modname);
+			sprintf(buffer, "%s+%#lx [%s]", name, offset, modname);
 		else
-			sprintf(buffer, "%s+%#lx/%#lx", name, offset, size);
+			sprintf(buffer, "%s+%#lx", name, offset);
 	}
 	return printk(fmt, buffer);
 }

Re: [PATCH 3/3] omit symbol size field in print_symbol()

[Andi Kleen]

On Monday 16 January 2006 13:18, Akinobu Mita wrote:
> I can't find useful usage for the symbol size in print_symbol().
> And symbolsize seems to be fixed when vmlinux or modules are compiled.
> So we can calculate it from vmlinux or modules.

Also not applied.

-Andi

Re: [PATCH 3/3] omit symbol size field in print_symbol()

[Arjan van de Ven]

On Mon, 2006-01-16 at 21:18 +0900, Akinobu Mita wrote:
> I can't find useful usage for the symbol size in print_symbol().
> And symbolsize seems to be fixed when vmlinux or modules are compiled.
> So we can calculate it from vmlinux or modules.


the use is that you can see if the EIP actually is inside the function,
or if the decoder is going bonkers. Quite useful feature that...

Re: [PATCH 3/3] omit symbol size field in print_symbol()

[Akinobu Mita]

On Mon, Jan 16, 2006 at 03:37:01PM +0100, Arjan van de Ven wrote:
> On Mon, 2006-01-16 at 21:18 +0900, Akinobu Mita wrote:
> > I can't find useful usage for the symbol size in print_symbol().
> > And symbolsize seems to be fixed when vmlinux or modules are compiled.
> > So we can calculate it from vmlinux or modules.
> 
> 
> the use is that you can see if the EIP actually is inside the function,
> or if the decoder is going bonkers. Quite useful feature that...
> 

If it is really useful, should we have it on x86-64 too?
The patches 1/3 and 2/3 will do that.
Andi Kleen doesn't want because The call trace on x86-64 could have more
than one symbol per line, but my patches will print more characters than
now.

Re: [PATCH 3/3] omit symbol size field in print_symbol()

[Akinobu Mita]

On Mon, Jan 16, 2006 at 03:37:01PM +0100, Arjan van de Ven wrote:
> On Mon, 2006-01-16 at 21:18 +0900, Akinobu Mita wrote:
> > I can't find useful usage for the symbol size in print_symbol().
> > And symbolsize seems to be fixed when vmlinux or modules are compiled.
> > So we can calculate it from vmlinux or modules.
> 
> 
> the use is that you can see if the EIP actually is inside the function,
> or if the decoder is going bonkers. Quite useful feature that...
> 

I still cannot understand the importance of symbolsize in print_symbol()
very well. When is the decoder going bonkers for example?
Isn't it the task for kallsyms itself to detect that bonkers?

Re: [PATCH 0/3] changes about Call Trace:

[Andi Kleen]

On Monday 16 January 2006 13:16, Akinobu Mita wrote:
> If I'm missing something, please let me know.
> 
> a) On x86-64 we get different Call Trace format than other architectures
>    when we get oops or press SysRq-t:
> 
>    <ffffffffa008ef6c>{:jbd:kjournald+1030}
> 
>    There is a architecture independent function print_symbol().
>    How about using it on x86-64? But it changes to:
> 
>    [<ffffffffa008ef6c>] kjournald+0x406/0x578 [jbd]

The x86-64 format is more compact.

> b) I can't find useful usage for the symbol size in print_symbol().
>    And symbolsize seems to be fixed when vmlinux or modules are compiled.
>    So we can calculate it from vmlinux or modules.
>    How about removing the field of symbolsize in print_symbol()?
> 
>    [<ffffffffa008ef6c>] kjournald+0x406 [jbd]

It's a double check that the oops is matching the vmlinux you're looking 
at.

-Andi

Re: [PATCH 0/3] changes about Call Trace:

[Akinobu Mita]

On Mon, Jan 16, 2006 at 01:22:11PM +0100, Andi Kleen wrote:
> On Monday 16 January 2006 13:16, Akinobu Mita wrote:
> > If I'm missing something, please let me know.
> > 
> > a) On x86-64 we get different Call Trace format than other architectures
> >    when we get oops or press SysRq-t:
> > 
> >    <ffffffffa008ef6c>{:jbd:kjournald+1030}
> > 
> >    There is a architecture independent function print_symbol().
> >    How about using it on x86-64? But it changes to:
> > 
> >    [<ffffffffa008ef6c>] kjournald+0x406/0x578 [jbd]
> 
> The x86-64 format is more compact.

How about this update?

1/3: change from "[<...>]" to  "<...>".
2/3: change the format of offset from hexadecimal to decimal in.

Re: [PATCH 0/3] changes about Call Trace:

[Akinobu Mita]

Use print_symbol() to dump call trace.
Signed-off-by: Akinobu Mita <mita@miraclelinux.com>

--- 2.6-git/arch/x86_64/kernel/traps.c.orig	2006-01-16 22:05:38.000000000 +0900
+++ 2.6-git/arch/x86_64/kernel/traps.c	2006-01-16 22:07:36.000000000 +0900
@@ -30,6 +30,7 @@
 #include <linux/moduleparam.h>
 #include <linux/nmi.h>
 #include <linux/kprobes.h>
+#include <linux/kallsyms.h> 
 
 #include <asm/system.h>
 #include <asm/uaccess.h>
@@ -92,30 +93,14 @@ static inline void conditional_sti(struc
 
 static int kstack_depth_to_print = 10;
 
-#ifdef CONFIG_KALLSYMS
-#include <linux/kallsyms.h> 
-int printk_address(unsigned long address)
-{ 
-	unsigned long offset = 0, symsize;
-	const char *symname;
-	char *modname;
-	char *delim = ":"; 
-	char namebuf[128];
-
-	symname = kallsyms_lookup(address, &symsize, &offset, &modname, namebuf); 
-	if (!symname) 
-		return printk("[<%016lx>]", address);
-	if (!modname) 
-		modname = delim = ""; 		
-        return printk("<%016lx>{%s%s%s%s%+ld}",
-		      address,delim,modname,delim,symname,offset); 
-} 
-#else
 int printk_address(unsigned long address)
 { 
-	return printk("[<%016lx>]", address);
-} 
-#endif
+	int len;
+
+	len = printk("<%016lx>", address);
+	len += print_symbol(" %s", address);
+	return len;
+}
 
 static unsigned long *in_exception_stack(unsigned cpu, unsigned long stack,
 					unsigned *usedp, const char **idp)

Re: [PATCH 0/3] changes about Call Trace:

[Akinobu Mita]

remove symbolsize, and change offset format from hexadecimal to
decimal in print_symbol()

Signed-off-by: Akinobu Mita <mita@miraclelinux.com>

--- 2.6-git/kernel/kallsyms.c.orig	2006-01-16 22:06:16.000000000 +0900
+++ 2.6-git/kernel/kallsyms.c	2006-01-16 22:09:52.000000000 +0900
@@ -237,7 +237,7 @@ int __print_symbol(const char *fmt, unsi
 	const char *name;
 	unsigned long offset, size;
 	char namebuf[KSYM_NAME_LEN+1];
-	char buffer[sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN +
+	char buffer[sizeof("%s+%ld [%s]") + KSYM_NAME_LEN +
 		    2*(BITS_PER_LONG*3/10) + MODULE_NAME_LEN + 1];
 
 	name = kallsyms_lookup(address, &size, &offset, &modname, namebuf);
@@ -246,10 +246,9 @@ int __print_symbol(const char *fmt, unsi
 		sprintf(buffer, "0x%lx", address);
 	else {
 		if (modname)
-			sprintf(buffer, "%s+%#lx/%#lx [%s]", name, offset,
-				size, modname);
+			sprintf(buffer, "%s+%ld [%s]", name, offset, modname);
 		else
-			sprintf(buffer, "%s+%#lx/%#lx", name, offset, size);
+			sprintf(buffer, "%s+%ld", name, offset);
 	}
 	return printk(fmt, buffer);
 }

Re: [PATCH 0/3] changes about Call Trace:

[Andi Kleen]

On Monday 16 January 2006 14:41, Akinobu Mita wrote:
> On Mon, Jan 16, 2006 at 01:22:11PM +0100, Andi Kleen wrote:
> > On Monday 16 January 2006 13:16, Akinobu Mita wrote:
> > > If I'm missing something, please let me know.
> > > 
> > > a) On x86-64 we get different Call Trace format than other architectures
> > >    when we get oops or press SysRq-t:
> > > 
> > >    <ffffffffa008ef6c>{:jbd:kjournald+1030}
> > > 
> > >    There is a architecture independent function print_symbol().
> > >    How about using it on x86-64? But it changes to:
> > > 
> > >    [<ffffffffa008ef6c>] kjournald+0x406/0x578 [jbd]
> > 
> > The x86-64 format is more compact.
> 
> How about this update?
> 
> 1/3: change from "[<...>]" to  "<...>".
> 2/3: change the format of offset from hexadecimal to decimal in.

Can you please repost it in a fresh thread? I lost track of what was
the latest patch.

In general if you can make the call trace more compact without
losing information it's ok for me. Better wrapping sounds like
a promising approach.

-Andi

Re: [PATCH 0/3] changes about Call Trace:

[Jan Engelhardt]

>> > The x86-64 format is more compact.
>> 
>> How about this update?
>> 
>> 1/3: change from "[<...>]" to  "<...>".
>> 2/3: change the format of offset from hexadecimal to decimal in.

i386 should also get these two things.


Jan Engelhardt
-- 

Re: [PATCH 0/3] changes about Call Trace:

[Akinobu Mita]

On Mon, Jan 16, 2006 at 01:22:11PM +0100, Andi Kleen wrote:
> > b) I can't find useful usage for the symbol size in print_symbol().
> >    And symbolsize seems to be fixed when vmlinux or modules are compiled.
> >    So we can calculate it from vmlinux or modules.
> >    How about removing the field of symbolsize in print_symbol()?
> > 
> >    [<ffffffffa008ef6c>] kjournald+0x406 [jbd]
> 
> It's a double check that the oops is matching the vmlinux you're looking 
> at.

If we add system_utsname.version in oops so that we can compare with
linux_banner[] in vmlinux, Will it be more precise and easier way to
double check than checking symbolsize?

--- 2.6-git/arch/i386/kernel/traps.c.orig	2006-01-17 12:45:41.000000000 +0900
+++ 2.6-git/arch/i386/kernel/traps.c	2006-01-17 12:49:35.000000000 +0900
@@ -239,9 +239,10 @@ void show_registers(struct pt_regs *regs
 	}
 	print_modules();
 	printk(KERN_EMERG "CPU:    %d\nEIP:    %04x:[<%08lx>]    %s VLI\n"
-			"EFLAGS: %08lx   (%s) \n",
+			"EFLAGS: %08lx   (%s %s) \n",
 		smp_processor_id(), 0xffff & regs->xcs, regs->eip,
-		print_tainted(), regs->eflags, system_utsname.release);
+		print_tainted(), regs->eflags, system_utsname.release,
+		system_utsname.version);
 	print_symbol(KERN_EMERG "EIP is at %s\n", regs->eip);
 	printk(KERN_EMERG "eax: %08lx   ebx: %08lx   ecx: %08lx   edx: %08lx\n",
 		regs->eax, regs->ebx, regs->ecx, regs->edx);