Re: [PATCH 3/4] compact print_symbol() output

Re: [PATCH 3/4] compact print_symbol() output

[Chuck Ebbert]

In-Reply-To: <200601171601.52995.ak@suse.de>

On Tue, 17 Jan 2006 at 16:01:52 +0100, Andi Kleen wrote:

> On Tuesday 17 January 2006 16:01, Hugh Dickins wrote:
> 
> > I've often found symbolsize useful.  Not when looking at an oops
> > from my own machine.  But when looking at an oops posted on LKML,
> > from someone who most likely has a different .config and different
> > compiler, different optimization and different inlining from mine.
> > symbolsize is a good clue as to how close their kernel is to the
> > one I've got built on my machine, how likely guesses I make based
> > on mine will apply to theirs, and whereabouts in the function that
> > it oopsed.
> 
> Yes that is why I want it too.

OK, how about this: remove the "0x" from the function size, i.e. print:

        kernel_symbol+0xd3/10e

instead of:

        kernel_symbol+0xd3/0x10e

This saves two characters per symbol and it should still be clear that
the second number is hexadecimal.

Does that break any tools?
-- 
Chuck

Re: [PATCH 3/4] compact print_symbol() output

[Keith Owens]

Chuck Ebbert (on Tue, 17 Jan 2006 22:05:27 -0500) wrote:
>In-Reply-To: <200601171601.52995.ak@suse.de>
>
>On Tue, 17 Jan 2006 at 16:01:52 +0100, Andi Kleen wrote:
>
>> On Tuesday 17 January 2006 16:01, Hugh Dickins wrote:
>> 
>> > I've often found symbolsize useful.  Not when looking at an oops
>> > from my own machine.  But when looking at an oops posted on LKML,
>> > from someone who most likely has a different .config and different
>> > compiler, different optimization and different inlining from mine.
>> > symbolsize is a good clue as to how close their kernel is to the
>> > one I've got built on my machine, how likely guesses I make based
>> > on mine will apply to theirs, and whereabouts in the function that
>> > it oopsed.
>> 
>> Yes that is why I want it too.
>
>OK, how about this: remove the "0x" from the function size, i.e. print:
>
>        kernel_symbol+0xd3/10e
>
>instead of:
>
>        kernel_symbol+0xd3/0x10e
>
>This saves two characters per symbol and it should still be clear that
>the second number is hexadecimal.
>
>Does that break any tools?

Not, just CONFUSE-A-HUMAN (incorporating AMAZE-A-VOLE LTD, STUN-A-STOAT
LTD, PUZZLE-A-PUMA LTD, STARTLE-A-THOMPSON'S GAZELLE LTD,
BEWILDEREBEEST INC, DISTRACT-A-BEE).  Yes, I need a life.

Re: [PATCH 3/4] compact print_symbol() output

[Valdis.Kletnieks]

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

On Tue, 17 Jan 2006 22:05:27 EST, Chuck Ebbert said:

> OK, how about this: remove the "0x" from the function size, i.e. print:
> 
>         kernel_symbol+0xd3/10e
> 
> instead of:
> 
>         kernel_symbol+0xd3/0x10e
> 
> This saves two characters per symbol and it should still be clear that
> the second number is hexadecimal.

Good.  Now repeat for a function that's 6 bytes shorter.

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH 3/4] compact print_symbol() output

[Chuck Ebbert]

In-Reply-To: <200601180325.k0I3P8tF008591@turing-police.cc.vt.edu>

On Tue, 17 Jan 2006 at 22:25:07 -0500, Valdis.Kletnieks wrote:

> On Tue, 17 Jan 2006 22:05:27 EST, Chuck Ebbert said:
> 
> > OK, how about this: remove the "0x" from the function size, i.e. print:
> > 
> >         kernel_symbol+0xd3/10e
> > 
> > instead of:
> > 
> >         kernel_symbol+0xd3/0x10e
> > 
> > This saves two characters per symbol and it should still be clear that
> > the second number is hexadecimal.
>
> Good.  Now repeat for a function that's 6 bytes shorter.

OK, I probably should have done that:

        kernel_symbol+0xd3/108

My point is that if the "numerator" is hex you should assume the
"denominator" is too.
-- 
Chuck
Currently reading: _Einstein's Bridge_ by John Cramer

Re: [PATCH 3/4] compact print_symbol() output

[Andi Kleen]

On Tuesday 17 January 2006 16:01, Hugh Dickins wrote:

> I've often found symbolsize useful.  Not when looking at an oops
> from my own machine.  But when looking at an oops posted on LKML,
> from someone who most likely has a different .config and different
> compiler, different optimization and different inlining from mine.
> symbolsize is a good clue as to how close their kernel is to the
> one I've got built on my machine, how likely guesses I make based
> on mine will apply to theirs, and whereabouts in the function that
> it oopsed.

Yes that is why I want it too.

-Andi

Re: [PATCH 3/4] compact print_symbol() output

[Hugh Dickins]

On Tue, 17 Jan 2006, Akinobu Mita wrote:
> 
> And do you have any objection to remove symbolsize output in
> print_symbol()? I can't find useful usage about symbolsize in
> print_symbol() except to do a double check that the oops is
> matching the vmlinux we're looking at. (so I made 4/4)
> Do you know any useful usage about symbolsize?

I've often found symbolsize useful.  Not when looking at an oops
from my own machine.  But when looking at an oops posted on LKML,
from someone who most likely has a different .config and different
compiler, different optimization and different inlining from mine.
symbolsize is a good clue as to how close their kernel is to the
one I've got built on my machine, how likely guesses I make based
on mine will apply to theirs, and whereabouts in the function that
it oopsed.

Hugh

Re: [PATCH 3/4] compact print_symbol() output

[Akinobu Mita]

On Tue, Jan 17, 2006 at 09:34:03PM +1100, Keith Owens wrote:
> Akinobu Mita (on Tue, 17 Jan 2006 19:15:55 +0900) wrote:
> >- remove symbolsize field
> >- change offset format from hexadecimal to decimal
> 
> That is silly.  Almost every binutils tool prints offsets in hex,
> including objdump and gdb.  Printing the trace offset in decimal just
> makes more work for users to convert back to decimal to match up with
> all the other tools.

Currently call trace on x86-64 prints offset in decimal.
Do you think it is better to print offset in hex on x86-64 too?
But Andi Kleen said he likes compact call trace in the reply to
my first version of these patches.

And do you have any objection to remove symbolsize output in
print_symbol()? I can't find useful usage about symbolsize in
print_symbol() except to do a double check that the oops is
matching the vmlinux we're looking at. (so I made 4/4)
Do you know any useful usage about symbolsize?

Re: [PATCH 3/4] compact print_symbol() output

[Keith Owens]

Akinobu Mita (on Tue, 17 Jan 2006 19:58:27 +0900) wrote:
>On Tue, Jan 17, 2006 at 11:52:19AM +0100, Jesper Juhl wrote:
>> On 1/17/06, Keith Owens <kaos@ocs.com.au> wrote:
>> > Akinobu Mita (on Tue, 17 Jan 2006 19:15:55 +0900) wrote:
>> > >- remove symbolsize field
>> > >- change offset format from hexadecimal to decimal
>> >
>> > That is silly.  Almost every binutils tool prints offsets in hex,
>> > including objdump and gdb.  Printing the trace offset in decimal just
>> > makes more work for users to convert back to decimal to match up with
>> > all the other tools.
>> >
>> Agreed.
>> Also, hex output is shorter and often more natural for this type of data.
>> 
>
>In my vmlinux, 99.9% of the functions are smaller than 10000 bytes.
>
>10000 == 0x2710
>So. strlen("10000") == 5 < strlen("0x2710") == 6
>Therefore call trace must be more compact.

Which is irrelevant when your change makes more work for everybody who
reads the trace.

Re: [PATCH 3/4] compact print_symbol() output

[Akinobu Mita]

On Tue, Jan 17, 2006 at 11:52:19AM +0100, Jesper Juhl wrote:
> On 1/17/06, Keith Owens <kaos@ocs.com.au> wrote:
> > Akinobu Mita (on Tue, 17 Jan 2006 19:15:55 +0900) wrote:
> > >- remove symbolsize field
> > >- change offset format from hexadecimal to decimal
> >
> > That is silly.  Almost every binutils tool prints offsets in hex,
> > including objdump and gdb.  Printing the trace offset in decimal just
> > makes more work for users to convert back to decimal to match up with
> > all the other tools.
> >
> Agreed.
> Also, hex output is shorter and often more natural for this type of data.
> 

In my vmlinux, 99.9% of the functions are smaller than 10000 bytes.

10000 == 0x2710
So. strlen("10000") == 5 < strlen("0x2710") == 6
Therefore call trace must be more compact.

Re: [PATCH 3/4] compact print_symbol() output

[Jesper Juhl]

On 1/17/06, Keith Owens <kaos@ocs.com.au> wrote:
> Akinobu Mita (on Tue, 17 Jan 2006 19:15:55 +0900) wrote:
> >- remove symbolsize field
> >- change offset format from hexadecimal to decimal
>
> That is silly.  Almost every binutils tool prints offsets in hex,
> including objdump and gdb.  Printing the trace offset in decimal just
> makes more work for users to convert back to decimal to match up with
> all the other tools.
>
Agreed.
Also, hex output is shorter and often more natural for this type of data.

--
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 3/4] compact print_symbol() output

[Keith Owens]

Akinobu Mita (on Tue, 17 Jan 2006 19:15:55 +0900) wrote:
>- remove symbolsize field
>- change offset format from hexadecimal to decimal

That is silly.  Almost every binutils tool prints offsets in hex,
including objdump and gdb.  Printing the trace offset in decimal just
makes more work for users to convert back to decimal to match up with
all the other tools.

[PATCH 3/4] compact print_symbol() output

[Akinobu Mita]

- remove symbolsize field
- change offset format from hexadecimal to decimal

99.9% of the functions in my vmlinux are smaller than 10000 bytes.
Therefore call trace must be more compact.

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

 kallsyms.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

--- 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);
 }