[Qemu-devel] [PATCH 1/4] trace: [ust] fix generation of 'trace.c' on events without args

[Qemu-devel] [PATCH 1/4] trace: [ust] fix generation of 'trace.c' on events without args

[Lluís]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/tracetool b/scripts/tracetool
index 412f695..d88cb43 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -338,6 +338,7 @@ linetoc_ust()
     name=$(get_name "$1")
     args=$(get_args "$1")
     argnames=$(get_argnames "$1", ",")
+    [ -z "$argnames" ] || argnames=", $argnames"
     fmt=$(get_fmt "$1")
 
     cat <<EOF
@@ -345,7 +346,7 @@ DEFINE_TRACE(ust_$name);
 
 static void ust_${name}_probe($args)
 {
-    trace_mark(ust, $name, "$fmt", $argnames);
+    trace_mark(ust, $name, "$fmt"$argnames);
 }
 EOF
 
@@ -488,7 +489,7 @@ EOF
         cat <<EOF
   $arg = \$arg$i;
 EOF
-	i="$((i+1))"
+        i="$((i+1))"
     done
 
     cat <<EOF
@@ -585,7 +586,7 @@ tracetostap()
        exit 1
     fi
     if [ -z "$probeprefix" ]; then
-	probeprefix="qemu.$targettype.$targetarch";
+        probeprefix="qemu.$targettype.$targetarch";
     fi
     echo "/* This file is autogenerated by tracetool, do not edit. */"
     convert stap

[Qemu-devel] [PATCH 2/4] trace: generalize the "property" concept in the trace-events file

[Lluís]

This adds/modifies the following functions:

* get_name: Get _only_ the event name
* has_property: Return whether an event has a property (keyword before the event
  name)

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 docs/tracing.txt  |    4 +--
 scripts/tracetool |   73 ++++++++++++++++++++++++-----------------------------
 2 files changed, 35 insertions(+), 42 deletions(-)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index f15069c..5dbd3c0 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -98,7 +98,7 @@ portability macros, ensure they are preceded and followed by double quotes:
 4. Name trace events after their function.  If there are multiple trace events
    in one function, append a unique distinguisher at the end of the name.
 
-5. Declare trace events with the "disable" keyword.  Some trace events can
+5. Declare trace events with the "disable" property.  Some trace events can
    produce a lot of output and users are typically only interested in a subset
    of trace events.  Marking trace events disabled by default saves the user
    from having to manually disable noisy trace events.
@@ -168,7 +168,7 @@ The st_change_trace_event_state() function can be used to enable or disable trac
 events at runtime inside QEMU:
 
     #include "trace.h"
-    
+
     st_change_trace_event_state("virtio_irq", true); /* enable */
     [...]
     st_change_trace_event_state("virtio_irq", false); /* disable */
diff --git a/scripts/tracetool b/scripts/tracetool
index d88cb43..6d8ead2 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -43,7 +43,26 @@ EOF
 # Get the name of a trace event
 get_name()
 {
-    echo ${1%%\(*}
+    local name
+    name=${1%%\(*}
+    echo "${name##* }"
+}
+
+# Get the given property of a trace event
+# 1: trace-events line
+# 2: property name
+# -> return 0 if property is present, or 1 otherwise
+has_property()
+{
+    local props prop
+    props=${1%%\(*}
+    props=${props% *}
+    for prop in $props; do
+        if [ "$prop" = "$2" ]; then
+            return 0
+        fi
+    done
+    return 1
 }
 
 # Get the argument list of a trace event, including types and names
@@ -101,20 +120,6 @@ get_fmt()
     echo "$fmt"
 }
 
-# Get the state of a trace event
-get_state()
-{
-    local str disable state
-    str=$(get_name "$1")
-    disable=${str##disable }
-    if [ "$disable" = "$str" ] ; then
-        state=1
-    else
-        state=0
-    fi
-    echo "$state"
-}
-
 linetoh_begin_nop()
 {
     return
@@ -174,14 +179,10 @@ cast_args_to_uint64_t()
 
 linetoh_simple()
 {
-    local name args argc trace_args state
+    local name args argc trace_args
     name=$(get_name "$1")
     args=$(get_args "$1")
     argc=$(get_argc "$1")
-    state=$(get_state "$1")
-    if [ "$state" = "0" ]; then
-        name=${name##disable }
-    fi
 
     trace_args="$simple_event_num"
     if [ "$argc" -gt 0 ]
@@ -222,9 +223,10 @@ linetoc_simple()
 {
     local name state
     name=$(get_name "$1")
-    state=$(get_state "$1")
-    if [ "$state" = "0" ] ; then
-        name=${name##disable }
+    if has_property "$1" "disable"; then
+        state="0"
+    else
+        state="1"
     fi
     cat <<EOF
 {.tp_name = "$name", .state=$state},
@@ -379,14 +381,10 @@ EOF
 
 linetoh_dtrace()
 {
-    local name args argnames state nameupper
+    local name args argnames nameupper
     name=$(get_name "$1")
     args=$(get_args "$1")
     argnames=$(get_argnames "$1", ",")
-    state=$(get_state "$1")
-    if [ "$state" = "0" ] ; then
-        name=${name##disable }
-    fi
 
     nameupper=`echo $name | tr '[:lower:]' '[:upper:]'`
 
@@ -430,13 +428,9 @@ EOF
 
 linetod_dtrace()
 {
-    local name args state
+    local name args
     name=$(get_name "$1")
     args=$(get_args "$1")
-    state=$(get_state "$1")
-    if [ "$state" = "0" ] ; then
-        name=${name##disable }
-    fi
 
     # DTrace provider syntax expects foo() for empty
     # params, not foo(void)
@@ -464,14 +458,10 @@ linetostap_begin_dtrace()
 
 linetostap_dtrace()
 {
-    local i arg name args arglist state
+    local i arg name args arglist
     name=$(get_name "$1")
     args=$(get_args "$1")
     arglist=$(get_argnames "$1", "")
-    state=$(get_state "$1")
-    if [ "$state" = "0" ] ; then
-        name=${name##disable }
-    fi
 
     # Define prototype for probe arguments
     cat <<EOF
@@ -517,9 +507,12 @@ convert()
         test -z "${str%%#*}" && continue
 
         # Process the line.  The nop backend handles disabled lines.
-        disable=${str%%disable *}
+        disable="0"
+        if has_property "$str" "disable"; then
+            disable="1"
+        fi
         echo
-        if test -z "$disable"; then
+        if [ "$disable" = "1" ]; then
             # Pass the disabled state as an arg for the simple
             # or DTrace backends which handle it dynamically.
             # For all other backends, call lineto$1_nop()

[Qemu-devel] [PATCH 3/4] trace: always use the "nop" backend on events with the "disable" keyword

[Lluís]

Any event with the keyword/property "disable" generates an empty trace event
using the "nop" backend, regardless of the current backend.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 docs/tracing.txt  |    3 +++
 scripts/tracetool |   15 ++-------------
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index 5dbd3c0..49e030e 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -126,6 +126,9 @@ The "nop" backend generates empty trace event functions so that the compiler
 can optimize out trace events completely.  This is the default and imposes no
 performance penalty.
 
+Note that regardless of the selected trace backend, events with the "disable"
+property will be generated with the "nop" backend.
+
 === Stderr ===
 
 The "stderr" backend sends trace events directly to standard error.  This
diff --git a/scripts/tracetool b/scripts/tracetool
index 6d8ead2..7506776 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -506,21 +506,10 @@ convert()
         # Skip comments and empty lines
         test -z "${str%%#*}" && continue
 
+        echo
         # Process the line.  The nop backend handles disabled lines.
-        disable="0"
         if has_property "$str" "disable"; then
-            disable="1"
-        fi
-        echo
-        if [ "$disable" = "1" ]; then
-            # Pass the disabled state as an arg for the simple
-            # or DTrace backends which handle it dynamically.
-            # For all other backends, call lineto$1_nop()
-            if [ $backend = "simple" -o "$backend" = "dtrace" ]; then
-                "$process_line" "$str"
-            else
-                "lineto$1_nop" "${str##disable }"
-            fi
+            "lineto$1_nop" "$str"
         else
             "$process_line" "$str"
         fi

[Qemu-devel] [PATCH 4/4] trace: [simple] always enable trace points

[Lluís]

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/scripts/tracetool b/scripts/tracetool
index 7506776..b355ac5 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -221,15 +221,10 @@ EOF
 
 linetoc_simple()
 {
-    local name state
+    local name
     name=$(get_name "$1")
-    if has_property "$1" "disable"; then
-        state="0"
-    else
-        state="1"
-    fi
     cat <<EOF
-{.tp_name = "$name", .state=$state},
+{.tp_name = "$name", .state=1},
 EOF
     simple_event_num=$((simple_event_num + 1))
 }

Re: [Qemu-devel] [PATCH 4/4] trace: [simple] always enable trace points

[Lluís]

Last patch was missing the trace-events hunk.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>

diff --git a/scripts/tracetool b/scripts/tracetool
index 7506776..b355ac5 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -221,15 +221,10 @@ EOF

 linetoc_simple()
 {
-    local name state
+    local name
     name=$(get_name "$1")
-    if has_property "$1" "disable"; then
-        state="0"
-    else
-        state="1"
-    fi
     cat <<EOF
-{.tp_name = "$name", .state=$state},
+{.tp_name = "$name", .state=1},
 EOF
     simple_event_num=$((simple_event_num + 1))
 }
diff --git a/trace-events b/trace-events
index 90c9e0b..d871ffc 100644
--- a/trace-events
+++ b/trace-events
@@ -17,9 +17,6 @@
 # Example: qemu_malloc(size_t size) "size %zu"
 #
 # The "disable" keyword will build without the trace event.
-# In case of 'simple' trace backend, it will allow the trace event to be
-# compiled, but this would be turned off by default. It can be toggled on via
-# the monitor.
 #
 # The <name> must be a valid as a C function name.
 #

Re: [Qemu-devel] [PATCH 4/4] trace: [simple] always enable trace points

[Lluís]

Last time there was discussion on disabling all tracee points in simple
by default, but providing an argument for a list of trace point to
enable at startup.

Instead, what I've done is enabling all trace points in simple by
default.

Given that "disable" will effectively disable all points, including the
dtrace backend, my question is what is preferred:

* leave it as it is in this patch
  * leave all points as disabled, so the user has to remove the
    "disable" regardless of the backend
  * simple acts as stderr, all non-disabled points will generate output

* remove the "disable" on all events
  * for any backend with trace point "states" leave the state as
    disabled by default (dynamically enabling them is a backendspecific
    operation, when supported)
  * this means implementing the event list argument for the simple
    backend in order to easily enable a set of events at program startup


Lluis

--
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth