Patchwork [v2] Add a DTrace tracing backend targetted for SystemTAP compatability

login
register
mail settings
Submitter Daniel P. Berrange
Date Oct. 20, 2010, 2:09 p.m.
Message ID <1287583790-9349-1-git-send-email-berrange@redhat.com>
Download mbox | patch
Permalink /patch/68442/
State New
Headers show

Comments

Daniel P. Berrange - Oct. 20, 2010, 2:09 p.m.
This introduces a new tracing backend that targets the SystemTAP
implementation of DTrace userspace tracing. The core functionality
should be applicable and standard across any DTrace implementation
on Solaris, OS-X, *BSD, but the Makefile rules will likely need
some small additional changes to cope with OS specific build
requirements.

This backend builds a little differently from the other tracing
backends. Specifically there is no 'trace.c' file, because the
'dtrace' command line tool generates a '.o' file directly from
the dtrace probe definition file. The probe definition is usually
named with a '.d' extension but QEMU uses '.d' files for its
external makefile dependancy tracking, so this uses '.dtrace' as
the extension for the probe definition file.

The 'tracetool' program gains the ability to generate a trace.h
file for DTrace, and also to generate the trace.d file containing
the dtrace probe definition, and finally a qemu.stp file which is
a wrapper around the probe definition providing more convenient
access from SystemTAP scripts.

eg, instead of

  probe process("qemu").mark("qemu_malloc") {
    printf("Malloc %d %p\n", $arg1, $arg2);
  }

The addition of qemu.stp to /usr/share/systemtap/tapset/
lets users write

  probe qemu.qemu_malloc {
    printf("Malloc %d %p\n", size, ptr);
  }

In v2:

  - Add check for 'dtrace' command in configure
  - Comply with coding standards in generated code
  - Misc fixes to tracetool
  - Add more generated files to make clean target
  - Mention 'dtrace' backend in configure help

Still todo in v3:

  - Change process("qemu") statement so that it applies
    to all 'qemu-system-XXX' binaries not just 'qemu'

* .gitignore: Ignore trace-dtrace.*
* Makefile: Extra rules for generating DTrace files
* Makefile.obj: Don't build trace.o for DTrace, use
  trace-dtrace.o generated by 'dtrace' instead
* tracetool: Support for generating DTrace/SystemTAP
  data files

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 .gitignore    |    3 +
 Makefile      |   32 +++++++++++
 Makefile.objs |    4 ++
 configure     |   12 ++++-
 tracetool     |  169 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 209 insertions(+), 11 deletions(-)
Stefan Hajnoczi - Oct. 20, 2010, 2:36 p.m.
On Wed, Oct 20, 2010 at 3:09 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> @@ -2175,6 +2175,16 @@ EOF
>     exit 1
>   fi
>  fi
> +
> +##########################################
> +# For 'dtrace' backend, test if 'dtrace' command is present
> +if ! has 'dtrace' ; then
> +  echo
> +  echo "Error: dtrace command is not found in PATH $PATH"
> +  echo
> +  exit 1
> +fi
> +
>  ##########################################
>  # End of CC checks
>  # After here, no more $cc or $ld runs

This doesn't seem to check that [ "$trace_backend" = "dtrace" ] and
will break ./configure on boxes without dtrace even when
--trace-backend=dtrace is not used.

Stefan
Prerna Saxena - Oct. 27, 2010, 6:56 a.m.
ACK, works well!
A suggestion though..

On 10/20/2010 07:39 PM, Daniel P. Berrange wrote:
>
> eg, instead of
>
>    probe process("qemu").mark("qemu_malloc") {
>      printf("Malloc %d %p\n", $arg1, $arg2);
>    }
>
> The addition of qemu.stp to /usr/share/systemtap/tapset/
> lets users write
>
>    probe qemu.qemu_malloc {
>      printf("Malloc %d %p\n", size, ptr);
>    }
>...
>
>
> diff --git a/tracetool b/tracetool
> index 7010858..047f16b 100755
> --- a/tracetool
> +++ b/tracetool
> +linetos_dtrace()
> +{
> +    local name args arglist state
> +
> +    # Define prototype for probe arguments
> +    cat<<EOF
> +probe qemu.$name = process("qemu").mark("$name")
> +{

The 'process' probes only work by looking for the binary in $PATH, 
unless the full path is specified. When compiling qemu in non-standard 
locations ( ie with --prefix), such probes would not point to the 
correct binary. It could be nice if tracetool could pass the full build 
path for defining the probe point. Eg,

probe qemu.qemu_malloc = 
process("/Path/to/build/dir/bin/qemu").mark("qemu_malloc") { .. }
Daniel P. Berrange - Oct. 27, 2010, 8:44 a.m.
On Wed, Oct 27, 2010 at 12:26:50PM +0530, Prerna Saxena wrote:
> ACK, works well!
> A suggestion though..
> 
> On 10/20/2010 07:39 PM, Daniel P. Berrange wrote:
> >
> >eg, instead of
> >
> >   probe process("qemu").mark("qemu_malloc") {
> >     printf("Malloc %d %p\n", $arg1, $arg2);
> >   }
> >
> >The addition of qemu.stp to /usr/share/systemtap/tapset/
> >lets users write
> >
> >   probe qemu.qemu_malloc {
> >     printf("Malloc %d %p\n", size, ptr);
> >   }
> >...
> >
> >
> >diff --git a/tracetool b/tracetool
> >index 7010858..047f16b 100755
> >--- a/tracetool
> >+++ b/tracetool
> >+linetos_dtrace()
> >+{
> >+    local name args arglist state
> >+
> >+    # Define prototype for probe arguments
> >+    cat<<EOF
> >+probe qemu.$name = process("qemu").mark("$name")
> >+{
> 
> The 'process' probes only work by looking for the binary in $PATH, 
> unless the full path is specified. When compiling qemu in non-standard 
> locations ( ie with --prefix), such probes would not point to the 
> correct binary. It could be nice if tracetool could pass the full build 
> path for defining the probe point. Eg,
> 
> probe qemu.qemu_malloc = 
> process("/Path/to/build/dir/bin/qemu").mark("qemu_malloc") { .. }

Yes, that's doable. The other problem with this tapset is that it
only works for the 'qemu' binary and not the 'qemu-system-XXXX'
binaries. I either need to make the probe cover all binaries, or
generate one tapset per architecture target.  I'm going to split
this patch in 2, one with just the core dtrace bits, and a second
with the systemtap tapset addon.

Regards,
Daniel

Patch

diff --git a/.gitignore b/.gitignore
index a43e4d1..0d27afd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,6 +4,9 @@  config-host.*
 config-target.*
 trace.h
 trace.c
+trace-dtrace.h
+trace-dtrace.dtrace
+qemu.stp
 *-timestamp
 *-softmmu
 *-darwin-user
diff --git a/Makefile b/Makefile
index 252c817..5aa85f6 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,9 @@ 
 # Makefile for QEMU.
 
 GENERATED_HEADERS = config-host.h trace.h
+ifeq ($(TRACE_BACKEND),dtrace)
+GENERATED_HEADERS += trace-dtrace.h
+endif
 
 ifneq ($(wildcard config-host.mak),)
 # Put the all: rule here so that config-host.mak can contain dependencies.
@@ -106,7 +109,11 @@  ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
 
 bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
 
+ifeq ($(TRACE_BACKEND),dtrace)
+trace.h: trace.h-timestamp trace-dtrace.h
+else
 trace.h: trace.h-timestamp
+endif
 trace.h-timestamp: $(SRC_PATH)/trace-events config-host.mak
 	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h < $< > $@,"  GEN   trace.h")
 	@cmp -s $@ trace.h || cp $@ trace.h
@@ -118,6 +125,23 @@  trace.c-timestamp: $(SRC_PATH)/trace-events config-host.mak
 
 trace.o: trace.c $(GENERATED_HEADERS)
 
+trace-dtrace.h: trace-dtrace.dtrace
+	$(call quiet-command,dtrace -o $@ -h -s $<, "  GEN   trace-dtrace.h")
+
+# Normal practice is to name DTrace probe file with a '.d' extension
+# but that gets picked up by QEMU's Makefile as an external dependancy
+# rule file. So we use '.dtrace' instead
+trace-dtrace.dtrace: trace-dtrace.dtrace-timestamp
+trace-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events config-host.mak
+	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -d < $< > $@,"  GEN   trace-dtrace.dtrace")
+	@cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace
+ifdef CONFIG_LINUX
+	$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -s < $< > qemu.stp,"  GEN   qemu.stp")
+endif
+
+trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
+	$(call quiet-command,dtrace -o $@ -G -s $<, "  GEN trace-dtrace.o")
+
 simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
 
 version.o: $(SRC_PATH)/version.rc config-host.mak
@@ -154,6 +178,8 @@  clean:
 	rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d
 	rm -f qemu-img-cmds.h
 	rm -f trace.c trace.h trace.c-timestamp trace.h-timestamp
+	rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
+	rm -f trace-dtrace.h trace-dtrace.h-timestamp qemu.stp
 	$(MAKE) -C tests clean
 	for d in $(ALL_SUBDIRS) libhw32 libhw64 libuser libdis libdis-user; do \
 	if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
@@ -214,6 +240,12 @@  ifneq ($(BLOBS),)
 		$(INSTALL_DATA) $(SRC_PATH)/pc-bios/$$x "$(DESTDIR)$(datadir)"; \
 	done
 endif
+ifeq ($(TRACE_BACKEND),dtrace)
+ifdef CONFIG_LINUX
+	$(INSTALL_DIR) "$(DESTDIR)$(datadir)/../systemtap/tapset"
+	$(INSTALL_DATA) qemu.stp "$(DESTDIR)$(datadir)/../systemtap/tapset"
+endif
+endif
 	$(INSTALL_DIR) "$(DESTDIR)$(datadir)/keymaps"
 	set -e; for x in $(KEYMAPS); do \
 		$(INSTALL_DATA) $(SRC_PATH)/pc-bios/keymaps/$$x "$(DESTDIR)$(datadir)/keymaps"; \
diff --git a/Makefile.objs b/Makefile.objs
index 816194a..ccecda0 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -273,10 +273,14 @@  libdis-$(CONFIG_SPARC_DIS) += sparc-dis.o
 ######################################################################
 # trace
 
+ifeq ($(TRACE_BACKEND),dtrace)
+trace-obj-y = trace-dtrace.o
+else
 trace-obj-y = trace.o
 ifeq ($(TRACE_BACKEND),simple)
 trace-obj-y += simpletrace.o
 endif
+endif
 
 vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
 
diff --git a/configure b/configure
index a079a49..92bb9c9 100755
--- a/configure
+++ b/configure
@@ -926,7 +926,7 @@  echo "  --enable-docs            enable documentation build"
 echo "  --disable-docs           disable documentation build"
 echo "  --disable-vhost-net      disable vhost-net acceleration support"
 echo "  --enable-vhost-net       enable vhost-net acceleration support"
-echo "  --trace-backend=B        Trace backend nop simple ust"
+echo "  --trace-backend=B        Trace backend nop simple ust dtrace"
 echo "  --trace-file=NAME        Full PATH,NAME of file to store traces"
 echo "                           Default:trace-<pid>"
 echo "  --disable-spice          disable spice"
@@ -2175,6 +2175,16 @@  EOF
     exit 1
   fi
 fi
+
+##########################################
+# For 'dtrace' backend, test if 'dtrace' command is present
+if ! has 'dtrace' ; then
+  echo
+  echo "Error: dtrace command is not found in PATH $PATH"
+  echo
+  exit 1
+fi
+
 ##########################################
 # End of CC checks
 # After here, no more $cc or $ld runs
diff --git a/tracetool b/tracetool
index 7010858..047f16b 100755
--- a/tracetool
+++ b/tracetool
@@ -20,10 +20,13 @@  Backends:
   --nop     Tracing disabled
   --simple  Simple built-in backend
   --ust     LTTng User Space Tracing backend
+  --dtrace  DTrace/SystemTAP backend
 
 Output formats:
   -h    Generate .h file
   -c    Generate .c file
+  -d    Generate .d file (DTrace only)
+  -s    Generate .stp file (DTrace with SystemTAP only)
 EOF
     exit 1
 }
@@ -46,8 +49,9 @@  get_args()
 # Get the argument name list of a trace event
 get_argnames()
 {
-    local nfields field name
+    local nfields field name sep
     nfields=0
+    sep="$2"
     for field in $(get_args "$1"); do
         nfields=$((nfields + 1))
 
@@ -58,7 +62,7 @@  get_argnames()
         name=${field%,}
         test "$field" = "$name" && continue
 
-        printf "%s" "$name, "
+        printf "%s%s " $name $sep
     done
 
     # Last argument name
@@ -73,7 +77,7 @@  get_argc()
 {
     local name argc
     argc=0
-    for name in $(get_argnames "$1"); do
+    for name in $(get_argnames "$1", ","); do
         argc=$((argc + 1))
     done
     echo $argc
@@ -154,7 +158,7 @@  EOF
 cast_args_to_uint64_t()
 {
     local arg
-    for arg in $(get_argnames "$1"); do
+    for arg in $(get_argnames "$1", ","); do
         printf "%s" "(uint64_t)(uintptr_t)$arg"
     done
 }
@@ -247,7 +251,7 @@  linetoh_ust()
     local name args argnames
     name=$(get_name "$1")
     args=$(get_args "$1")
-    argnames=$(get_argnames "$1")
+    argnames=$(get_argnames "$1", ",")
 
     cat <<EOF
 DECLARE_TRACE(ust_$name, TP_PROTO($args), TP_ARGS($argnames));
@@ -274,7 +278,7 @@  linetoc_ust()
     local name args argnames fmt
     name=$(get_name "$1")
     args=$(get_args "$1")
-    argnames=$(get_argnames "$1")
+    argnames=$(get_argnames "$1", ",")
     fmt=$(get_fmt "$1")
 
     cat <<EOF
@@ -306,6 +310,128 @@  EOF
     echo "}"
 }
 
+linetoh_begin_dtrace()
+{
+    cat <<EOF
+#include "trace-dtrace.h"
+EOF
+}
+
+linetoh_dtrace()
+{
+    local name args argnames state 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:]'`
+
+    # Define an empty function for the trace event
+    cat <<EOF
+static inline void trace_$name($args) {
+    if (QEMU_${nameupper}_ENABLED()) {
+        QEMU_${nameupper}($argnames);
+    }
+}
+EOF
+}
+
+linetoh_end_dtrace()
+{
+    return
+}
+
+linetoc_begin_dtrace()
+{
+    return
+}
+
+linetoc_dtrace()
+{
+    # No need for function definitions in dtrace backend
+    return
+}
+
+linetoc_end_dtrace()
+{
+    return
+}
+
+linetod_begin_dtrace()
+{
+    cat <<EOF
+provider qemu {
+EOF
+}
+
+linetod_dtrace()
+{
+    local name args state
+    name=$(get_name "$1")
+    args=$(get_args "$1")
+    state=$(get_state "$1")
+    if [ "$state" = "0" ] ; then
+        name=${name##disable }
+    fi
+
+    # Define prototype for probe arguments
+    cat <<EOF
+        probe $name($args);
+EOF
+}
+
+linetod_end_dtrace()
+{
+    cat <<EOF
+};
+EOF
+}
+
+linetos_begin_dtrace()
+{
+    return
+}
+
+linetos_dtrace()
+{
+    local name args arglist state
+    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
+probe qemu.$name = process("qemu").mark("$name")
+{
+EOF
+
+    i=1
+    for arg in $arglist
+    do
+        cat <<EOF
+  $arg = \$arg$i;
+EOF
+	i="$((i+1))"
+    done
+
+    cat <<EOF
+}
+EOF
+}
+
+linetos_end_dtrace()
+{
+    return
+}
+
 # Process stdin by calling begin, line, and end functions for the backend
 convert()
 {
@@ -324,9 +450,10 @@  convert()
         disable=${str%%disable *}
         echo
         if test -z "$disable"; then
-            # Pass the disabled state as an arg to lineto$1_simple().
-            # For all other cases, call lineto$1_nop()
-            if [ $backend = "simple" ]; 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 }"
@@ -360,9 +487,29 @@  tracetoc()
     convert c
 }
 
+tracetod()
+{
+    if [ $backend != "dtrace" ]; then
+       echo "DTrace probe generator not applicable to $backend backend"
+       exit 1
+    fi
+    echo "/* This file is autogenerated by tracetool, do not edit. */"
+    convert d
+}
+
+tracetos()
+{
+    if [ $backend != "dtrace" ]; then
+       echo "SystemTAP tapset generator not applicable to $backend backend"
+       exit 1
+    fi
+    echo "/* This file is autogenerated by tracetool, do not edit. */"
+    convert s
+}
+
 # Choose backend
 case "$1" in
-"--nop" | "--simple" | "--ust") backend="${1#--}" ;;
+"--nop" | "--simple" | "--ust" | "--dtrace") backend="${1#--}" ;;
 *) usage ;;
 esac
 shift
@@ -370,6 +517,8 @@  shift
 case "$1" in
 "-h") tracetoh ;;
 "-c") tracetoc ;;
+"-d") tracetod ;;
+"-s") tracetos ;;
 "--check-backend") exit 0 ;; # used by ./configure to test for backend
 *) usage ;;
 esac