[patch] clean hex output of ftrace

[patch] clean hex output of ftrace

[trem]

Fix the output of ftrace in hex mode. Without this patch, the ouput of ftrace is :

raw mode : 6474 0 141531612444 0 140 + 6402 120 S
hex mode : 000091a4 00000000 000000023f1f50c1 00000000 c8 000000b2 00009120 87 ffff00c8 00000035

There is an inversion on ouput hex(6474) is 194a

Signed-off-by: Philippe Reynes <tremyfr@yahoo.fr>
---

Index: linux-2.6.26/kernel/trace/trace.c
===================================================================
--- linux-2.6.26/kernel/trace/trace.c
+++ linux-2.6.26/kernel/trace/trace.c   2008-10-08 22:30:29.000000000 +0200
@@ -415,8 +415,8 @@
 #endif
                byte = data[i];
 
-               hex[j++] = hex2asc[byte & 0x0f];
                hex[j++] = hex2asc[byte >> 4];
+               hex[j++] = hex2asc[byte & 0x0f];
        }
        hex[j++] = ' ';

Re: [patch] clean hex output of ftrace

[Joe Perches]

On Thu, 9 Oct 2008, trem wrote:
> Index: linux-2.6.26/kernel/trace/trace.c
> ===================================================================
> --- linux-2.6.26/kernel/trace/trace.c
> +++ linux-2.6.26/kernel/trace/trace.c   2008-10-08 22:30:29.000000000 +0200
> @@ -415,8 +415,8 @@
>  #endif
>                 byte = data[i];
>
> -               hex[j++] = hex2asc[byte & 0x0f];
>                 hex[j++] = hex2asc[byte >> 4];
> +               hex[j++] = hex2asc[byte & 0x0f];
>         }
>         hex[j++] = ' ';

I'm surprised Harvey Harrison hasn't changed it to pack_hex_byte
and removed the static.

Re: [patch] clean hex output of ftrace

[Harvey Harrison]

On Wed, 2008-10-08 at 15:22 -0700, Joe Perches wrote:
> I'm surprised Harvey Harrison hasn't changed it to pack_hex_byte
> and removed the static.
> 

From: Harvey Harrison <harvey.harrison@gmail.com>
Subject: [PATCH] ftrace: Fix inversion of hex output and use common routines

Fix the output of ftrace in hex mode as the hi/lo nibbles are output in
reverse order. Without this patch, the output of ftrace is:

raw mode : 6474 0 141531612444 0 140 + 6402 120 S
hex mode : 000091a4 00000000 000000023f1f50c1 00000000 c8 000000b2 00009120 87 ffff00c8 00000035

There is an inversion on ouput hex(6474) is 194a

[based on a patch by Philippe Reynes <tremyfr@yahoo.fr>]
Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 kernel/trace/trace.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8f3fb3d..763f763 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -396,14 +396,12 @@ trace_seq_putmem(struct trace_seq *s, void *mem, size_t len)
 }
 
 #define HEX_CHARS 17
-static const char hex2asc[] = "0123456789abcdef";
 
 static int
 trace_seq_putmem_hex(struct trace_seq *s, void *mem, size_t len)
 {
 	unsigned char hex[HEX_CHARS];
 	unsigned char *data = mem;
-	unsigned char byte;
 	int i, j;
 
 	BUG_ON(len >= HEX_CHARS);
@@ -413,10 +411,8 @@ trace_seq_putmem_hex(struct trace_seq *s, void *mem, size_t len)
 #else
 	for (i = len-1, j = 0; i >= 0; i--) {
 #endif
-		byte = data[i];
-
-		hex[j++] = hex2asc[byte & 0x0f];
-		hex[j++] = hex2asc[byte >> 4];
+		hex[j++] = hex_asc_hi(data[i]);
+		hex[j++] = hex_asc_lo(data[i]);
 	}
 	hex[j++] = ' ';
 
-- 
1.6.0.2.471.g47a76

Re: [patch] clean hex output of ftrace

[Harvey Harrison]

On Wed, 2008-10-08 at 17:16 -0700, Joe Perches wrote:
> On Wed, 8 Oct 2008, Harvey Harrison wrote:
> > From: Harvey Harrison <harvey.harrison@gmail.com>
> > Subject: [PATCH] ftrace: Fix inversion of hex output and use common routines
> 
> You're quick...
> 
> >  	BUG_ON(len >= HEX_CHARS);
> 
> I think the BUG_ON is senseless.
> 

Agreed, it probably meant to say BUG_ON(len * 2 > HEX_CHARS -1)

But as this is only called from within a helper macro, it could be
changed to a build-time check instead of a runtime:

From: Harvey Harrison <harvey.harrison@gmail.com>
Subject: [PATCH] trace: add build-time check to avoid overrunning hex buffer

Remove the runtime BUG_ON and change to a compile-time check in
the macro that calls the hex format routine

[Noticed by Joe Perches]
Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 kernel/trace/trace.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 763f763..1ffbc24 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -395,7 +395,8 @@ trace_seq_putmem(struct trace_seq *s, void *mem, size_t len)
 	return len;
 }
 
-#define HEX_CHARS 17
+#define MAX_MEMHEX_BYTES	(8)
+#define HEX_CHARS		((MAX_MEMHEX_BYTES * 2) + 1)
 
 static int
 trace_seq_putmem_hex(struct trace_seq *s, void *mem, size_t len)
@@ -404,8 +405,6 @@ trace_seq_putmem_hex(struct trace_seq *s, void *mem, size_t len)
 	unsigned char *data = mem;
 	int i, j;
 
-	BUG_ON(len >= HEX_CHARS);
-
 #ifdef __BIG_ENDIAN
 	for (i = 0, j = 0; i < len; i++) {
 #else
@@ -1706,6 +1705,7 @@ do {							\
 
 #define SEQ_PUT_HEX_FIELD_RET(s, x)			\
 do {							\
+	BUILD_BUG_ON(sizeof(x) > MAX_MEMHEX_BYTES);	\
 	if (!trace_seq_putmem_hex(s, &(x), sizeof(x)))	\
 		return 0;				\
 } while (0)
-- 
1.6.0.2.471.g47a76

Re: [patch] clean hex output of ftrace

[Ingo Molnar]

* Harvey Harrison <harvey.harrison@gmail.com> wrote:

> On Wed, 2008-10-08 at 17:16 -0700, Joe Perches wrote:
> > On Wed, 8 Oct 2008, Harvey Harrison wrote:
> > > From: Harvey Harrison <harvey.harrison@gmail.com>
> > > Subject: [PATCH] ftrace: Fix inversion of hex output and use common routines
> > 
> > You're quick...
> > 
> > >  	BUG_ON(len >= HEX_CHARS);
> > 
> > I think the BUG_ON is senseless.
> > 
> 
> Agreed, it probably meant to say BUG_ON(len * 2 > HEX_CHARS -1)
> 
> But as this is only called from within a helper macro, it could be
> changed to a build-time check instead of a runtime:
> 
> From: Harvey Harrison <harvey.harrison@gmail.com>
> Subject: [PATCH] trace: add build-time check to avoid overrunning hex buffer

applied to tip/tracing/core:

 bb86ba7: trace: add build-time check to avoid overrunning hex buffer
 32ce86f: ftrace: fix hex output mode of ftrace

thanks guys!

	Ingo

Re: [patch] clean hex output of ftrace

[Harvey Harrison]

On Thu, 2008-10-09 at 14:31 +0200, Ingo Molnar wrote:
> * Harvey Harrison <harvey.harrison@gmail.com> wrote:
> 
> > On Wed, 2008-10-08 at 17:16 -0700, Joe Perches wrote:
> > > On Wed, 8 Oct 2008, Harvey Harrison wrote:
> > > > From: Harvey Harrison <harvey.harrison@gmail.com>
> > > > Subject: [PATCH] ftrace: Fix inversion of hex output and use common routines
> > > 
> > > You're quick...
> > > 
> > > >  	BUG_ON(len >= HEX_CHARS);
> > > 
> > > I think the BUG_ON is senseless.
> > > 
> > 
> > Agreed, it probably meant to say BUG_ON(len * 2 > HEX_CHARS -1)
> > 
> > But as this is only called from within a helper macro, it could be
> > changed to a build-time check instead of a runtime:
> > 
> > From: Harvey Harrison <harvey.harrison@gmail.com>
> > Subject: [PATCH] trace: add build-time check to avoid overrunning hex buffer
> 
> applied to tip/tracing/core:
> 
>  bb86ba7: trace: add build-time check to avoid overrunning hex buffer
>  32ce86f: ftrace: fix hex output mode of ftrace
> 

Thinking about this, these should probably be forwarded to -stable once
they go into mainline.

Cheers,

Harvey