Patchwork trace: Make trace record fields 64-bit

login
register
mail settings
Submitter Stefan Hajnoczi
Date Aug. 9, 2010, 1:35 p.m.
Message ID <1281360914-9937-1-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/61280/
State New
Headers show

Comments

Stefan Hajnoczi - Aug. 9, 2010, 1:35 p.m.
Explicitly use 64-bit fields in trace records so that timestamps and
magic numbers work for 32-bit host builds.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 simpletrace.c  |   31 +++++++++++++++++++++----------
 simpletrace.h  |   11 ++++++-----
 simpletrace.py |    2 +-
 tracetool      |    6 +++---
 4 files changed, 31 insertions(+), 19 deletions(-)
Prerna Saxena - Aug. 11, 2010, 6:03 a.m.
On 08/09/2010 07:05 PM, Stefan Hajnoczi wrote:
> Explicitly use 64-bit fields in trace records so that timestamps and
> magic numbers work for 32-bit host builds.
>
> Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com>
> ---
>   simpletrace.c  |   31 +++++++++++++++++++++----------
>   simpletrace.h  |   11 ++++++-----
>   simpletrace.py |    2 +-
>   tracetool      |    6 +++---
>   4 files changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/simpletrace.c b/simpletrace.c
> index 954cc4e..01acfc5 100644
> --- a/simpletrace.c
> +++ b/simpletrace.c
> @@ -9,18 +9,29 @@
>    */
>
>   #include<stdlib.h>
> +#include<stdint.h>
>   #include<stdio.h>
>   #include<time.h>
>   #include "trace.h"
>
> +/** Trace file header event ID */
> +#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with TraceEventIDs */
> +
> +/** Trace file magic number */
> +#define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
> +
> +/** Trace file version number, bump if format changes */
> +#define HEADER_VERSION 0
> +
> +/** Trace buffer entry */
>   typedef struct {
> -    unsigned long event;
> -    unsigned long timestamp_ns;
> -    unsigned long x1;
> -    unsigned long x2;
> -    unsigned long x3;
> -    unsigned long x4;
> -    unsigned long x5;
> +    uint64_t event;
> +    uint64_t timestamp_ns;
> +    uint64_t x1;
> +    uint64_t x2;
> +    uint64_t x3;
> +    uint64_t x4;
> +    uint64_t x5;
>   } TraceRecord;
>
>   enum {
> @@ -42,9 +53,9 @@ void st_print_trace_file_status(FILE *stream, int (*stream_printf)(FILE *stream,
>   static bool write_header(FILE *fp)
>   {
>       TraceRecord header = {
> -        .event = -1UL, /* max avoids conflicting with TraceEventIDs */
> -        .timestamp_ns = 0xf2b177cb0aa429b4, /* magic number */
> -        .x1 = 0, /* bump this version number if file format changes */
> +        .event = HEADER_EVENT_ID,
> +        .timestamp_ns = HEADER_MAGIC,
> +        .x1 = HEADER_VERSION,
>       };
>
>       return fwrite(&header, sizeof header, 1, fp) == 1;
> diff --git a/simpletrace.h b/simpletrace.h
> index 6a2b8d9..f81aa8e 100644
> --- a/simpletrace.h
> +++ b/simpletrace.h
> @@ -10,6 +10,7 @@
>   #define SIMPLETRACE_H
>
>   #include<stdbool.h>
> +#include<stdint.h>
>   #include<stdio.h>
>
>   typedef unsigned int TraceEventID;

It would be useful to have :

typedef uint64_t TraceEventID;

This ensures that the maximum number of trace events available on both 
32 and 64 bit builds is same.

> @@ -20,11 +21,11 @@ typedef struct {
>   } TraceEvent;
>
>   void trace0(TraceEventID event);
> -void trace1(TraceEventID event, unsigned long x1);
> -void trace2(TraceEventID event, unsigned long x1, unsigned long x2);
> -void trace3(TraceEventID event, unsigned long x1, unsigned long x2, unsigned long x3);
> -void trace4(TraceEventID event, unsigned long x1, unsigned long x2, unsigned long x3, unsigned long x4);
> -void trace5(TraceEventID event, unsigned long x1, unsigned long x2, unsigned long x3, unsigned long x4, unsigned long x5);
> +void trace1(TraceEventID event, uint64_t x1);
> +void trace2(TraceEventID event, uint64_t x1, uint64_t x2);
> +void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3);
> +void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4);
> +void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5);
>   void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
>   void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
>   void st_change_trace_event_state(const char *tname, bool tstate);
> diff --git a/simpletrace.py b/simpletrace.py
> index 979d911..fdf0eb5 100755
> --- a/simpletrace.py
> +++ b/simpletrace.py
> @@ -17,7 +17,7 @@ header_event_id = 0xffffffffffffffff
>   header_magic    = 0xf2b177cb0aa429b4
>   header_version  = 0
>
> -trace_fmt = 'LLLLLLL'
> +trace_fmt = '=QQQQQQQ'
>   trace_len = struct.calcsize(trace_fmt)
>   event_re  = re.compile(r'(disable\s+)?([a-zA-Z0-9_]+)\(([^)]*)\)\s+"([^"]*)"')
>
> diff --git a/tracetool b/tracetool
> index c5a5bdc..b78cd97 100755
> --- a/tracetool
> +++ b/tracetool
> @@ -151,11 +151,11 @@ EOF
>       simple_event_num=0
>   }
>
> -cast_args_to_ulong()
> +cast_args_to_uint64_t()
>   {
>       local arg
>       for arg in $(get_argnames "$1"); do
> -        echo -n "(unsigned long)$arg"
> +        echo -n "(uint64_t)$arg"

Tested this on a 32 bit host. It throws up some warnings, and we need :
echo -n "(uint64_t)(uintptr_t)$arg"

>       done
>   }
>
> @@ -173,7 +173,7 @@ linetoh_simple()
>       trace_args="$simple_event_num"
>       if [ "$argc" -gt 0 ]
>       then
> -        trace_args="$trace_args, $(cast_args_to_ulong "$1")"
> +        trace_args="$trace_args, $(cast_args_to_uint64_t "$1")"
>       fi
>
>       cat<<EOF
malc - Aug. 11, 2010, 9:37 a.m.
On Wed, 11 Aug 2010, Prerna Saxena wrote:

> On 08/09/2010 07:05 PM, Stefan Hajnoczi wrote:
> > Explicitly use 64-bit fields in trace records so that timestamps and
> > magic numbers work for 32-bit host builds.
> > 
> > Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com>
> > ---
> >   simpletrace.c  |   31 +++++++++++++++++++++----------
> >   simpletrace.h  |   11 ++++++-----
> >   simpletrace.py |    2 +-
> >   tracetool      |    6 +++---
> >   4 files changed, 31 insertions(+), 19 deletions(-)
> > 
> > diff --git a/simpletrace.c b/simpletrace.c
> > index 954cc4e..01acfc5 100644
> > --- a/simpletrace.c
> > +++ b/simpletrace.c
> > @@ -9,18 +9,29 @@
> >    */
> > 
> >   #include<stdlib.h>
> > +#include<stdint.h>
> >   #include<stdio.h>
> >   #include<time.h>
> >   #include "trace.h"
> > 
> > +/** Trace file header event ID */
> > +#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with
> > TraceEventIDs */
> > +
> > +/** Trace file magic number */
> > +#define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
> > +
> > +/** Trace file version number, bump if format changes */
> > +#define HEADER_VERSION 0
> > +
> > +/** Trace buffer entry */
> >   typedef struct {
> > -    unsigned long event;
> > -    unsigned long timestamp_ns;
> > -    unsigned long x1;
> > -    unsigned long x2;
> > -    unsigned long x3;
> > -    unsigned long x4;
> > -    unsigned long x5;
> > +    uint64_t event;
> > +    uint64_t timestamp_ns;
> > +    uint64_t x1;
> > +    uint64_t x2;
> > +    uint64_t x3;
> > +    uint64_t x4;
> > +    uint64_t x5;
> >   } TraceRecord;
> > 
> >   enum {
> > @@ -42,9 +53,9 @@ void st_print_trace_file_status(FILE *stream, int
> > (*stream_printf)(FILE *stream,
> >   static bool write_header(FILE *fp)
> >   {
> >       TraceRecord header = {
> > -        .event = -1UL, /* max avoids conflicting with TraceEventIDs */
> > -        .timestamp_ns = 0xf2b177cb0aa429b4, /* magic number */
> > -        .x1 = 0, /* bump this version number if file format changes */
> > +        .event = HEADER_EVENT_ID,
> > +        .timestamp_ns = HEADER_MAGIC,
> > +        .x1 = HEADER_VERSION,
> >       };
> > 
> >       return fwrite(&header, sizeof header, 1, fp) == 1;
> > diff --git a/simpletrace.h b/simpletrace.h
> > index 6a2b8d9..f81aa8e 100644
> > --- a/simpletrace.h
> > +++ b/simpletrace.h
> > @@ -10,6 +10,7 @@
> >   #define SIMPLETRACE_H
> > 
> >   #include<stdbool.h>
> > +#include<stdint.h>
> >   #include<stdio.h>
> > 
> >   typedef unsigned int TraceEventID;
> 
> It would be useful to have :
> 
> typedef uint64_t TraceEventID;
> 
> This ensures that the maximum number of trace events available on both 32 and
> 64 bit builds is same.
> 
> > @@ -20,11 +21,11 @@ typedef struct {
> >   } TraceEvent;
> > 
> >   void trace0(TraceEventID event);
> > -void trace1(TraceEventID event, unsigned long x1);
> > -void trace2(TraceEventID event, unsigned long x1, unsigned long x2);
> > -void trace3(TraceEventID event, unsigned long x1, unsigned long x2,
> > unsigned long x3);
> > -void trace4(TraceEventID event, unsigned long x1, unsigned long x2,
> > unsigned long x3, unsigned long x4);
> > -void trace5(TraceEventID event, unsigned long x1, unsigned long x2,
> > unsigned long x3, unsigned long x4, unsigned long x5);
> > +void trace1(TraceEventID event, uint64_t x1);
> > +void trace2(TraceEventID event, uint64_t x1, uint64_t x2);
> > +void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3);
> > +void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3,
> > uint64_t x4);
> > +void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3,
> > uint64_t x4, uint64_t x5);
> >   void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const
> > char *fmt, ...));
> >   void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE
> > *stream, const char *fmt, ...));
> >   void st_change_trace_event_state(const char *tname, bool tstate);
> > diff --git a/simpletrace.py b/simpletrace.py
> > index 979d911..fdf0eb5 100755
> > --- a/simpletrace.py
> > +++ b/simpletrace.py
> > @@ -17,7 +17,7 @@ header_event_id = 0xffffffffffffffff
> >   header_magic    = 0xf2b177cb0aa429b4
> >   header_version  = 0
> > 
> > -trace_fmt = 'LLLLLLL'
> > +trace_fmt = '=QQQQQQQ'
> >   trace_len = struct.calcsize(trace_fmt)
> >   event_re  =
> > re.compile(r'(disable\s+)?([a-zA-Z0-9_]+)\(([^)]*)\)\s+"([^"]*)"')
> > 
> > diff --git a/tracetool b/tracetool
> > index c5a5bdc..b78cd97 100755
> > --- a/tracetool
> > +++ b/tracetool
> > @@ -151,11 +151,11 @@ EOF
> >       simple_event_num=0
> >   }
> > 
> > -cast_args_to_ulong()
> > +cast_args_to_uint64_t()
> >   {
> >       local arg
> >       for arg in $(get_argnames "$1"); do
> > -        echo -n "(unsigned long)$arg"
> > +        echo -n "(uint64_t)$arg"
> 
> Tested this on a 32 bit host. It throws up some warnings, and we need :
> echo -n "(uint64_t)(uintptr_t)$arg"

Generic echo doesn't have any command line options, please use printf
instead.

> 
> >       done
> >   }
> > 
> > @@ -173,7 +173,7 @@ linetoh_simple()
> >       trace_args="$simple_event_num"
> >       if [ "$argc" -gt 0 ]
> >       then
> > -        trace_args="$trace_args, $(cast_args_to_ulong "$1")"
> > +        trace_args="$trace_args, $(cast_args_to_uint64_t "$1")"
> >       fi
> > 
> >       cat<<EOF
> 
> 
>

Patch

diff --git a/simpletrace.c b/simpletrace.c
index 954cc4e..01acfc5 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -9,18 +9,29 @@ 
  */
 
 #include <stdlib.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <time.h>
 #include "trace.h"
 
+/** Trace file header event ID */
+#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with TraceEventIDs */
+
+/** Trace file magic number */
+#define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
+
+/** Trace file version number, bump if format changes */
+#define HEADER_VERSION 0
+
+/** Trace buffer entry */
 typedef struct {
-    unsigned long event;
-    unsigned long timestamp_ns;
-    unsigned long x1;
-    unsigned long x2;
-    unsigned long x3;
-    unsigned long x4;
-    unsigned long x5;
+    uint64_t event;
+    uint64_t timestamp_ns;
+    uint64_t x1;
+    uint64_t x2;
+    uint64_t x3;
+    uint64_t x4;
+    uint64_t x5;
 } TraceRecord;
 
 enum {
@@ -42,9 +53,9 @@  void st_print_trace_file_status(FILE *stream, int (*stream_printf)(FILE *stream,
 static bool write_header(FILE *fp)
 {
     TraceRecord header = {
-        .event = -1UL, /* max avoids conflicting with TraceEventIDs */
-        .timestamp_ns = 0xf2b177cb0aa429b4, /* magic number */
-        .x1 = 0, /* bump this version number if file format changes */
+        .event = HEADER_EVENT_ID,
+        .timestamp_ns = HEADER_MAGIC,
+        .x1 = HEADER_VERSION,
     };
 
     return fwrite(&header, sizeof header, 1, fp) == 1;
diff --git a/simpletrace.h b/simpletrace.h
index 6a2b8d9..f81aa8e 100644
--- a/simpletrace.h
+++ b/simpletrace.h
@@ -10,6 +10,7 @@ 
 #define SIMPLETRACE_H
 
 #include <stdbool.h>
+#include <stdint.h>
 #include <stdio.h>
 
 typedef unsigned int TraceEventID;
@@ -20,11 +21,11 @@  typedef struct {
 } TraceEvent;
 
 void trace0(TraceEventID event);
-void trace1(TraceEventID event, unsigned long x1);
-void trace2(TraceEventID event, unsigned long x1, unsigned long x2);
-void trace3(TraceEventID event, unsigned long x1, unsigned long x2, unsigned long x3);
-void trace4(TraceEventID event, unsigned long x1, unsigned long x2, unsigned long x3, unsigned long x4);
-void trace5(TraceEventID event, unsigned long x1, unsigned long x2, unsigned long x3, unsigned long x4, unsigned long x5);
+void trace1(TraceEventID event, uint64_t x1);
+void trace2(TraceEventID event, uint64_t x1, uint64_t x2);
+void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3);
+void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4);
+void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5);
 void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
 void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
 void st_change_trace_event_state(const char *tname, bool tstate);
diff --git a/simpletrace.py b/simpletrace.py
index 979d911..fdf0eb5 100755
--- a/simpletrace.py
+++ b/simpletrace.py
@@ -17,7 +17,7 @@  header_event_id = 0xffffffffffffffff
 header_magic    = 0xf2b177cb0aa429b4
 header_version  = 0
 
-trace_fmt = 'LLLLLLL'
+trace_fmt = '=QQQQQQQ'
 trace_len = struct.calcsize(trace_fmt)
 event_re  = re.compile(r'(disable\s+)?([a-zA-Z0-9_]+)\(([^)]*)\)\s+"([^"]*)"')
 
diff --git a/tracetool b/tracetool
index c5a5bdc..b78cd97 100755
--- a/tracetool
+++ b/tracetool
@@ -151,11 +151,11 @@  EOF
     simple_event_num=0
 }
 
-cast_args_to_ulong()
+cast_args_to_uint64_t()
 {
     local arg
     for arg in $(get_argnames "$1"); do
-        echo -n "(unsigned long)$arg"
+        echo -n "(uint64_t)$arg"
     done
 }
 
@@ -173,7 +173,7 @@  linetoh_simple()
     trace_args="$simple_event_num"
     if [ "$argc" -gt 0 ]
     then
-        trace_args="$trace_args, $(cast_args_to_ulong "$1")"
+        trace_args="$trace_args, $(cast_args_to_uint64_t "$1")"
     fi
 
     cat <<EOF