Re: [Qemu-devel] [PATCH] trace: Make trace record fields 64-bit

[Prerna Saxena <prerna@linux.vnet.ibm.com>]

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


-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India