qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/4] Tracing patches
@ 2013-05-03 12:01 Stefan Hajnoczi
  2013-05-03 12:01 ` [Qemu-devel] [PATCH 1/4] kvm-all: add kvm_ioctl, kvm_vm_ioctl, kvm_vcpu_ioctl tracepoints Stefan Hajnoczi
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2013-05-03 12:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Stefan Hajnoczi

This tracing pull request is long overdue for QEMU 1.5.

Eiichi Tsukata's ftrace backend makes it easy to correlate QEMU events with
host kernel events.  He also reports good performance.

Kazuya Saito's trace events make it easier to observe the KVM run loop.

The following changes since commit 8ca27ce2e1150486ea2db4116a03706b28294f16:

  Merge remote-tracking branch 'afaerber/qom-cpu' into staging (2013-05-02 10:57:01 -0500)

are available in the git repository at:


  git://github.com/stefanha/qemu.git tracing

for you to fetch changes up to e64dd5efb2c6d522a3bc9d096cd49a4e53f0ae10:

  trace: document ftrace backend (2013-05-03 13:58:09 +0200)

----------------------------------------------------------------
Eiichi Tsukata (2):
      trace: Add ftrace tracing backend
      trace: document ftrace backend

Kazuya Saito (2):
      kvm-all: add kvm_ioctl, kvm_vm_ioctl, kvm_vcpu_ioctl tracepoints
      kvm-all: add kvm_run_exit tracepoint

 configure                           |   8 +++
 docs/tracing.txt                    |  16 ++++++
 kvm-all.c                           |   5 ++
 scripts/tracetool/backend/ftrace.py |  54 +++++++++++++++++++
 trace-events                        |   7 +++
 trace/Makefile.objs                 |   1 +
 trace/ftrace.c                      | 102 ++++++++++++++++++++++++++++++++++++
 trace/ftrace.h                      |  10 ++++
 8 files changed, 203 insertions(+)
 create mode 100644 scripts/tracetool/backend/ftrace.py
 create mode 100644 trace/ftrace.c
 create mode 100644 trace/ftrace.h

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 1/4] kvm-all: add kvm_ioctl, kvm_vm_ioctl, kvm_vcpu_ioctl tracepoints
  2013-05-03 12:01 [Qemu-devel] [PULL 0/4] Tracing patches Stefan Hajnoczi
@ 2013-05-03 12:01 ` Stefan Hajnoczi
  2013-05-03 12:12   ` Andreas Färber
  2013-05-03 12:01 ` [Qemu-devel] [PATCH 2/4] kvm-all: add kvm_run_exit tracepoint Stefan Hajnoczi
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Stefan Hajnoczi @ 2013-05-03 12:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Kazuya Saito, Stefan Hajnoczi

From: Kazuya Saito <saito.kazuya@jp.fujitsu.com>

This patch adds tracepoints at ioctl to kvm. Tracing these ioctl is
useful for clarification whether the cause of troubles is qemu or kvm.

Signed-off-by: Kazuya Saito <saito.kazuya@jp.fujitsu.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 kvm-all.c    | 4 ++++
 trace-events | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/kvm-all.c b/kvm-all.c
index f6c0f4a..4f73b98 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -33,6 +33,7 @@
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
 #include "qemu/event_notifier.h"
+#include "trace.h"
 
 /* This check must be after config-host.h is included */
 #ifdef CONFIG_EVENTFD
@@ -1687,6 +1688,7 @@ int kvm_ioctl(KVMState *s, int type, ...)
     arg = va_arg(ap, void *);
     va_end(ap);
 
+    trace_kvm_ioctl(type, arg);
     ret = ioctl(s->fd, type, arg);
     if (ret == -1) {
         ret = -errno;
@@ -1704,6 +1706,7 @@ int kvm_vm_ioctl(KVMState *s, int type, ...)
     arg = va_arg(ap, void *);
     va_end(ap);
 
+    trace_kvm_vm_ioctl(type, arg);
     ret = ioctl(s->vmfd, type, arg);
     if (ret == -1) {
         ret = -errno;
@@ -1721,6 +1724,7 @@ int kvm_vcpu_ioctl(CPUState *cpu, int type, ...)
     arg = va_arg(ap, void *);
     va_end(ap);
 
+    trace_kvm_vcpu_ioctl(cpu->cpu_index, type, arg);
     ret = ioctl(cpu->kvm_fd, type, arg);
     if (ret == -1) {
         ret = -errno;
diff --git a/trace-events b/trace-events
index 55e80be..d5bc7a5 100644
--- a/trace-events
+++ b/trace-events
@@ -1153,3 +1153,8 @@ virtio_ccw_new_device(int cssid, int ssid, int schid, int devno, const char *dev
 
 # migration.c
 migrate_set_state(int new_state) "new state %d"
+
+# kvm-all.c
+kvm_ioctl(int type, void *arg) "type %d, arg %p"
+kvm_vm_ioctl(int type, void *arg) "type %d, arg %p"
+kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type %d, arg %p"
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/4] kvm-all: add kvm_run_exit tracepoint
  2013-05-03 12:01 [Qemu-devel] [PULL 0/4] Tracing patches Stefan Hajnoczi
  2013-05-03 12:01 ` [Qemu-devel] [PATCH 1/4] kvm-all: add kvm_ioctl, kvm_vm_ioctl, kvm_vcpu_ioctl tracepoints Stefan Hajnoczi
@ 2013-05-03 12:01 ` Stefan Hajnoczi
  2013-05-03 12:01 ` [Qemu-devel] [PATCH 3/4] trace: Add ftrace tracing backend Stefan Hajnoczi
  2013-05-03 12:01 ` [Qemu-devel] [PATCH 4/4] trace: document ftrace backend Stefan Hajnoczi
  3 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2013-05-03 12:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Kazuya Saito, Stefan Hajnoczi

From: Kazuya Saito <saito.kazuya@jp.fujitsu.com>

This patch enable us to know exit reason of KVM_RUN. It will help us
know where the trouble is caused.

Signed-off-by: Kazuya Saito <saito.kazuya@jp.fujitsu.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 kvm-all.c    | 1 +
 trace-events | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/kvm-all.c b/kvm-all.c
index 4f73b98..3a31602 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1627,6 +1627,7 @@ int kvm_cpu_exec(CPUArchState *env)
             abort();
         }
 
+        trace_kvm_run_exit(cpu->cpu_index, run->exit_reason);
         switch (run->exit_reason) {
         case KVM_EXIT_IO:
             DPRINTF("handle_io\n");
diff --git a/trace-events b/trace-events
index d5bc7a5..17d75ab 100644
--- a/trace-events
+++ b/trace-events
@@ -1158,3 +1158,5 @@ migrate_set_state(int new_state) "new state %d"
 kvm_ioctl(int type, void *arg) "type %d, arg %p"
 kvm_vm_ioctl(int type, void *arg) "type %d, arg %p"
 kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type %d, arg %p"
+kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, reason %d"
+
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 3/4] trace: Add ftrace tracing backend
  2013-05-03 12:01 [Qemu-devel] [PULL 0/4] Tracing patches Stefan Hajnoczi
  2013-05-03 12:01 ` [Qemu-devel] [PATCH 1/4] kvm-all: add kvm_ioctl, kvm_vm_ioctl, kvm_vcpu_ioctl tracepoints Stefan Hajnoczi
  2013-05-03 12:01 ` [Qemu-devel] [PATCH 2/4] kvm-all: add kvm_run_exit tracepoint Stefan Hajnoczi
@ 2013-05-03 12:01 ` Stefan Hajnoczi
  2013-05-03 12:01 ` [Qemu-devel] [PATCH 4/4] trace: document ftrace backend Stefan Hajnoczi
  3 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2013-05-03 12:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Eiichi Tsukata, Stefan Hajnoczi

From: Eiichi Tsukata <eiichi.tsukata.xh@hitachi.com>

This patch adds a ftrace tracing backend which sends trace event to
ftrace marker file. You can effectively compare qemu trace data and
kernel(especially, kvm.ko when using KVM) trace data.
The ftrace backend is restricted to Linux only.

To try out the ftrace backend:

 $ ./configure --trace-backend=ftrace
 $ make

if you use KVM, enable kvm events in ftrace:

 # sudo echo 1 > /sys/kernel/debug/tracing/events/kvm/enable

After running qemu by root user, you can get the trace:

 # cat /sys/kernel/debug/tracing/trace

Signed-off-by: Eiichi Tsukata <eiichi.tsukata.xh@hitachi.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 configure                           |   8 +++
 scripts/tracetool/backend/ftrace.py |  54 +++++++++++++++++++
 trace/Makefile.objs                 |   1 +
 trace/ftrace.c                      | 102 ++++++++++++++++++++++++++++++++++++
 trace/ftrace.h                      |  10 ++++
 5 files changed, 175 insertions(+)
 create mode 100644 scripts/tracetool/backend/ftrace.py
 create mode 100644 trace/ftrace.c
 create mode 100644 trace/ftrace.h

diff --git a/configure b/configure
index c4d85ba..e818e8b 100755
--- a/configure
+++ b/configure
@@ -4004,6 +4004,14 @@ if test "$trace_backend" = "dtrace"; then
     echo "CONFIG_TRACE_SYSTEMTAP=y" >> $config_host_mak
   fi
 fi
+if test "$trace_backend" = "ftrace"; then
+  if test "$linux" = "yes" ; then
+    echo "CONFIG_TRACE_FTRACE=y" >> $config_host_mak
+    trace_default=no
+  else
+    feature_not_found "ftrace(trace backend)"
+  fi
+fi
 echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
 if test "$trace_default" = "yes"; then
   echo "CONFIG_TRACE_DEFAULT=y" >> $config_host_mak
diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
new file mode 100644
index 0000000..888c361
--- /dev/null
+++ b/scripts/tracetool/backend/ftrace.py
@@ -0,0 +1,54 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Ftrace built-in backend.
+"""
+
+__author__     = "Eiichi Tsukata <eiichi.tsukata.xh@hitachi.com>"
+__copyright__  = "Copyright (C) 2013 Hitachi, Ltd."
+__license__    = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__      = "stefanha@redhat.com"
+
+
+from tracetool import out
+
+
+PUBLIC = True
+
+
+def c(events):
+    pass
+
+def h(events):
+    out('#include "trace/ftrace.h"',
+        '#include "trace/control.h"',
+        '',
+        )
+
+    for e in events:
+        argnames = ", ".join(e.args.names())
+        if len(e.args) > 0:
+            argnames = ", " + argnames
+
+        out('static inline void trace_%(name)s(%(args)s)',
+            '{',
+            '    char ftrace_buf[MAX_TRACE_STRLEN];',
+            '    int unused __attribute__ ((unused));',
+            '    int trlen;',
+            '    bool _state = trace_event_get_state(%(event_id)s);',
+            '    if (_state) {',
+            '        trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,',
+            '                         "%(name)s " %(fmt)s "\\n" %(argnames)s);',
+            '        trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
+            '        unused = write(trace_marker_fd, ftrace_buf, trlen);',
+            '    }',
+            '}',
+            name = e.name,
+            args = e.args,
+            event_id = "TRACE_" + e.name.upper(),
+            fmt = e.fmt.rstrip("\n"),
+            argnames = argnames,
+            )
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index a043072..3b88e49 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -76,5 +76,6 @@ endif
 util-obj-$(CONFIG_TRACE_DEFAULT) += default.o
 util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o
 util-obj-$(CONFIG_TRACE_STDERR) += stderr.o
+util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o
 util-obj-y += control.o
 util-obj-y += generated-tracers.o
diff --git a/trace/ftrace.c b/trace/ftrace.c
new file mode 100644
index 0000000..46b7fdb
--- /dev/null
+++ b/trace/ftrace.c
@@ -0,0 +1,102 @@
+/*
+ * Ftrace trace backend
+ *
+ * Copyright (C) 2013 Hitachi, Ltd.
+ * Created by Eiichi Tsukata <eiichi.tsukata.xh@hitachi.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+#include <limits.h>
+#include "trace.h"
+#include "trace/control.h"
+
+int trace_marker_fd;
+
+static int find_debugfs(char *debugfs)
+{
+    char type[100];
+    FILE *fp;
+
+    fp = fopen("/proc/mounts", "r");
+    if (fp == NULL) {
+        return 0;
+    }
+
+    while (fscanf(fp, "%*s %" STR(PATH_MAX) "s %99s %*s %*d %*d\n",
+                  debugfs, type) == 2) {
+        if (strcmp(type, "debugfs") == 0) {
+            break;
+        }
+    }
+    fclose(fp);
+
+    if (strcmp(type, "debugfs") != 0) {
+        return 0;
+    }
+    return 1;
+}
+
+void trace_print_events(FILE *stream, fprintf_function stream_printf)
+{
+    TraceEventID i;
+
+    for (i = 0; i < trace_event_count(); i++) {
+        TraceEvent *ev = trace_event_id(i);
+        stream_printf(stream, "%s [Event ID %u] : state %u\n",
+                      trace_event_get_name(ev), i, trace_event_get_state_dynamic(ev));
+    }
+}
+
+void trace_event_set_state_dynamic_backend(TraceEvent *ev, bool state)
+{
+    ev->dstate = state;
+}
+
+bool trace_backend_init(const char *events, const char *file)
+{
+    char debugfs[PATH_MAX];
+    char path[PATH_MAX];
+    int debugfs_found;
+    int trace_fd = -1;
+
+    if (file) {
+        fprintf(stderr, "error: -trace file=...: "
+                "option not supported by the selected tracing backend\n");
+        return false;
+    }
+
+    debugfs_found = find_debugfs(debugfs);
+    if (debugfs_found) {
+        snprintf(path, PATH_MAX, "%s/tracing/tracing_on", debugfs);
+        trace_fd = open(path, O_WRONLY);
+        if (trace_fd < 0) {
+            perror("Could not open ftrace 'tracing_on' file");
+            return false;
+        } else {
+            if (write(trace_fd, "1", 1) < 0) {
+                perror("Could not write to 'tracing_on' file");
+                close(trace_fd);
+                return false;
+            }
+            close(trace_fd);
+        }
+        snprintf(path, PATH_MAX, "%s/tracing/trace_marker", debugfs);
+        trace_marker_fd = open(path, O_WRONLY);
+        if (trace_marker_fd < 0) {
+            perror("Could not open ftrace 'trace_marker' file");
+            return false;
+        }
+    } else {
+        fprintf(stderr, "debugfs is not mounted\n");
+        return false;
+    }
+
+    trace_backend_init_events(events);
+    return true;
+}
diff --git a/trace/ftrace.h b/trace/ftrace.h
new file mode 100644
index 0000000..94cb8d5
--- /dev/null
+++ b/trace/ftrace.h
@@ -0,0 +1,10 @@
+#ifndef TRACE_FTRACE_H
+#define TRACE_FTRACE_H
+
+#define MAX_TRACE_STRLEN 512
+#define _STR(x) #x
+#define STR(x) _STR(x)
+
+extern int trace_marker_fd;
+
+#endif /* ! TRACE_FTRACE_H */
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 4/4] trace: document ftrace backend
  2013-05-03 12:01 [Qemu-devel] [PULL 0/4] Tracing patches Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2013-05-03 12:01 ` [Qemu-devel] [PATCH 3/4] trace: Add ftrace tracing backend Stefan Hajnoczi
@ 2013-05-03 12:01 ` Stefan Hajnoczi
  3 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2013-05-03 12:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Eiichi Tsukata, Stefan Hajnoczi

From: Eiichi Tsukata <eiichi.tsukata.xh@hitachi.com>

Add documentation of ftrace backend.

Signed-off-by: Eiichi Tsukata <eiichi.tsukata.xh@hitachi.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/tracing.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index cf53c17..60ff9c5 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -175,6 +175,22 @@ unless you have specific needs for more advanced backends.
 The "simple" backend currently does not capture string arguments, it simply
 records the char* pointer value instead of the string that is pointed to.
 
+=== Ftrace ===
+
+The "ftrace" backend writes trace data to ftrace marker. This effectively
+sends trace events to ftrace ring buffer, and you can compare qemu trace
+data and kernel(especially kvm.ko when using KVM) trace data.
+
+if you use KVM, enable kvm events in ftrace:
+
+   # echo 1 > /sys/kernel/debug/tracing/events/kvm/enable
+
+After running qemu by root user, you can get the trace:
+
+   # cat /sys/kernel/debug/tracing/trace
+
+Restriction: "ftrace" backend is restricted to Linux only.
+
 ==== Monitor commands ====
 
 * trace-file on|off|flush|set <path>
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] kvm-all: add kvm_ioctl, kvm_vm_ioctl, kvm_vcpu_ioctl tracepoints
  2013-05-03 12:01 ` [Qemu-devel] [PATCH 1/4] kvm-all: add kvm_ioctl, kvm_vm_ioctl, kvm_vcpu_ioctl tracepoints Stefan Hajnoczi
@ 2013-05-03 12:12   ` Andreas Färber
  2013-05-03 13:31     ` Eduardo Habkost
  2013-05-08  8:48     ` Stefan Hajnoczi
  0 siblings, 2 replies; 8+ messages in thread
From: Andreas Färber @ 2013-05-03 12:12 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Igor Mammedov, Anthony Liguori, qemu-devel, Eduardo Habkost,
	Kazuya Saito

Am 03.05.2013 14:01, schrieb Stefan Hajnoczi:
> From: Kazuya Saito <saito.kazuya@jp.fujitsu.com>
> 
> This patch adds tracepoints at ioctl to kvm. Tracing these ioctl is
> useful for clarification whether the cause of troubles is qemu or kvm.
> 
> Signed-off-by: Kazuya Saito <saito.kazuya@jp.fujitsu.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  kvm-all.c    | 4 ++++
>  trace-events | 5 +++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index f6c0f4a..4f73b98 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -33,6 +33,7 @@
>  #include "exec/memory.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/event_notifier.h"
> +#include "trace.h"
>  
>  /* This check must be after config-host.h is included */
>  #ifdef CONFIG_EVENTFD
> @@ -1687,6 +1688,7 @@ int kvm_ioctl(KVMState *s, int type, ...)
>      arg = va_arg(ap, void *);
>      va_end(ap);
>  
> +    trace_kvm_ioctl(type, arg);
>      ret = ioctl(s->fd, type, arg);
>      if (ret == -1) {
>          ret = -errno;
> @@ -1704,6 +1706,7 @@ int kvm_vm_ioctl(KVMState *s, int type, ...)
>      arg = va_arg(ap, void *);
>      va_end(ap);
>  
> +    trace_kvm_vm_ioctl(type, arg);
>      ret = ioctl(s->vmfd, type, arg);
>      if (ret == -1) {
>          ret = -errno;
> @@ -1721,6 +1724,7 @@ int kvm_vcpu_ioctl(CPUState *cpu, int type, ...)
>      arg = va_arg(ap, void *);
>      va_end(ap);
>  
> +    trace_kvm_vcpu_ioctl(cpu->cpu_index, type, arg);
>      ret = ioctl(cpu->kvm_fd, type, arg);
>      if (ret == -1) {
>          ret = -errno;
> diff --git a/trace-events b/trace-events
> index 55e80be..d5bc7a5 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1153,3 +1153,8 @@ virtio_ccw_new_device(int cssid, int ssid, int schid, int devno, const char *dev
>  
>  # migration.c
>  migrate_set_state(int new_state) "new state %d"
> +
> +# kvm-all.c
> +kvm_ioctl(int type, void *arg) "type %d, arg %p"
> +kvm_vm_ioctl(int type, void *arg) "type %d, arg %p"
> +kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type %d, arg %p"

Sorry that I'm just seeing this patch now (wasn't CC'ed), but I wonder
whether cpu_index is the best thing to trace here? Can we still change
trace event API or would we have to nack/change now?

CC'ing Igor since he just introduced a cpu_get_arch_id() and there's
also a kvm_arch_vcpu_id() introduced earlier by Eduardo.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] kvm-all: add kvm_ioctl, kvm_vm_ioctl, kvm_vcpu_ioctl tracepoints
  2013-05-03 12:12   ` Andreas Färber
@ 2013-05-03 13:31     ` Eduardo Habkost
  2013-05-08  8:48     ` Stefan Hajnoczi
  1 sibling, 0 replies; 8+ messages in thread
From: Eduardo Habkost @ 2013-05-03 13:31 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Igor Mammedov, Anthony Liguori, qemu-devel, Stefan Hajnoczi,
	Kazuya Saito

On Fri, May 03, 2013 at 02:12:14PM +0200, Andreas Färber wrote:
> Am 03.05.2013 14:01, schrieb Stefan Hajnoczi:
> > From: Kazuya Saito <saito.kazuya@jp.fujitsu.com>
> > 
> > This patch adds tracepoints at ioctl to kvm. Tracing these ioctl is
> > useful for clarification whether the cause of troubles is qemu or kvm.
> > 
> > Signed-off-by: Kazuya Saito <saito.kazuya@jp.fujitsu.com>
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  kvm-all.c    | 4 ++++
> >  trace-events | 5 +++++
> >  2 files changed, 9 insertions(+)
> > 
> > diff --git a/kvm-all.c b/kvm-all.c
> > index f6c0f4a..4f73b98 100644
> > --- a/kvm-all.c
> > +++ b/kvm-all.c
> > @@ -33,6 +33,7 @@
> >  #include "exec/memory.h"
> >  #include "exec/address-spaces.h"
> >  #include "qemu/event_notifier.h"
> > +#include "trace.h"
> >  
> >  /* This check must be after config-host.h is included */
> >  #ifdef CONFIG_EVENTFD
> > @@ -1687,6 +1688,7 @@ int kvm_ioctl(KVMState *s, int type, ...)
> >      arg = va_arg(ap, void *);
> >      va_end(ap);
> >  
> > +    trace_kvm_ioctl(type, arg);
> >      ret = ioctl(s->fd, type, arg);
> >      if (ret == -1) {
> >          ret = -errno;
> > @@ -1704,6 +1706,7 @@ int kvm_vm_ioctl(KVMState *s, int type, ...)
> >      arg = va_arg(ap, void *);
> >      va_end(ap);
> >  
> > +    trace_kvm_vm_ioctl(type, arg);
> >      ret = ioctl(s->vmfd, type, arg);
> >      if (ret == -1) {
> >          ret = -errno;
> > @@ -1721,6 +1724,7 @@ int kvm_vcpu_ioctl(CPUState *cpu, int type, ...)
> >      arg = va_arg(ap, void *);
> >      va_end(ap);
> >  
> > +    trace_kvm_vcpu_ioctl(cpu->cpu_index, type, arg);
> >      ret = ioctl(cpu->kvm_fd, type, arg);
> >      if (ret == -1) {
> >          ret = -errno;
> > diff --git a/trace-events b/trace-events
> > index 55e80be..d5bc7a5 100644
> > --- a/trace-events
> > +++ b/trace-events
> > @@ -1153,3 +1153,8 @@ virtio_ccw_new_device(int cssid, int ssid, int schid, int devno, const char *dev
> >  
> >  # migration.c
> >  migrate_set_state(int new_state) "new state %d"
> > +
> > +# kvm-all.c
> > +kvm_ioctl(int type, void *arg) "type %d, arg %p"
> > +kvm_vm_ioctl(int type, void *arg) "type %d, arg %p"
> > +kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type %d, arg %p"
> 
> Sorry that I'm just seeing this patch now (wasn't CC'ed), but I wonder
> whether cpu_index is the best thing to trace here? Can we still change
> trace event API or would we have to nack/change now?
> 
> CC'ing Igor since he just introduced a cpu_get_arch_id() and there's
> also a kvm_arch_vcpu_id() introduced earlier by Eduardo.

Being kvm_vcpu_ioctl() a very low-level KVM function, I believe the KVM
VCPU "id" (the argument passed to KVM_CREATE_VCPU, that needs to be the
APIC ID on x86, and is returned by kvm_arch_vcpu_id()) is the best CPU
identifier to be included here.

cpu_index is the most ambiguous and least reliable CPU identifier we
have today. I wouldn't use it in any new code.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] kvm-all: add kvm_ioctl, kvm_vm_ioctl, kvm_vcpu_ioctl tracepoints
  2013-05-03 12:12   ` Andreas Färber
  2013-05-03 13:31     ` Eduardo Habkost
@ 2013-05-08  8:48     ` Stefan Hajnoczi
  1 sibling, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2013-05-08  8:48 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Igor Mammedov, Anthony Liguori, qemu-devel, Eduardo Habkost,
	Kazuya Saito

On Fri, May 03, 2013 at 02:12:14PM +0200, Andreas Färber wrote:
> > +# kvm-all.c
> > +kvm_ioctl(int type, void *arg) "type %d, arg %p"
> > +kvm_vm_ioctl(int type, void *arg) "type %d, arg %p"
> > +kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type %d, arg %p"
> 
> Sorry that I'm just seeing this patch now (wasn't CC'ed), but I wonder
> whether cpu_index is the best thing to trace here? Can we still change
> trace event API or would we have to nack/change now?

Trace events are not stable.  We can change them.

STefan

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-05-08  8:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-03 12:01 [Qemu-devel] [PULL 0/4] Tracing patches Stefan Hajnoczi
2013-05-03 12:01 ` [Qemu-devel] [PATCH 1/4] kvm-all: add kvm_ioctl, kvm_vm_ioctl, kvm_vcpu_ioctl tracepoints Stefan Hajnoczi
2013-05-03 12:12   ` Andreas Färber
2013-05-03 13:31     ` Eduardo Habkost
2013-05-08  8:48     ` Stefan Hajnoczi
2013-05-03 12:01 ` [Qemu-devel] [PATCH 2/4] kvm-all: add kvm_run_exit tracepoint Stefan Hajnoczi
2013-05-03 12:01 ` [Qemu-devel] [PATCH 3/4] trace: Add ftrace tracing backend Stefan Hajnoczi
2013-05-03 12:01 ` [Qemu-devel] [PATCH 4/4] trace: document ftrace backend Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).