[Qemu-devel] [PATCH] trace: fix simple trace "disable" keyword

[Qemu-devel] [PATCH] trace: fix simple trace "disable" keyword

[Stefan Hajnoczi]

The trace-events "disable" keyword turns an event into a nop at
compile-time.  This is important for high-frequency events that can
impact performance.

The "disable" keyword is currently broken in the simple trace backend.
This patch fixes the problem as follows:

Trace events are identified by their TraceEventID number.  When events
are disabled there are two options for assigning TraceEventID numbers:
1. Skip disabled events and don't assign them a number.
2. Assign numbers for all events regardless of the disabled keyword.

The simple trace backend and its binary file format uses approach #1.

The tracetool infrastructure has been using approach #2 for a while.

The result is that the numbers used in simple trace files do not
correspond with TraceEventIDs.  In trace/simple.c we assumed that they
are identical and therefore emitted bogus numbers.

This patch fixes the bug by using TraceEventID for trace_event_id()
while sticking to approach #1 for simple trace file numbers.  This
preserves simple trace file format compatibility.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/tracetool/backend/simple.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 37ef599..7b72164 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -56,7 +56,7 @@ def c(events):
 
 
         out('',
-            '    TraceEvent *eventp = trace_event_id(%(event_id)s);',
+            '    TraceEvent *eventp = trace_event_id(%(event_enum)s);',
             '    bool _state = trace_event_get_state_dynamic(eventp);',
             '    if (!_state) {',
             '        return;',
@@ -65,6 +65,7 @@ def c(events):
             '    if (trace_record_start(&rec, %(event_id)s, %(size_str)s)) {',
             '        return; /* Trace Buffer Full, Event Dropped ! */',
             '    }',
+            event_enum = 'TRACE_' + event.name.upper(),
             event_id = num,
             size_str = sizestr,
             )
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH] trace: fix simple trace "disable" keyword

[Stefan Hajnoczi]

On Wed, Jan 15, 2014 at 11:24:07AM +0800, Stefan Hajnoczi wrote:
> The trace-events "disable" keyword turns an event into a nop at
> compile-time.  This is important for high-frequency events that can
> impact performance.
> 
> The "disable" keyword is currently broken in the simple trace backend.
> This patch fixes the problem as follows:
> 
> Trace events are identified by their TraceEventID number.  When events
> are disabled there are two options for assigning TraceEventID numbers:
> 1. Skip disabled events and don't assign them a number.
> 2. Assign numbers for all events regardless of the disabled keyword.
> 
> The simple trace backend and its binary file format uses approach #1.
> 
> The tracetool infrastructure has been using approach #2 for a while.
> 
> The result is that the numbers used in simple trace files do not
> correspond with TraceEventIDs.  In trace/simple.c we assumed that they
> are identical and therefore emitted bogus numbers.
> 
> This patch fixes the bug by using TraceEventID for trace_event_id()
> while sticking to approach #1 for simple trace file numbers.  This
> preserves simple trace file format compatibility.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  scripts/tracetool/backend/simple.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

I applied this fairly quickly, so feel free to speak up if you have any
code review feedback.  I can still drop it from the tracing queue before
sending the next pull request.

Stefan