Re: [Qemu-devel] [PATCH] trace: Provide a per-event status define for conditional compilation

[Blue Swirl]

2011/9/21 Lluís Vilanova <address@hidden>:
> Adds a 'trace_${name}_enabled' preprocessor define for each tracing event in
> "trace.h".
>
> This lets the user conditionally compile code with a relatively high execution
> cost that is only necessary when producing the tracing information for an 
> event
> that is enabled.
>
> Note that events using this define will probably have the "disable" property 
> by
> default, in order to avoid such costs on regular builds.
>
> Signed-off-by: Lluís Vilanova <address@hidden>
> ---
>  docs/tracing.txt  |   46 ++++++++++++++++++++++++++++++++++++++++------
>  scripts/tracetool |    8 +++++++-
>  2 files changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/docs/tracing.txt b/docs/tracing.txt
> index b36b54b..5bee61e 100644
> --- a/docs/tracing.txt
> +++ b/docs/tracing.txt
> @@ -103,12 +103,6 @@ respectively.  This ensures portability between 32- and 
> 64-bit platforms.
>  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. If specific trace events are going to be called a huge number of times, 
> this
> -   might have a noticeable performance impact even when the trace events are
> -   programmatically disabled. In this case you should declare the trace event
> -   with the "disable" property, which will effectively disable it at compile
> -   time (using the "nop" backend).
> -
>  == Generic interface and monitor commands ==
>
>  You can programmatically query and control the dynamic state of trace events
> @@ -229,3 +223,43 @@ probes:
>                       --target-type system \
>                       --target-arch x86_64 \
>                       <trace-events >qemu.stp
> +
> +== Trace event properties ==
> +
> +Each event in the "trace-events" file can be prefixed with a space-separated
> +list of zero or more of the following event properties.
> +
> +=== "disable" ===
> +
> +If a specific trace event is going to be invoked a huge number of times, this
> +might have a noticeable performance impact even when the event is
> +programmatically disabled.
> +
> +In this case you should declare such event with the "disable" property. This
> +will effectively disable the event at compile time (by using the "nop" 
> backend),
> +thus having no performance impact at all on regular builds (i.e., unless you
> +edit the "trace-events" file).

Nack, by default events should have no performance impact.

> +
> +In addition, there might be cases where relatively complex computations must 
> be
> +performed to generate values that are only used as arguments for a trace
> +function. In these cases you can guard such computations to avoid its
> +compilation when the event is disabled:
> +
> +    #include "trace.h"  /* needed for trace event prototype */
> +
> +    void *qemu_vmalloc(size_t size)
> +    {
> +        void *ptr;
> +        size_t align = QEMU_VMALLOC_ALIGN;
> +
> +        if (size < align) {
> +            align = getpagesize();
> +        }
> +        ptr = qemu_memalign(align, size);
> +        if (trace_qemu_vmalloc_enabled) { /* preprocessor macro */

Macros should be uppercase to avoid confusion.

> +            void *complex;
> +            /* some complex computations to produce the 'complex' value */
> +            trace_qemu_vmalloc(size, ptr, complex);
> +        }
> +        return ptr;
> +    }
> diff --git a/scripts/tracetool b/scripts/tracetool
> index 4c9951d..97f9f1b 100755
> --- a/scripts/tracetool
> +++ b/scripts/tracetool
> @@ -519,7 +519,7 @@ linetostap_end_dtrace()
>  # Process stdin by calling begin, line, and end functions for the backend
>  convert()
>  {
> -    local begin process_line end str disable
> +    local begin process_line end str name enabled
>     begin="lineto$1_begin_$backend"
>     process_line="lineto$1_$backend"
>     end="lineto$1_end_$backend"
> @@ -534,8 +534,14 @@ convert()
>         # Process the line.  The nop backend handles disabled lines.
>         if has_property "$str" "disable"; then
>             "lineto$1_nop" "$str"
> +            enabled=0
>         else
>             "$process_line" "$str"
> +            enabled=1

Instead of '1', couldn't this be a function that checks if the trace
point is enabled during run time (from monitor etc.)?

> +        fi
> +        if [ "$1" = "h" ]; then
> +            name=$(get_name "$str")
> +            echo "#define trace_${name}_enabled ${enabled}"
>         fi
>     done
>
>
>
>