* [Qemu-devel] [PATCH v4 0/6] trace: Per-vCPU tracing states
@ 2016-06-14 13:10 Lluís Vilanova
2016-06-14 13:10 ` [Qemu-devel] [PATCH v4 1/6] trace: Identify events with the 'vcpu' property Lluís Vilanova
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-14 13:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Eric Blake, Eduardo Habkost, Stefan Hajnoczi
NOTE: This series applies on top of "trace: Show vCPU info in guest code events"
Provides per-vCPU dynamic controls of the tracing state of events with the
"vcpu" property.
A later series proposes an optimization where tracing code can be elided for
dynamically disabled events (it uses multiple virtual TB caches optimized for
the current tracing state of the executing vCPU).
Changes in v4
=============
* Make trace_events_dstate an uint16_t [Paolo Bonzini].
* Revert trace_event_get_state_dynamic_by_id [Stefan Hajnoczi].
* Do not add superfluous asserts [Paolo Bonzini].
* Replace cpu -> vcpu in function names and arguments [Paolo Bonzini].
* Add 'trace_event_is_vcpu' [Paolo Bonzini].
* Remove 'trace_event_cpu_count' [Paolo Bonzini].
Changes in v3
=============
* Update QAPI version annotations [Eric Blake].
Changes in v2
=============
* Rebase on 9bbbf64.
* Fix removal of macro '_' in all target architectures.
* Document behaviour of 'trace_events_dstate'.
* Use a proper bitmap for CPUState::trace_dstate [Stefan Hajnoczi].
Changes in v1
=============
* Rebase on 1b16240.
* Split from v4 of "trace: Per-vCPU tracing states".
* Simplify event state initialization.
* Simplify logic deciding which events are treated by this patch (previously,
execution-time events with 'tcg' and 'vcpu' properties; now it's simply events
with the 'vcpu' property).
* Make tracing backends comply with the per-vCPU tracing state.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
Lluís Vilanova (6):
trace: Identify events with the 'vcpu' property
disas: Remove unused macro '_'
[trivial] trace: Cosmetic changes on fast-path tracing
trace: Add per-vCPU tracing states for events with the 'vcpu' property
trace: Conditionally trace events based on their per-vCPU state
trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state
Makefile.objs | 1
bsd-user/main.c | 2
disas/alpha.c | 6 +
disas/arm.c | 2
disas/i386.c | 2
disas/m68k.c | 4 -
disas/mips.c | 50 ++++++------
disas/ppc.c | 22 +++--
disas/sparc.c | 6 +
include/disas/bfd.h | 1
include/qom/cpu.h | 6 +
linux-user/main.c | 2
monitor.c | 4 -
qapi/trace.json | 20 ++++-
qmp-commands.hx | 17 ++++
qom/cpu.c | 1
scripts/tracetool/backend/dtrace.py | 4 -
scripts/tracetool/backend/ftrace.py | 20 ++---
scripts/tracetool/backend/log.py | 26 ++++--
scripts/tracetool/backend/simple.py | 13 ++-
scripts/tracetool/backend/ust.py | 4 -
scripts/tracetool/format/events_c.py | 11 ++-
scripts/tracetool/format/events_h.py | 12 +++
scripts/tracetool/format/h.py | 18 ++++
trace/Makefile.objs | 26 ++++++
trace/control-internal.h | 47 +++++++++--
trace/control-stub.c | 28 +++++++
trace/control-target.c | 53 +++++++++++++
trace/control.c | 29 ++++++-
trace/control.h | 78 ++++++++++++++++++-
trace/event-internal.h | 4 +
trace/qmp.c | 143 +++++++++++++++++++++++++++-------
translate-all.h | 3 +
vl.c | 1
34 files changed, 536 insertions(+), 130 deletions(-)
create mode 100644 trace/control-stub.c
create mode 100644 trace/control-target.c
To: qemu-devel@nongnu.org
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v4 1/6] trace: Identify events with the 'vcpu' property
2016-06-14 13:10 [Qemu-devel] [PATCH v4 0/6] trace: Per-vCPU tracing states Lluís Vilanova
@ 2016-06-14 13:10 ` Lluís Vilanova
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 2/6] disas: Remove unused macro '_' Lluís Vilanova
` (4 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-14 13:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Eric Blake, Eduardo Habkost, Stefan Hajnoczi
A new event attribute 'cpu_id' is added to have a separate ID
space ('TRACE_VCPU_*') for all events with the 'vcpu' property.
These are later used to identify which events are enabled on each vCPU.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
scripts/tracetool/format/events_c.py | 11 +++++++++--
scripts/tracetool/format/events_h.py | 12 +++++++++++-
trace/control-internal.h | 12 +++++++++++-
trace/control.h | 17 +++++++++++++++++
trace/event-internal.h | 4 +++-
5 files changed, 51 insertions(+), 5 deletions(-)
diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/format/events_c.py
index 1cc6a49..4012063 100644
--- a/scripts/tracetool/format/events_c.py
+++ b/scripts/tracetool/format/events_c.py
@@ -6,7 +6,7 @@ trace/generated-events.c
"""
__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__ = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2016, Lluís Vilanova <vilanova@ac.upc.edu>"
__license__ = "GPL version 2 or (at your option) any later version"
__maintainer__ = "Stefan Hajnoczi"
@@ -28,8 +28,15 @@ def generate(events, backend):
out('TraceEvent trace_events[TRACE_EVENT_COUNT] = {')
for e in events:
- out(' { .id = %(id)s, .name = \"%(name)s\", .sstate = %(sstate)s },',
+ if "vcpu" in e.properties:
+ vcpu_id = "TRACE_VCPU_" + e.name.upper()
+ else:
+ vcpu_id = "TRACE_VCPU_EVENT_COUNT"
+ out(' { .id = %(id)s, .vcpu_id = %(vcpu_id)s,'
+ ' .name = \"%(name)s\",'
+ ' .sstate = %(sstate)s },',
id = "TRACE_" + e.name.upper(),
+ vcpu_id = vcpu_id,
name = e.name,
sstate = "TRACE_%s_ENABLED" % e.name.upper())
diff --git a/scripts/tracetool/format/events_h.py b/scripts/tracetool/format/events_h.py
index 4529263..a9da60b 100644
--- a/scripts/tracetool/format/events_h.py
+++ b/scripts/tracetool/format/events_h.py
@@ -32,13 +32,23 @@ def generate(events, backend):
out(' TRACE_EVENT_COUNT',
'} TraceEventID;')
+ # per-vCPU event identifiers
+ out('typedef enum {')
+
+ for e in events:
+ if "vcpu" in e.properties:
+ out(' TRACE_VCPU_%s,' % e.name.upper())
+
+ out(' TRACE_VCPU_EVENT_COUNT',
+ '} TraceEventVCPUID;')
+
# static state
for e in events:
if 'disable' in e.properties:
enabled = 0
else:
enabled = 1
- if "tcg-trans" in e.properties:
+ if "tcg-exec" in e.properties:
# a single define for the two "sub-events"
out('#define TRACE_%(name)s_ENABLED %(enabled)d',
name=e.original.name.upper(),
diff --git a/trace/control-internal.h b/trace/control-internal.h
index dcf67f5..0ba10fe 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -1,7 +1,7 @@
/*
* Interface for configuring and controlling the state of tracing events.
*
- * Copyright (C) 2011-2014 Lluís Vilanova <vilanova@ac.upc.edu>
+ * Copyright (C) 2011-2016 Lluís Vilanova <vilanova@ac.upc.edu>
*
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
@@ -40,6 +40,16 @@ static inline TraceEventID trace_event_get_id(TraceEvent *ev)
return ev->id;
}
+static inline TraceEventVCPUID trace_event_get_vcpu_id(TraceEvent *ev)
+{
+ return ev->vcpu_id;
+}
+
+static inline bool trace_event_is_vcpu(TraceEvent *ev)
+{
+ return ev->vcpu_id == TRACE_VCPU_EVENT_COUNT;
+}
+
static inline const char * trace_event_get_name(TraceEvent *ev)
{
assert(ev != NULL);
diff --git a/trace/control.h b/trace/control.h
index e2ba6d4..d37bce7 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -86,6 +86,23 @@ static TraceEventID trace_event_count(void);
static TraceEventID trace_event_get_id(TraceEvent *ev);
/**
+ * trace_event_get_vcpu_id:
+ *
+ * Get the per-vCPU identifier of an event.
+ *
+ * Special value #TRACE_VCPU_EVENT_COUNT means the event is not vCPU-specific
+ * (does not have the "vcpu" property).
+ */
+static TraceEventVCPUID trace_event_get_vcpu_id(TraceEvent *ev);
+
+/**
+ * trace_event_is_vcpu:
+ *
+ * Whether this is a per-vCPU event.
+ */
+static bool trace_event_is_vcpu(TraceEvent *ev);
+
+/**
* trace_event_get_name:
*
* Get the name of an event.
diff --git a/trace/event-internal.h b/trace/event-internal.h
index 86f6a51..202781a 100644
--- a/trace/event-internal.h
+++ b/trace/event-internal.h
@@ -1,7 +1,7 @@
/*
* Interface for configuring and controlling the state of tracing events.
*
- * Copyright (C) 2012 Lluís Vilanova <vilanova@ac.upc.edu>
+ * Copyright (C) 2012-2016 Lluís Vilanova <vilanova@ac.upc.edu>
*
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
@@ -16,6 +16,7 @@
/**
* TraceEvent:
* @id: Unique event identifier.
+ * @vcpu_id: Unique per-vCPU event identifier.
* @name: Event name.
* @sstate: Static tracing state.
*
@@ -23,6 +24,7 @@
*/
typedef struct TraceEvent {
TraceEventID id;
+ TraceEventVCPUID vcpu_id;
const char * name;
const bool sstate;
} TraceEvent;
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v4 2/6] disas: Remove unused macro '_'
2016-06-14 13:10 [Qemu-devel] [PATCH v4 0/6] trace: Per-vCPU tracing states Lluís Vilanova
2016-06-14 13:10 ` [Qemu-devel] [PATCH v4 1/6] trace: Identify events with the 'vcpu' property Lluís Vilanova
@ 2016-06-14 13:11 ` Lluís Vilanova
2016-06-17 15:25 ` Stefan Hajnoczi
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 3/6] [trivial] trace: Cosmetic changes on fast-path tracing Lluís Vilanova
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-14 13:11 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Eric Blake, Eduardo Habkost, Stefan Hajnoczi,
Richard Henderson, Andrzej Zaborowski, Peter Maydell,
Aurelien Jarno, Leon Alrae, David Gibson, Alexander Graf,
Vassili Karpov (malc), Blue Swirl, Mark Cave-Ayland,
open list:ARM target, open list:PowerPC
Eliminates a future compilation error when UI code includes the tracing
headers (indirectly pulling "disas/bfd.h" through "qom/cpu.h") and
GLib's i18n '_' macro.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
disas/alpha.c | 6 +++---
disas/arm.c | 2 +-
disas/i386.c | 2 +-
disas/m68k.c | 4 ++--
disas/mips.c | 50 +++++++++++++++++++++++++-------------------------
disas/ppc.c | 22 +++++++++++-----------
disas/sparc.c | 6 +++---
include/disas/bfd.h | 1 -
8 files changed, 46 insertions(+), 47 deletions(-)
diff --git a/disas/alpha.c b/disas/alpha.c
index 44d00a3..b7b0ae0 100644
--- a/disas/alpha.c
+++ b/disas/alpha.c
@@ -521,7 +521,7 @@ static unsigned
insert_bdisp(unsigned insn, int value, const char **errmsg)
{
if (errmsg != (const char **)NULL && (value & 3))
- *errmsg = _("branch operand unaligned");
+ *errmsg = "branch operand unaligned";
return insn | ((value / 4) & 0x1FFFFF);
}
@@ -539,7 +539,7 @@ static unsigned
insert_jhint(unsigned insn, int value, const char **errmsg)
{
if (errmsg != (const char **)NULL && (value & 3))
- *errmsg = _("jump hint unaligned");
+ *errmsg = "jump hint unaligned";
return insn | ((value / 4) & 0x3FFF);
}
@@ -556,7 +556,7 @@ static unsigned
insert_ev6hwjhint(unsigned insn, int value, const char **errmsg)
{
if (errmsg != (const char **)NULL && (value & 3))
- *errmsg = _("jump hint unaligned");
+ *errmsg = "jump hint unaligned";
return insn | ((value / 4) & 0x1FFF);
}
diff --git a/disas/arm.c b/disas/arm.c
index 70da529..32f8ca9 100644
--- a/disas/arm.c
+++ b/disas/arm.c
@@ -1817,7 +1817,7 @@ print_insn_coprocessor (bfd_vma pc, struct disassemble_info *info, long given,
func (stream, "e");
break;
default:
- func (stream, _("<illegal precision>"));
+ func (stream, "<illegal precision>");
break;
}
break;
diff --git a/disas/i386.c b/disas/i386.c
index c0e717a..57145d0 100644
--- a/disas/i386.c
+++ b/disas/i386.c
@@ -3406,7 +3406,7 @@ static const struct dis386 three_byte_table[][256] = {
}
};
-#define INTERNAL_DISASSEMBLER_ERROR _("<internal disassembler error>")
+#define INTERNAL_DISASSEMBLER_ERROR "<internal disassembler error>"
static void
ckprefix (void)
diff --git a/disas/m68k.c b/disas/m68k.c
index 8f74ae1..8e7c3f7 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -1676,7 +1676,7 @@ print_insn_arg (const char *d,
(*info->fprintf_func) (info->stream, "%%sfc");
else
/* xgettext:c-format */
- (*info->fprintf_func) (info->stream, _("<function code %d>"), fc);
+ (*info->fprintf_func) (info->stream, "<function code %d>", fc);
}
break;
@@ -1827,7 +1827,7 @@ match_insn_m68k (bfd_vma memaddr,
{
info->fprintf_func (info->stream,
/* xgettext:c-format */
- _("<internal error in opcode table: %s %s>\n"),
+ "<internal error in opcode table: %s %s>\n",
best->name, best->args);
info->fprintf_func = save_printer;
info->print_address_func = save_print_address;
diff --git a/disas/mips.c b/disas/mips.c
index 249931b..97f661a 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -4257,7 +4257,7 @@ print_insn_args (const char *d,
case '\0':
/* xgettext:c-format */
(*info->fprintf_func) (info->stream,
- _("# internal error, incomplete extension sequence (+)"));
+ "# internal error, incomplete extension sequence (+)");
return;
case 'A':
@@ -4515,7 +4515,7 @@ print_insn_args (const char *d,
default:
/* xgettext:c-format */
(*info->fprintf_func) (info->stream,
- _("# internal error, undefined extension sequence (+%c)"),
+ "# internal error, undefined extension sequence (+%c)",
*d);
return;
}
@@ -4875,7 +4875,7 @@ print_insn_args (const char *d,
default:
/* xgettext:c-format */
(*info->fprintf_func) (info->stream,
- _("# internal error, undefined modifier(%c)"),
+ "# internal error, undefined modifier(%c)",
*d);
return;
}
@@ -5739,7 +5739,7 @@ print_mips16_insn_arg (char type,
/* xgettext:c-format */
(*info->fprintf_func)
(info->stream,
- _("# internal disassembler error, unrecognised modifier (%c)"),
+ "# internal disassembler error, unrecognised modifier (%c)",
type);
abort ();
}
@@ -5750,51 +5750,51 @@ print_mips_disassembler_options (FILE *stream)
{
unsigned int i;
- fprintf (stream, _("\n\
+ fprintf (stream, "\n\
The following MIPS specific disassembler options are supported for use\n\
-with the -M switch (multiple options should be separated by commas):\n"));
+with the -M switch (multiple options should be separated by commas):\n");
- fprintf (stream, _("\n\
+ fprintf (stream, "\n\
gpr-names=ABI Print GPR names according to specified ABI.\n\
- Default: based on binary being disassembled.\n"));
+ Default: based on binary being disassembled.\n");
- fprintf (stream, _("\n\
+ fprintf (stream, "\n\
fpr-names=ABI Print FPR names according to specified ABI.\n\
- Default: numeric.\n"));
+ Default: numeric.\n");
- fprintf (stream, _("\n\
+ fprintf (stream, "\n\
cp0-names=ARCH Print CP0 register names according to\n\
specified architecture.\n\
- Default: based on binary being disassembled.\n"));
+ Default: based on binary being disassembled.\n");
- fprintf (stream, _("\n\
+ fprintf (stream, "\n\
hwr-names=ARCH Print HWR names according to specified\n\
architecture.\n\
- Default: based on binary being disassembled.\n"));
+ Default: based on binary being disassembled.\n");
- fprintf (stream, _("\n\
+ fprintf (stream, "\n\
reg-names=ABI Print GPR and FPR names according to\n\
- specified ABI.\n"));
+ specified ABI.\n");
- fprintf (stream, _("\n\
+ fprintf (stream, "\n\
reg-names=ARCH Print CP0 register and HWR names according to\n\
- specified architecture.\n"));
+ specified architecture.\n");
- fprintf (stream, _("\n\
+ fprintf (stream, "\n\
For the options above, the following values are supported for \"ABI\":\n\
- "));
+ ");
for (i = 0; i < ARRAY_SIZE (mips_abi_choices); i++)
fprintf (stream, " %s", mips_abi_choices[i].name);
- fprintf (stream, _("\n"));
+ fprintf (stream, "\n");
- fprintf (stream, _("\n\
+ fprintf (stream, "\n\
For the options above, The following values are supported for \"ARCH\":\n\
- "));
+ ");
for (i = 0; i < ARRAY_SIZE (mips_arch_choices); i++)
if (*mips_arch_choices[i].name != '\0')
fprintf (stream, " %s", mips_arch_choices[i].name);
- fprintf (stream, _("\n"));
+ fprintf (stream, "\n");
- fprintf (stream, _("\n"));
+ fprintf (stream, "\n");
}
#endif
diff --git a/disas/ppc.c b/disas/ppc.c
index 478332b..052cebe 100644
--- a/disas/ppc.c
+++ b/disas/ppc.c
@@ -1120,7 +1120,7 @@ insert_bo (unsigned long insn,
const char **errmsg)
{
if (!valid_bo (value, dialect, 0))
- *errmsg = _("invalid conditional option");
+ *errmsg = "invalid conditional option";
return insn | ((value & 0x1f) << 21);
}
@@ -1148,9 +1148,9 @@ insert_boe (unsigned long insn,
const char **errmsg)
{
if (!valid_bo (value, dialect, 0))
- *errmsg = _("invalid conditional option");
+ *errmsg = "invalid conditional option";
else if ((value & 1) != 0)
- *errmsg = _("attempt to set y bit when using + or - modifier");
+ *errmsg = "attempt to set y bit when using + or - modifier";
return insn | ((value & 0x1f) << 21);
}
@@ -1182,7 +1182,7 @@ insert_fxm (unsigned long insn,
{
if (value == 0 || (value & -value) != value)
{
- *errmsg = _("invalid mask field");
+ *errmsg = "invalid mask field";
value = 0;
}
}
@@ -1208,7 +1208,7 @@ insert_fxm (unsigned long insn,
/* Any other value on mfcr is an error. */
else if ((insn & (0x3ff << 1)) == 19 << 1)
{
- *errmsg = _("ignoring invalid mfcr mask");
+ *errmsg = "ignoring invalid mfcr mask";
value = 0;
}
@@ -1258,7 +1258,7 @@ insert_mbe (unsigned long insn,
if (uval == 0)
{
- *errmsg = _("illegal bitmask");
+ *errmsg = "illegal bitmask";
return insn;
}
@@ -1293,7 +1293,7 @@ insert_mbe (unsigned long insn,
me = 32;
if (count != 2 && (count != 0 || ! last))
- *errmsg = _("illegal bitmask");
+ *errmsg = "illegal bitmask";
return insn | (mb << 6) | ((me - 1) << 1);
}
@@ -1413,7 +1413,7 @@ insert_ram (unsigned long insn,
const char **errmsg)
{
if ((unsigned long) value >= ((insn >> 21) & 0x1f))
- *errmsg = _("index register in load range");
+ *errmsg = "index register in load range";
return insn | ((value & 0x1f) << 16);
}
@@ -1429,7 +1429,7 @@ insert_raq (unsigned long insn,
long rtvalue = (insn & RT_MASK) >> 21;
if (value == rtvalue)
- *errmsg = _("source and target register operands must be different");
+ *errmsg = "source and target register operands must be different";
return insn | ((value & 0x1f) << 16);
}
@@ -1444,7 +1444,7 @@ insert_ras (unsigned long insn,
const char **errmsg)
{
if (value == 0)
- *errmsg = _("invalid register operand when updating");
+ *errmsg = "invalid register operand when updating";
return insn | ((value & 0x1f) << 16);
}
@@ -1526,7 +1526,7 @@ insert_sprg (unsigned long insn,
if (value > 7
|| (value > 3
&& (dialect & (PPC_OPCODE_BOOKE | PPC_OPCODE_403)) == 0))
- *errmsg = _("invalid sprg number");
+ *errmsg = "invalid sprg number";
/* If this is mfsprg4..7 then use spr 260..263 which can be read in
user mode. Anything else must use spr 272..279. */
diff --git a/disas/sparc.c b/disas/sparc.c
index 64bba8d..f120f4e 100644
--- a/disas/sparc.c
+++ b/disas/sparc.c
@@ -2494,7 +2494,7 @@ compare_opcodes (const void * a, const void * b)
fprintf
(stderr,
/* xgettext:c-format */
- _("Internal error: bad sparc-opcode.h: \"%s\", %#.8lx, %#.8lx\n"),
+ "Internal error: bad sparc-opcode.h: \"%s\", %#.8lx, %#.8lx\n",
op0->name, match0, lose0);
op0->lose &= ~op0->match;
lose0 = op0->lose;
@@ -2505,7 +2505,7 @@ compare_opcodes (const void * a, const void * b)
fprintf
(stderr,
/* xgettext:c-format */
- _("Internal error: bad sparc-opcode.h: \"%s\", %#.8lx, %#.8lx\n"),
+ "Internal error: bad sparc-opcode.h: \"%s\", %#.8lx, %#.8lx\n",
op1->name, match1, lose1);
op1->lose &= ~op1->match;
lose1 = op1->lose;
@@ -2555,7 +2555,7 @@ compare_opcodes (const void * a, const void * b)
else
fprintf (stderr,
/* xgettext:c-format */
- _("Internal error: bad sparc-opcode.h: \"%s\" == \"%s\"\n"),
+ "Internal error: bad sparc-opcode.h: \"%s\" == \"%s\"\n",
op0->name, op1->name);
}
diff --git a/include/disas/bfd.h b/include/disas/bfd.h
index a112e9c..a761d5b 100644
--- a/include/disas/bfd.h
+++ b/include/disas/bfd.h
@@ -477,7 +477,6 @@ int generic_symbol_at_address(bfd_vma, struct disassemble_info *);
(INFO).disassembler_options = NULL, \
(INFO).insn_info_valid = 0
-#define _(x) x
#define ATTRIBUTE_UNUSED __attribute__((unused))
/* from libbfd */
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v4 3/6] [trivial] trace: Cosmetic changes on fast-path tracing
2016-06-14 13:10 [Qemu-devel] [PATCH v4 0/6] trace: Per-vCPU tracing states Lluís Vilanova
2016-06-14 13:10 ` [Qemu-devel] [PATCH v4 1/6] trace: Identify events with the 'vcpu' property Lluís Vilanova
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 2/6] disas: Remove unused macro '_' Lluís Vilanova
@ 2016-06-14 13:11 ` Lluís Vilanova
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property Lluís Vilanova
` (2 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-14 13:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Eric Blake, Eduardo Habkost, Stefan Hajnoczi
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
trace/control-internal.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/trace/control-internal.h b/trace/control-internal.h
index 0ba10fe..3f368fd 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -62,14 +62,17 @@ static inline bool trace_event_get_state_static(TraceEvent *ev)
return ev->sstate;
}
-static inline bool trace_event_get_state_dynamic_by_id(int id)
+static inline bool trace_event_get_state_dynamic_by_id(TraceEventID id)
{
+ /* it's on fast path, avoid consistency checks (asserts) */
return unlikely(trace_events_enabled_count) && trace_events_dstate[id];
}
static inline bool trace_event_get_state_dynamic(TraceEvent *ev)
{
- int id = trace_event_get_id(ev);
+ TraceEventID id;
+ assert(trace_event_get_state_static(ev));
+ id = trace_event_get_id(ev);
return trace_event_get_state_dynamic_by_id(id);
}
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v4 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property
2016-06-14 13:10 [Qemu-devel] [PATCH v4 0/6] trace: Per-vCPU tracing states Lluís Vilanova
` (2 preceding siblings ...)
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 3/6] [trivial] trace: Cosmetic changes on fast-path tracing Lluís Vilanova
@ 2016-06-14 13:11 ` Lluís Vilanova
2016-06-17 15:24 ` Stefan Hajnoczi
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 5/6] trace: Conditionally trace events based on their per-vCPU state Lluís Vilanova
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state Lluís Vilanova
5 siblings, 1 reply; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-14 13:11 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Eric Blake, Eduardo Habkost, Stefan Hajnoczi,
Blue Swirl, Riku Voipio, Peter Crosthwaite, Richard Henderson
Each vCPU gets a 'trace_dstate' bitmap to control the per-vCPU dynamic
tracing state of events with the 'vcpu' property.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
Makefile.objs | 1 +
bsd-user/main.c | 2 ++
include/qom/cpu.h | 6 +++++
linux-user/main.c | 2 ++
qom/cpu.c | 1 +
trace/Makefile.objs | 26 ++++++++++++++++++++
trace/control-internal.h | 28 ++++++++++++++++-----
trace/control-stub.c | 28 +++++++++++++++++++++
trace/control-target.c | 53 ++++++++++++++++++++++++++++++++++++++++
trace/control.c | 29 ++++++++++++++++++++--
trace/control.h | 61 +++++++++++++++++++++++++++++++++++++++++++++-
translate-all.h | 3 ++
vl.c | 1 +
13 files changed, 231 insertions(+), 10 deletions(-)
create mode 100644 trace/control-stub.c
create mode 100644 trace/control-target.c
diff --git a/Makefile.objs b/Makefile.objs
index da49b71..30ae957 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -102,6 +102,7 @@ version-lobj-$(CONFIG_WIN32) += $(BUILD_DIR)/version.lo
# tracing
util-obj-y += trace/
target-obj-y += trace/
+stub-obj-y += trace/
######################################################################
# guest agent
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 9f592be..e43a583 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -27,6 +27,7 @@
#include "cpu.h"
#include "exec/exec-all.h"
#include "tcg.h"
+#include "trace/control.h"
#include "qemu/timer.h"
#include "qemu/envlist.h"
#include "exec/log.h"
@@ -1116,6 +1117,7 @@ int main(int argc, char **argv)
gdbserver_start (gdbstub_port);
gdb_handlesig(cpu, 0);
}
+ trace_init_vcpu_events();
cpu_loop(env);
/* never exits */
return 0;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 32f3af3..c27489f 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -24,8 +24,10 @@
#include "disas/bfd.h"
#include "exec/hwaddr.h"
#include "exec/memattrs.h"
+#include "qemu/bitmap.h"
#include "qemu/queue.h"
#include "qemu/thread.h"
+#include "trace/generated-events.h"
typedef int (*WriteCoreDumpFunction)(const void *buf, size_t size,
void *opaque);
@@ -273,6 +275,7 @@ struct qemu_work_item {
* @kvm_fd: vCPU file descriptor for KVM.
* @work_mutex: Lock to prevent multiple access to queued_work_*.
* @queued_work_first: First asynchronous work pending.
+ * @trace_dstate: Dynamic tracing state of events for this vCPU (bitmask).
*
* State of one CPU core or thread.
*/
@@ -340,6 +343,9 @@ struct CPUState {
struct KVMState *kvm_state;
struct kvm_run *kvm_run;
+ /* Used for events with 'vcpu' and *without* the 'disabled' properties */
+ DECLARE_BITMAP(trace_dstate, TRACE_VCPU_EVENT_COUNT);
+
/* TODO Move common fields from CPUArchState here. */
int cpu_index; /* used by alpha TCG */
uint32_t halted; /* used by alpha, cris, ppc TCG */
diff --git a/linux-user/main.c b/linux-user/main.c
index f8a8764..b49d764 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -29,6 +29,7 @@
#include "cpu.h"
#include "exec/exec-all.h"
#include "tcg.h"
+#include "trace/control.h"
#include "qemu/timer.h"
#include "qemu/envlist.h"
#include "elf.h"
@@ -4776,6 +4777,7 @@ int main(int argc, char **argv, char **envp)
}
gdb_handlesig(cpu, 0);
}
+ trace_init_vcpu_events();
cpu_loop(env);
/* never exits */
return 0;
diff --git a/qom/cpu.c b/qom/cpu.c
index 751e992..929ecbe 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -330,6 +330,7 @@ static void cpu_common_initfn(Object *obj)
qemu_mutex_init(&cpu->work_mutex);
QTAILQ_INIT(&cpu->breakpoints);
QTAILQ_INIT(&cpu->watchpoints);
+ bitmap_zero(cpu->trace_dstate, TRACE_VCPU_EVENT_COUNT);
}
static void cpu_common_finalize(Object *obj)
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 5145b34..902d47b 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -12,6 +12,8 @@ tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
# Auto-generated event descriptions for LTTng ust code
ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
+
+ifndef MAKEFILE_GUARD_TRACE
$(obj)/generated-ust-provider.h: $(obj)/generated-ust-provider.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
$(obj)/generated-ust-provider.h-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
@@ -30,11 +32,14 @@ $(obj)/generated-ust.c-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
$(obj)/generated-events.h: $(obj)/generated-ust-provider.h
$(obj)/generated-events.c: $(obj)/generated-ust.c
+endif # MAKEFILE_GUARD_TRACE
+
endif
######################################################################
# Auto-generated event descriptions
+ifndef MAKEFILE_GUARD_TRACE
$(obj)/generated-events.h: $(obj)/generated-events.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
$(obj)/generated-events.h-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
@@ -50,6 +55,7 @@ $(obj)/generated-events.c-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
--format=events-c \
--backends=$(TRACE_BACKENDS) \
< $< > $@," GEN $(patsubst %-timestamp,%,$@)")
+endif # MAKEFILE_GUARD_TRACE
util-obj-y += generated-events.o
@@ -60,6 +66,7 @@ util-obj-y += generated-events.o
##################################################
# Execution level
+ifndef MAKEFILE_GUARD_TRACE
$(obj)/generated-tracers.h: $(obj)/generated-tracers.h-timestamp
@cmp -s $< $@ || cp $< $@
$(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
@@ -67,10 +74,12 @@ $(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/conf
--format=h \
--backends=$(TRACE_BACKENDS) \
< $< > $@," GEN $(patsubst %-timestamp,%,$@)")
+endif # MAKEFILE_GUARD_TRACE
##############################
# non-DTrace
+ifndef MAKEFILE_GUARD_TRACE
$(obj)/generated-tracers.c: $(obj)/generated-tracers.c-timestamp
@cmp -s $< $@ || cp $< $@
$(obj)/generated-tracers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
@@ -80,6 +89,7 @@ $(obj)/generated-tracers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/conf
< $< > $@," GEN $(patsubst %-timestamp,%,$@)")
$(obj)/generated-tracers.o: $(obj)/generated-tracers.c $(obj)/generated-tracers.h
+endif # MAKEFILE_GUARD_TRACE
##############################
# DTrace
@@ -88,6 +98,8 @@ $(obj)/generated-tracers.o: $(obj)/generated-tracers.c $(obj)/generated-tracers.
# but that gets picked up by QEMU's Makefile as an external dependency
# rule file. So we use '.dtrace' instead
ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
+
+ifndef MAKEFILE_GUARD_TRACE
$(obj)/generated-tracers-dtrace.dtrace: $(obj)/generated-tracers-dtrace.dtrace-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
$(obj)/generated-tracers-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
@@ -100,6 +112,7 @@ $(obj)/generated-tracers-dtrace.h: $(obj)/generated-tracers-dtrace.dtrace
$(call quiet-command,dtrace -o $@ -h -s $<, " GEN $@")
$(obj)/generated-tracers-dtrace.o: $(obj)/generated-tracers-dtrace.dtrace
+endif # MAKEFILE_GUARD_TRACE
util-obj-y += generated-tracers-dtrace.o
endif
@@ -107,6 +120,7 @@ endif
##################################################
# Translation level
+ifndef MAKEFILE_GUARD_TRACE
$(obj)/generated-helpers-wrappers.h: $(obj)/generated-helpers-wrappers.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
$(obj)/generated-helpers-wrappers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
@@ -132,10 +146,12 @@ $(obj)/generated-helpers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/conf
< $< > $@," GEN $(patsubst %-timestamp,%,$@)")
$(obj)/generated-helpers.o: $(obj)/generated-helpers.c
+endif # MAKEFILE_GUARD_TRACE
target-obj-y += generated-helpers.o
+ifndef MAKEFILE_GUARD_TRACE
$(obj)/generated-tcg-tracers.h: $(obj)/generated-tcg-tracers.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
$(obj)/generated-tcg-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak $(tracetool-y)
@@ -143,6 +159,7 @@ $(obj)/generated-tcg-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/
--format=tcg-h \
--backend=$(TRACE_BACKENDS) \
< $< > $@," GEN $(patsubst %-timestamp,%,$@)")
+endif # MAKEFILE_GUARD_TRACE
######################################################################
@@ -152,4 +169,13 @@ util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o generated-tracers.o
util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o
util-obj-$(CONFIG_TRACE_UST) += generated-ust.o
util-obj-y += control.o
+target-obj-y += control-target.o
+stub-obj-y += control-stub.o
util-obj-y += qmp.o
+
+
+######################################################################
+# Avoid rule overrides when included from multiple top-level variables
+ifndef MAKEFILE_GUARD_TRACE
+MAKEFILE_GUARD_TRACE = 1
+endif
diff --git a/trace/control-internal.h b/trace/control-internal.h
index 3f368fd..fde1bba 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -10,10 +10,13 @@
#ifndef TRACE__CONTROL_INTERNAL_H
#define TRACE__CONTROL_INTERNAL_H
+#include <stddef.h> /* size_t */
+
+#include "qom/cpu.h"
extern TraceEvent trace_events[];
-extern bool trace_events_dstate[];
+extern uint16_t trace_events_dstate[];
extern int trace_events_enabled_count;
@@ -76,13 +79,24 @@ static inline bool trace_event_get_state_dynamic(TraceEvent *ev)
return trace_event_get_state_dynamic_by_id(id);
}
-static inline void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
+static inline bool trace_event_get_vcpu_state_dynamic_by_vcpu_id(CPUState *vcpu,
+ TraceEventVCPUID id)
{
- int id = trace_event_get_id(ev);
- assert(ev != NULL);
- assert(trace_event_get_state_static(ev));
- trace_events_enabled_count += state - trace_events_dstate[id];
- trace_events_dstate[id] = state;
+ /* it's on fast path, avoid consistency checks (asserts) */
+ if (unlikely(trace_events_enabled_count)) {
+ return test_bit(id, vcpu->trace_dstate);
+ } else {
+ return false;
+ }
+}
+
+static inline bool trace_event_get_vcpu_state_dynamic(CPUState *vcpu,
+ TraceEvent *ev)
+{
+ TraceEventVCPUID id;
+ assert(trace_event_is_vcpu(ev));
+ id = trace_event_get_vcpu_id(ev);
+ return trace_event_get_vcpu_state_dynamic_by_vcpu_id(vcpu, id);
}
#endif /* TRACE__CONTROL_INTERNAL_H */
diff --git a/trace/control-stub.c b/trace/control-stub.c
new file mode 100644
index 0000000..fe59836
--- /dev/null
+++ b/trace/control-stub.c
@@ -0,0 +1,28 @@
+/*
+ * Interface for configuring and controlling the state of tracing events.
+ *
+ * Copyright (C) 2014-2016 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "trace/control.h"
+
+
+void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
+{
+ TraceEventID id;
+ assert(trace_event_get_state_static(ev));
+ id = trace_event_get_id(ev);
+ trace_events_enabled_count += state - trace_events_dstate[id];
+ trace_events_dstate[id] = state;
+}
+
+void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
+ TraceEvent *ev, bool state)
+{
+ /* should never be called on non-target binaries */
+ abort();
+}
diff --git a/trace/control-target.c b/trace/control-target.c
new file mode 100644
index 0000000..74c029a
--- /dev/null
+++ b/trace/control-target.c
@@ -0,0 +1,53 @@
+/*
+ * Interface for configuring and controlling the state of tracing events.
+ *
+ * Copyright (C) 2014-2016 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "trace/control.h"
+#include "translate-all.h"
+
+
+void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
+{
+ CPUState *vcpu;
+ assert(trace_event_get_state_static(ev));
+ if (trace_event_is_vcpu(ev)) {
+ CPU_FOREACH(vcpu) {
+ trace_event_set_vcpu_state_dynamic(vcpu, ev, state);
+ }
+ } else {
+ TraceEventID id = trace_event_get_id(ev);
+ trace_events_enabled_count += state - trace_events_dstate[id];
+ trace_events_dstate[id] = state;
+ }
+}
+
+void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
+ TraceEvent *ev, bool state)
+{
+ TraceEventID id;
+ TraceEventVCPUID vcpu_id;
+ bool state_pre;
+ assert(trace_event_get_state_static(ev));
+ assert(trace_event_is_vcpu(ev));
+ id = trace_event_get_id(ev);
+ vcpu_id = trace_event_get_vcpu_id(ev);
+ state_pre = test_bit(vcpu_id, vcpu->trace_dstate);
+ if (state_pre != state) {
+ if (state) {
+ trace_events_enabled_count++;
+ set_bit(vcpu_id, vcpu->trace_dstate);
+ trace_events_dstate[id]++;
+ } else {
+ trace_events_enabled_count--;
+ clear_bit(vcpu_id, vcpu->trace_dstate);
+ trace_events_dstate[id]--;
+ }
+ }
+}
diff --git a/trace/control.c b/trace/control.c
index d099f73..b0a5dcf 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -1,7 +1,7 @@
/*
* Interface for configuring and controlling the state of tracing events.
*
- * Copyright (C) 2011-2014 Lluís Vilanova <vilanova@ac.upc.edu>
+ * Copyright (C) 2011-2016 Lluís Vilanova <vilanova@ac.upc.edu>
*
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
@@ -23,7 +23,14 @@
#include "monitor/monitor.h"
int trace_events_enabled_count;
-bool trace_events_dstate[TRACE_EVENT_COUNT];
+/*
+ * Interpretation depends on wether the event has the 'vcpu' property:
+ * - false: Boolean value indicating whether the event is active.
+ * - true : Integral counting the number of vCPUs that have this event enabled.
+ */
+uint16_t trace_events_dstate[TRACE_EVENT_COUNT];
+/* Marks events for late vCPU state init */
+static bool trace_events_dstate_init[TRACE_EVENT_COUNT];
TraceEvent *trace_event_name(const char *name)
{
@@ -112,7 +119,10 @@ static void do_trace_enable_events(const char *line_buf)
TraceEvent *ev = NULL;
while ((ev = trace_event_pattern(line_ptr, ev)) != NULL) {
if (trace_event_get_state_static(ev)) {
+ /* start tracing */
trace_event_set_state_dynamic(ev, enable);
+ /* mark for late vCPU init */
+ trace_events_dstate_init[ev->id] = true;
}
}
} else {
@@ -124,7 +134,10 @@ static void do_trace_enable_events(const char *line_buf)
error_report("WARNING: trace event '%s' is not traceable",
line_ptr);
} else {
+ /* start tracing */
trace_event_set_state_dynamic(ev, enable);
+ /* mark for late vCPU init */
+ trace_events_dstate_init[ev->id] = true;
}
}
}
@@ -216,3 +229,15 @@ bool trace_init_backends(void)
return true;
}
+
+void trace_init_vcpu_events(void)
+{
+ TraceEvent *ev = NULL;
+ while ((ev = trace_event_pattern("*", ev)) != NULL) {
+ if (trace_event_is_vcpu(ev) &&
+ trace_event_get_state_static(ev) &&
+ trace_events_dstate_init[ev->id]) {
+ trace_event_set_state_dynamic(ev, true);
+ }
+ }
+}
diff --git a/trace/control.h b/trace/control.h
index d37bce7..5234a8e 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -124,6 +124,23 @@ static const char * trace_event_get_name(TraceEvent *ev);
((id ##_ENABLED) && trace_event_get_state_dynamic_by_id(id))
/**
+ * trace_event_get_vcpu_state:
+ * @vcpu: Target vCPU.
+ * @id: Event identifier (TraceEventID).
+ * @vcpu_id: Per-vCPU event identifier (TraceEventVCPUID).
+ *
+ * Get the tracing state of an event (both static and dynamic) for the given
+ * vCPU.
+ *
+ * If the event has the disabled property, the check will have no performance
+ * impact.
+ *
+ * As a down side, you must always use an immediate #TraceEventID value.
+ */
+#define trace_event_get_vcpu_state(vcpu, id, vcpu_id) \
+ ((id ##_ENABLED) && trace_event_get_vcpu_state_dynamic_by_vcpu_id(vcpu, vcpu_id))
+
+/**
* trace_event_get_state_static:
* @id: Event identifier.
*
@@ -138,10 +155,19 @@ static bool trace_event_get_state_static(TraceEvent *ev);
* trace_event_get_state_dynamic:
*
* Get the dynamic tracing state of an event.
+ *
+ * If the event has the 'vcpu' property, gets the OR'ed state of all vCPUs.
*/
static bool trace_event_get_state_dynamic(TraceEvent *ev);
/**
+ * trace_event_get_vcpu_state_dynamic:
+ *
+ * Get the dynamic tracing state of an event for the given vCPU.
+ */
+static bool trace_event_get_vcpu_state_dynamic(CPUState *vcpu, TraceEvent *ev);
+
+/**
* trace_event_set_state:
*
* Set the tracing state of an event (only if possible).
@@ -155,13 +181,38 @@ static bool trace_event_get_state_dynamic(TraceEvent *ev);
} while (0)
/**
+ * trace_event_set_vcpu_state:
+ *
+ * Set the tracing state of an event for the given vCPU (only if not disabled).
+ */
+#define trace_event_set_vcpu_state(vcpu, id, state) \
+ do { \
+ if ((id ##_ENABLED)) { \
+ TraceEvent *_e = trace_event_id(id); \
+ trace_event_set_vcpu_state_dynamic(vcpu, _e, state); \
+ } \
+ } while (0)
+
+/**
* trace_event_set_state_dynamic:
*
* Set the dynamic tracing state of an event.
*
+ * If the event has the 'vcpu' property, sets the state on all vCPUs.
+ *
* Pre-condition: trace_event_get_state_static(ev) == true
*/
-static void trace_event_set_state_dynamic(TraceEvent *ev, bool state);
+void trace_event_set_state_dynamic(TraceEvent *ev, bool state);
+
+/**
+ * trace_event_set_vcpu_state_dynamic:
+ *
+ * Set the dynamic tracing state of an event for the given vCPU.
+ *
+ * Pre-condition: trace_event_get_vcpu_state_static(ev) == true
+ */
+void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
+ TraceEvent *ev, bool state);
@@ -214,6 +265,14 @@ void trace_list_events(void);
*/
void trace_enable_events(const char *line_buf);
+/**
+ * trace_init_vcpu_events:
+ *
+ * Re-synchronize initial event state with vCPUs (which can be created after
+ * trace_init_events()).
+ */
+void trace_init_vcpu_events(void);
+
#include "trace/control-internal.h"
diff --git a/translate-all.h b/translate-all.h
index 0384640..bb8f542 100644
--- a/translate-all.h
+++ b/translate-all.h
@@ -19,6 +19,9 @@
#ifndef TRANSLATE_ALL_H
#define TRANSLATE_ALL_H
+#include "exec/exec-all.h"
+
+
/* translate-all.c */
void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len);
void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
diff --git a/vl.c b/vl.c
index b0bcc25..e3d8f65 100644
--- a/vl.c
+++ b/vl.c
@@ -4624,6 +4624,7 @@ int main(int argc, char **argv, char **envp)
os_setup_post();
+ trace_init_vcpu_events();
main_loop();
replay_disable_events();
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v4 5/6] trace: Conditionally trace events based on their per-vCPU state
2016-06-14 13:10 [Qemu-devel] [PATCH v4 0/6] trace: Per-vCPU tracing states Lluís Vilanova
` (3 preceding siblings ...)
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property Lluís Vilanova
@ 2016-06-14 13:11 ` Lluís Vilanova
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state Lluís Vilanova
5 siblings, 0 replies; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-14 13:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Eric Blake, Eduardo Habkost, Stefan Hajnoczi
Events with the 'vcpu' property are conditionally emitted according to
their per-vCPU state. Other events are emitted normally based on their
global tracing state.
Note that the per-vCPU condition check applies to all tracing backends.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
scripts/tracetool/backend/dtrace.py | 4 ++--
scripts/tracetool/backend/ftrace.py | 20 ++++++++++----------
scripts/tracetool/backend/log.py | 26 ++++++++++++++++----------
scripts/tracetool/backend/simple.py | 13 ++++++++++---
scripts/tracetool/backend/ust.py | 4 ++--
scripts/tracetool/format/h.py | 18 ++++++++++++++++--
6 files changed, 56 insertions(+), 29 deletions(-)
diff --git a/scripts/tracetool/backend/dtrace.py b/scripts/tracetool/backend/dtrace.py
index fabfe99..ab9ecfa 100644
--- a/scripts/tracetool/backend/dtrace.py
+++ b/scripts/tracetool/backend/dtrace.py
@@ -6,7 +6,7 @@ DTrace/SystemTAP backend.
"""
__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__ = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2016, Lluís Vilanova <vilanova@ac.upc.edu>"
__license__ = "GPL version 2 or (at your option) any later version"
__maintainer__ = "Stefan Hajnoczi"
@@ -41,6 +41,6 @@ def generate_h_begin(events):
def generate_h(event):
- out(' QEMU_%(uppername)s(%(argnames)s);',
+ out(' QEMU_%(uppername)s(%(argnames)s);',
uppername=event.name.upper(),
argnames=", ".join(event.args.names()))
diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
index d798c71..80dcf30 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -30,17 +30,17 @@ def generate_h(event):
if len(event.args) > 0:
argnames = ", " + argnames
- out(' {',
- ' char ftrace_buf[MAX_TRACE_STRLEN];',
- ' int unused __attribute__ ((unused));',
- ' int trlen;',
- ' if (trace_event_get_state(%(event_id)s)) {',
- ' 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);',
+ out(' {',
+ ' char ftrace_buf[MAX_TRACE_STRLEN];',
+ ' int unused __attribute__ ((unused));',
+ ' int trlen;',
+ ' if (trace_event_get_state(%(event_id)s)) {',
+ ' 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=event.name,
args=event.args,
event_id="TRACE_" + event.name.upper(),
diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
index e409b73..b3ff064 100644
--- a/scripts/tracetool/backend/log.py
+++ b/scripts/tracetool/backend/log.py
@@ -6,7 +6,7 @@ Stderr built-in backend.
"""
__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__ = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2016, Lluís Vilanova <vilanova@ac.upc.edu>"
__license__ = "GPL version 2 or (at your option) any later version"
__maintainer__ = "Stefan Hajnoczi"
@@ -30,15 +30,21 @@ def generate_h(event):
if len(event.args) > 0:
argnames = ", " + argnames
- out(' if (trace_event_get_state(%(event_id)s)) {',
- ' struct timeval _now;',
- ' gettimeofday(&_now, NULL);',
- ' qemu_log_mask(LOG_TRACE, "%%d@%%zd.%%06zd:%(name)s " %(fmt)s "\\n",',
- ' getpid(),',
- ' (size_t)_now.tv_sec, (size_t)_now.tv_usec',
- ' %(argnames)s);',
- ' }',
- event_id="TRACE_" + event.name.upper(),
+ if "vcpu" in event.properties:
+ # already checked on the generic format code
+ cond = "true"
+ else:
+ cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
+
+ out(' if (%(cond)s) {',
+ ' struct timeval _now;',
+ ' gettimeofday(&_now, NULL);',
+ ' qemu_log_mask(LOG_TRACE, "%%d@%%zd.%%06zd:%(name)s " %(fmt)s "\\n",',
+ ' getpid(),',
+ ' (size_t)_now.tv_sec, (size_t)_now.tv_usec',
+ ' %(argnames)s);',
+ ' }',
+ cond=cond,
name=event.name,
fmt=event.fmt.rstrip("\n"),
argnames=argnames)
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 3246c20..1bccada 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -36,7 +36,7 @@ def generate_h_begin(events):
def generate_h(event):
- out(' _simple_%(api)s(%(args)s);',
+ out(' _simple_%(api)s(%(args)s);',
api=event.api(),
args=", ".join(event.args.names()))
@@ -68,16 +68,23 @@ def generate_c(event):
if len(event.args) == 0:
sizestr = '0'
+ event_id = 'TRACE_' + event.name.upper()
+ if "vcpu" in event.properties:
+ # already checked on the generic format code
+ cond = "true"
+ else:
+ cond = "trace_event_get_state(%s)" % event_id
out('',
- ' if (!trace_event_get_state(%(event_id)s)) {',
+ ' if (!%(cond)s) {',
' return;',
' }',
'',
' if (trace_record_start(&rec, %(event_id)s, %(size_str)s)) {',
' return; /* Trace Buffer Full, Event Dropped ! */',
' }',
- event_id='TRACE_' + event.name.upper(),
+ cond=cond,
+ event_id=event_id,
size_str=sizestr)
if len(event.args) > 0:
diff --git a/scripts/tracetool/backend/ust.py b/scripts/tracetool/backend/ust.py
index 2f8f44a..ed4c227 100644
--- a/scripts/tracetool/backend/ust.py
+++ b/scripts/tracetool/backend/ust.py
@@ -6,7 +6,7 @@ LTTng User Space Tracing backend.
"""
__author__ = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__ = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__ = "Copyright 2012-2016, Lluís Vilanova <vilanova@ac.upc.edu>"
__license__ = "GPL version 2 or (at your option) any later version"
__maintainer__ = "Stefan Hajnoczi"
@@ -30,6 +30,6 @@ def generate_h(event):
if len(event.args) > 0:
argnames = ", " + argnames
- out(' tracepoint(qemu, %(name)s%(tp_args)s);',
+ out(' tracepoint(qemu, %(name)s%(tp_args)s);',
name=event.name,
tp_args=argnames)
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index 0835406..f4311fd 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -28,16 +28,30 @@ def generate(events, backend):
backend.generate_begin(events)
for e in events:
+ if "vcpu" in e.properties:
+ trace_cpu = next(iter(e.args))[1]
+ cond = "trace_event_get_vcpu_state(%(cpu)s,"\
+ " TRACE_%(id)s,"\
+ " TRACE_VCPU_%(id)s)"\
+ % dict(
+ cpu=trace_cpu,
+ id=e.name.upper())
+ else:
+ cond = "true"
+
out('',
'static inline void %(api)s(%(args)s)',
'{',
+ ' if (%(cond)s) {',
api=e.api(),
- args=e.args)
+ args=e.args,
+ cond=cond)
if "disable" not in e.properties:
backend.generate(e)
- out('}')
+ out(' }',
+ '}')
backend.generate_end(events)
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state
2016-06-14 13:10 [Qemu-devel] [PATCH v4 0/6] trace: Per-vCPU tracing states Lluís Vilanova
` (4 preceding siblings ...)
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 5/6] trace: Conditionally trace events based on their per-vCPU state Lluís Vilanova
@ 2016-06-14 13:11 ` Lluís Vilanova
2016-06-20 7:23 ` Markus Armbruster
5 siblings, 1 reply; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-14 13:11 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Eric Blake, Eduardo Habkost, Stefan Hajnoczi,
Luiz Capitulino, Markus Armbruster
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
monitor.c | 4 +-
qapi/trace.json | 20 ++++++--
qmp-commands.hx | 17 ++++++-
trace/qmp.c | 143 ++++++++++++++++++++++++++++++++++++++++++++-----------
4 files changed, 147 insertions(+), 37 deletions(-)
diff --git a/monitor.c b/monitor.c
index a27e115..bb89877 100644
--- a/monitor.c
+++ b/monitor.c
@@ -910,7 +910,7 @@ static void hmp_trace_event(Monitor *mon, const QDict *qdict)
bool new_state = qdict_get_bool(qdict, "option");
Error *local_err = NULL;
- qmp_trace_event_set_state(tp_name, new_state, true, true, &local_err);
+ qmp_trace_event_set_state(tp_name, new_state, true, true, false, 0, &local_err);
if (local_err) {
error_report_err(local_err);
}
@@ -1069,7 +1069,7 @@ static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
{
- TraceEventInfoList *events = qmp_trace_event_get_state("*", NULL);
+ TraceEventInfoList *events = qmp_trace_event_get_state("*", false, 0, NULL);
TraceEventInfoList *elem;
for (elem = events; elem != NULL; elem = elem->next) {
diff --git a/qapi/trace.json b/qapi/trace.json
index 01b0a52..25d8095 100644
--- a/qapi/trace.json
+++ b/qapi/trace.json
@@ -1,6 +1,6 @@
# -*- mode: python -*-
#
-# Copyright (C) 2011-2014 Lluís Vilanova <vilanova@ac.upc.edu>
+# Copyright (C) 2011-2016 Lluís Vilanova <vilanova@ac.upc.edu>
#
# This work is licensed under the terms of the GNU GPL, version 2 or later.
# See the COPYING file in the top-level directory.
@@ -29,11 +29,12 @@
#
# @name: Event name.
# @state: Tracing state.
+# @vcpu: Whether this is a per-vCPU event (since 2.7).
#
# Since 2.2
##
{ 'struct': 'TraceEventInfo',
- 'data': {'name': 'str', 'state': 'TraceEventState'} }
+ 'data': {'name': 'str', 'state': 'TraceEventState', 'vcpu': 'bool'} }
##
# @trace-event-get-state:
@@ -41,13 +42,18 @@
# Query the state of events.
#
# @name: Event name pattern (case-sensitive glob).
+# @vcpu: #optional The vCPU to check (any by default; since 2.7).
#
# Returns: a list of @TraceEventInfo for the matching events
#
+# For any event without the "vcpu" property:
+# - If @name is a pattern and @vcpu is set, events are ignored.
+# - If @name is not a pattern and @vcpu is set, an error is raised.
+#
# Since 2.2
##
{ 'command': 'trace-event-get-state',
- 'data': {'name': 'str'},
+ 'data': {'name': 'str', '*vcpu': 'int'},
'returns': ['TraceEventInfo'] }
##
@@ -58,8 +64,14 @@
# @name: Event name pattern (case-sensitive glob).
# @enable: Whether to enable tracing.
# @ignore-unavailable: #optional Do not match unavailable events with @name.
+# @vcpu: #optional The vCPU to act upon (all by default; since 2.7).
+#
+# For any event without the "vcpu" property:
+# - If @name is a pattern and @vcpu is set, events are ignored.
+# - If @name is not a pattern and @vcpu is set, an error is raised.
#
# Since 2.2
##
{ 'command': 'trace-event-set-state',
- 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool'} }
+ 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool',
+ '*vcpu': 'int'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 28801a2..c9eb25c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4676,7 +4676,7 @@ EQMP
{
.name = "trace-event-get-state",
- .args_type = "name:s",
+ .args_type = "name:s,vcpu:i?",
.mhandler.cmd_new = qmp_marshal_trace_event_get_state,
},
@@ -4686,6 +4686,11 @@ trace-event-get-state
Query the state of events.
+Arguments:
+
+- "name": Event name pattern (json-string).
+- "vcpu": Specific vCPU to query, any vCPU by default (json-int, optional).
+
Example:
-> { "execute": "trace-event-get-state", "arguments": { "name": "qemu_memalign" } }
@@ -4694,7 +4699,7 @@ EQMP
{
.name = "trace-event-set-state",
- .args_type = "name:s,enable:b,ignore-unavailable:b?",
+ .args_type = "name:s,enable:b,ignore-unavailable:b?,vcpu:i?",
.mhandler.cmd_new = qmp_marshal_trace_event_set_state,
},
@@ -4704,6 +4709,14 @@ trace-event-set-state
Set the state of events.
+Arguments:
+
+- "name": Event name pattern (json-string).
+- "enable": Whether to enable or disable the event (json-bool).
+- "ignore-unavailable": Whether to ignore errors for events that cannot be
+ changed (json-bool, optional).
+- "vcpu": Specific vCPU to set, all vCPUs by default (json-int, optional).
+
Example:
-> { "execute": "trace-event-set-state", "arguments": { "name": "qemu_memalign", "enable": "true" } }
diff --git a/trace/qmp.c b/trace/qmp.c
index 8aa2660..c18e750 100644
--- a/trace/qmp.c
+++ b/trace/qmp.c
@@ -1,7 +1,7 @@
/*
* QMP commands for tracing events.
*
- * Copyright (C) 2014 Lluís Vilanova <vilanova@ac.upc.edu>
+ * Copyright (C) 2014-2016 Lluís Vilanova <vilanova@ac.upc.edu>
*
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
@@ -12,63 +12,148 @@
#include "trace/control.h"
-TraceEventInfoList *qmp_trace_event_get_state(const char *name, Error **errp)
+static bool get_cpu(bool has_vcpu, int vcpu, CPUState **cpu, Error **errp)
+{
+ if (has_vcpu) {
+ *cpu = qemu_get_cpu(vcpu);
+ if (*cpu == NULL) {
+ error_setg(errp, "invalid vCPU index %u", vcpu);
+ return false;
+ }
+ } else {
+ *cpu = NULL;
+ }
+ return true;
+}
+
+static bool check_events(bool has_vcpu, bool ignore_unavailable, bool is_pattern,
+ const char *name, Error **errp)
+{
+ if (!is_pattern) {
+ TraceEvent *ev = trace_event_name(name);
+
+ /* error for non-existing event */
+ if (ev == NULL) {
+ error_setg(errp, "unknown event \"%s\"", name);
+ return false;
+ }
+
+ /* error for non-vcpu event */
+ if (has_vcpu && trace_event_is_vcpu(ev)) {
+ error_setg(errp, "event \"%s\" is not vCPU-specific", name);
+ return false;
+ }
+
+ /* error for unavailable event */
+ if (!ignore_unavailable && !trace_event_get_state_static(ev)) {
+ error_setg(errp, "event \"%s\" is disabled", name);
+ return false;
+ }
+
+ return true;
+ } else {
+ /* error for unavailable events */
+ TraceEvent *ev = NULL;
+ while ((ev = trace_event_pattern(name, ev)) != NULL) {
+ if (!ignore_unavailable && !trace_event_get_state_static(ev)) {
+ error_setg(errp, "event \"%s\" is disabled", trace_event_get_name(ev));
+ return false;
+ }
+ }
+ return true;
+ }
+}
+
+TraceEventInfoList *qmp_trace_event_get_state(const char *name,
+ bool has_vcpu, int64_t vcpu,
+ Error **errp)
{
TraceEventInfoList *events = NULL;
- bool found = false;
TraceEvent *ev;
+ bool is_pattern = trace_event_is_pattern(name);
+ CPUState *cpu;
+ /* Check provided vcpu */
+ if (!get_cpu(has_vcpu, vcpu, &cpu, errp)) {
+ return NULL;
+ }
+
+ /* Check events */
+ if (!check_events(has_vcpu, true, is_pattern, name, errp)) {
+ return NULL;
+ }
+
+ /* Get states (all errors checked above) */
ev = NULL;
while ((ev = trace_event_pattern(name, ev)) != NULL) {
- TraceEventInfoList *elem = g_new(TraceEventInfoList, 1);
+ TraceEventInfoList *elem;
+ bool is_vcpu = trace_event_is_vcpu(ev);
+ if (has_vcpu && !is_vcpu) {
+ continue;
+ }
+
+ elem = g_new(TraceEventInfoList, 1);
elem->value = g_new(TraceEventInfo, 1);
+ elem->value->vcpu = is_vcpu;
elem->value->name = g_strdup(trace_event_get_name(ev));
+
if (!trace_event_get_state_static(ev)) {
elem->value->state = TRACE_EVENT_STATE_UNAVAILABLE;
- } else if (!trace_event_get_state_dynamic(ev)) {
- elem->value->state = TRACE_EVENT_STATE_DISABLED;
} else {
- elem->value->state = TRACE_EVENT_STATE_ENABLED;
+ if (has_vcpu) {
+ if (is_vcpu) {
+ if (trace_event_get_vcpu_state_dynamic(cpu, ev)) {
+ elem->value->state = TRACE_EVENT_STATE_ENABLED;
+ } else {
+ elem->value->state = TRACE_EVENT_STATE_DISABLED;
+ }
+ }
+ /* else: already skipped above */
+ } else {
+ if (trace_event_get_state_dynamic(ev)) {
+ elem->value->state = TRACE_EVENT_STATE_ENABLED;
+ } else {
+ elem->value->state = TRACE_EVENT_STATE_DISABLED;
+ }
+ }
}
elem->next = events;
events = elem;
- found = true;
- }
-
- if (!found && !trace_event_is_pattern(name)) {
- error_setg(errp, "unknown event \"%s\"", name);
}
return events;
}
void qmp_trace_event_set_state(const char *name, bool enable,
- bool has_ignore_unavailable,
- bool ignore_unavailable, Error **errp)
+ bool has_ignore_unavailable, bool ignore_unavailable,
+ bool has_vcpu, int64_t vcpu,
+ Error **errp)
{
- bool found = false;
TraceEvent *ev;
+ bool is_pattern = trace_event_is_pattern(name);
+ CPUState *cpu;
- /* Check all selected events are dynamic */
- ev = NULL;
- while ((ev = trace_event_pattern(name, ev)) != NULL) {
- found = true;
- if (!(has_ignore_unavailable && ignore_unavailable) &&
- !trace_event_get_state_static(ev)) {
- error_setg(errp, "cannot set dynamic tracing state for \"%s\"",
- trace_event_get_name(ev));
- return;
- }
+ /* Check provided vcpu */
+ if (!get_cpu(has_vcpu, vcpu, &cpu, errp)) {
+ return;
}
- if (!found && !trace_event_is_pattern(name)) {
- error_setg(errp, "unknown event \"%s\"", name);
+
+ /* Check events */
+ if (!check_events(has_vcpu, has_ignore_unavailable && ignore_unavailable,
+ is_pattern, name, errp)) {
return;
}
- /* Apply changes */
+ /* Apply changes (all errors checked above) */
ev = NULL;
while ((ev = trace_event_pattern(name, ev)) != NULL) {
- if (trace_event_get_state_static(ev)) {
+ if (!trace_event_get_state_static(ev) ||
+ (has_vcpu && trace_event_is_vcpu(ev))) {
+ continue;
+ }
+ if (has_vcpu) {
+ trace_event_set_vcpu_state_dynamic(cpu, ev, enable);
+ } else {
trace_event_set_state_dynamic(ev, enable);
}
}
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property Lluís Vilanova
@ 2016-06-17 15:24 ` Stefan Hajnoczi
2016-06-17 20:16 ` Lluís Vilanova
0 siblings, 1 reply; 20+ messages in thread
From: Stefan Hajnoczi @ 2016-06-17 15:24 UTC (permalink / raw)
To: Lluís Vilanova
Cc: qemu-devel, Paolo Bonzini, Eric Blake, Eduardo Habkost,
Blue Swirl, Riku Voipio, Peter Crosthwaite, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 740 bytes --]
On Tue, Jun 14, 2016 at 03:11:12PM +0200, Lluís Vilanova wrote:
> @@ -1116,6 +1117,7 @@ int main(int argc, char **argv)
> gdbserver_start (gdbstub_port);
> gdb_handlesig(cpu, 0);
> }
> + trace_init_vcpu_events();
Do vcpu events make sense in *-user builds? I thought this feature is
only available in *-softmmu.
> +######################################################################
> +# Avoid rule overrides when included from multiple top-level variables
> +ifndef MAKEFILE_GUARD_TRACE
> +MAKEFILE_GUARD_TRACE = 1
> +endif
Are you including this makefile multiple times? We need to find a
cleaner solution. If this is related to "stubs", then could
control-stub.o could go in stubs/?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/6] disas: Remove unused macro '_'
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 2/6] disas: Remove unused macro '_' Lluís Vilanova
@ 2016-06-17 15:25 ` Stefan Hajnoczi
0 siblings, 0 replies; 20+ messages in thread
From: Stefan Hajnoczi @ 2016-06-17 15:25 UTC (permalink / raw)
To: Lluís Vilanova
Cc: qemu-devel, Paolo Bonzini, Eric Blake, Eduardo Habkost,
Richard Henderson, Andrzej Zaborowski, Peter Maydell,
Aurelien Jarno, Leon Alrae, David Gibson, Alexander Graf,
Vassili Karpov (malc), Blue Swirl, Mark Cave-Ayland,
open list:ARM target, open list:PowerPC
[-- Attachment #1: Type: text/plain, Size: 760 bytes --]
On Tue, Jun 14, 2016 at 03:11:01PM +0200, Lluís Vilanova wrote:
> Eliminates a future compilation error when UI code includes the tracing
> headers (indirectly pulling "disas/bfd.h" through "qom/cpu.h") and
> GLib's i18n '_' macro.
>
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
> disas/alpha.c | 6 +++---
> disas/arm.c | 2 +-
> disas/i386.c | 2 +-
> disas/m68k.c | 4 ++--
> disas/mips.c | 50 +++++++++++++++++++++++++-------------------------
> disas/ppc.c | 22 +++++++++++-----------
> disas/sparc.c | 6 +++---
> include/disas/bfd.h | 1 -
> 8 files changed, 46 insertions(+), 47 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property
2016-06-17 15:24 ` Stefan Hajnoczi
@ 2016-06-17 20:16 ` Lluís Vilanova
2016-06-20 12:05 ` Stefan Hajnoczi
0 siblings, 1 reply; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-17 20:16 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Eduardo Habkost, Peter Crosthwaite, Riku Voipio, qemu-devel,
Blue Swirl, Paolo Bonzini, Richard Henderson
Stefan Hajnoczi writes:
> On Tue, Jun 14, 2016 at 03:11:12PM +0200, Lluís Vilanova wrote:
>> @@ -1116,6 +1117,7 @@ int main(int argc, char **argv)
>> gdbserver_start (gdbstub_port);
>> gdb_handlesig(cpu, 0);
>> }
>> + trace_init_vcpu_events();
> Do vcpu events make sense in *-user builds? I thought this feature is
> only available in *-softmmu.
It's here for completeness, although it's currently useless. AFAIK, QEMU has
multiple vCPUs when the user application is multi-threaded, but user QEMU has no
interface to interactively control event states.
I can remove it if you want.
>> +######################################################################
>> +# Avoid rule overrides when included from multiple top-level variables
>> +ifndef MAKEFILE_GUARD_TRACE
>> +MAKEFILE_GUARD_TRACE = 1
>> +endif
> Are you including this makefile multiple times? We need to find a
> cleaner solution. If this is related to "stubs", then could
> control-stub.o could go in stubs/?
The stubs are the reason, yes. I'll refactor these into the stubs directory.
Thanks,
Lluis
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state Lluís Vilanova
@ 2016-06-20 7:23 ` Markus Armbruster
2016-06-20 10:48 ` Lluís Vilanova
2016-06-20 10:53 ` Lluís Vilanova
0 siblings, 2 replies; 20+ messages in thread
From: Markus Armbruster @ 2016-06-20 7:23 UTC (permalink / raw)
To: Lluís Vilanova
Cc: qemu-devel, Eduardo Habkost, Luiz Capitulino, Stefan Hajnoczi,
Paolo Bonzini
Lluís Vilanova <vilanova@ac.upc.edu> writes:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> monitor.c | 4 +-
> qapi/trace.json | 20 ++++++--
> qmp-commands.hx | 17 ++++++-
> trace/qmp.c | 143 ++++++++++++++++++++++++++++++++++++++++++++-----------
> 4 files changed, 147 insertions(+), 37 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index a27e115..bb89877 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -910,7 +910,7 @@ static void hmp_trace_event(Monitor *mon, const QDict *qdict)
> bool new_state = qdict_get_bool(qdict, "option");
> Error *local_err = NULL;
>
> - qmp_trace_event_set_state(tp_name, new_state, true, true, &local_err);
> + qmp_trace_event_set_state(tp_name, new_state, true, true, false, 0, &local_err);
> if (local_err) {
> error_report_err(local_err);
> }
> @@ -1069,7 +1069,7 @@ static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
>
> static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
> {
> - TraceEventInfoList *events = qmp_trace_event_get_state("*", NULL);
> + TraceEventInfoList *events = qmp_trace_event_get_state("*", false, 0, NULL);
> TraceEventInfoList *elem;
>
> for (elem = events; elem != NULL; elem = elem->next) {
The new feature remains inaccessible in HMP. Any plans to extend HMP?
Any reasons not to?
> diff --git a/qapi/trace.json b/qapi/trace.json
> index 01b0a52..25d8095 100644
> --- a/qapi/trace.json
> +++ b/qapi/trace.json
> @@ -1,6 +1,6 @@
> # -*- mode: python -*-
> #
> -# Copyright (C) 2011-2014 Lluís Vilanova <vilanova@ac.upc.edu>
> +# Copyright (C) 2011-2016 Lluís Vilanova <vilanova@ac.upc.edu>
> #
> # This work is licensed under the terms of the GNU GPL, version 2 or later.
> # See the COPYING file in the top-level directory.
> @@ -29,11 +29,12 @@
> #
> # @name: Event name.
> # @state: Tracing state.
> +# @vcpu: Whether this is a per-vCPU event (since 2.7).
> #
> # Since 2.2
> ##
> { 'struct': 'TraceEventInfo',
> - 'data': {'name': 'str', 'state': 'TraceEventState'} }
> + 'data': {'name': 'str', 'state': 'TraceEventState', 'vcpu': 'bool'} }
>
> ##
> # @trace-event-get-state:
> @@ -41,13 +42,18 @@
> # Query the state of events.
> #
> # @name: Event name pattern (case-sensitive glob).
> +# @vcpu: #optional The vCPU to check (any by default; since 2.7).
> #
> # Returns: a list of @TraceEventInfo for the matching events
> #
> +# For any event without the "vcpu" property:
All events have the vcpu property, but for some of them the property's
value is false.
> +# - If @name is a pattern and @vcpu is set, events are ignored.
I figure "ignored" means they're not included in the return value.
Correct?
> +# - If @name is not a pattern and @vcpu is set, an error is raised.
Perhaps we could clarify as follows:
# Returns: a list of @TraceEventInfo for the matching events
#
# An event matches if
# - its name matches the @name pattern, and
# - if @vcpu is given, its vCPU equals @vcpu.
# It follows that with @vcpu given, the query can match only per-vCPU
# events. Special case: if the name is an exact match, you get an error
# instead of an empty list.
> +#
> # Since 2.2
> ##
> { 'command': 'trace-event-get-state',
> - 'data': {'name': 'str'},
> + 'data': {'name': 'str', '*vcpu': 'int'},
> 'returns': ['TraceEventInfo'] }
>
> ##
> @@ -58,8 +64,14 @@
> # @name: Event name pattern (case-sensitive glob).
> # @enable: Whether to enable tracing.
> # @ignore-unavailable: #optional Do not match unavailable events with @name.
> +# @vcpu: #optional The vCPU to act upon (all by default; since 2.7).
> +#
> +# For any event without the "vcpu" property:
> +# - If @name is a pattern and @vcpu is set, events are ignored.
> +# - If @name is not a pattern and @vcpu is set, an error is raised.
Likewise.
> #
> # Since 2.2
> ##
> { 'command': 'trace-event-set-state',
> - 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool'} }
> + 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool',
> + '*vcpu': 'int'} }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 28801a2..c9eb25c 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4676,7 +4676,7 @@ EQMP
>
> {
> .name = "trace-event-get-state",
> - .args_type = "name:s",
> + .args_type = "name:s,vcpu:i?",
> .mhandler.cmd_new = qmp_marshal_trace_event_get_state,
> },
>
> @@ -4686,6 +4686,11 @@ trace-event-get-state
>
> Query the state of events.
>
> +Arguments:
> +
> +- "name": Event name pattern (json-string).
> +- "vcpu": Specific vCPU to query, any vCPU by default (json-int, optional).
> +
Less information than you give in the schema. Easy enough to fix.
> Example:
>
> -> { "execute": "trace-event-get-state", "arguments": { "name": "qemu_memalign" } }
> @@ -4694,7 +4699,7 @@ EQMP
>
> {
> .name = "trace-event-set-state",
> - .args_type = "name:s,enable:b,ignore-unavailable:b?",
> + .args_type = "name:s,enable:b,ignore-unavailable:b?,vcpu:i?",
> .mhandler.cmd_new = qmp_marshal_trace_event_set_state,
> },
>
> @@ -4704,6 +4709,14 @@ trace-event-set-state
>
> Set the state of events.
>
> +Arguments:
> +
> +- "name": Event name pattern (json-string).
> +- "enable": Whether to enable or disable the event (json-bool).
> +- "ignore-unavailable": Whether to ignore errors for events that cannot be
> + changed (json-bool, optional).
> +- "vcpu": Specific vCPU to set, all vCPUs by default (json-int, optional).
> +
Likewise.
> Example:
>
> -> { "execute": "trace-event-set-state", "arguments": { "name": "qemu_memalign", "enable": "true" } }
> diff --git a/trace/qmp.c b/trace/qmp.c
> index 8aa2660..c18e750 100644
> --- a/trace/qmp.c
> +++ b/trace/qmp.c
> @@ -1,7 +1,7 @@
> /*
> * QMP commands for tracing events.
> *
> - * Copyright (C) 2014 Lluís Vilanova <vilanova@ac.upc.edu>
> + * Copyright (C) 2014-2016 Lluís Vilanova <vilanova@ac.upc.edu>
> *
> * This work is licensed under the terms of the GNU GPL, version 2 or later.
> * See the COPYING file in the top-level directory.
> @@ -12,63 +12,148 @@
> #include "trace/control.h"
>
>
> -TraceEventInfoList *qmp_trace_event_get_state(const char *name, Error **errp)
> +static bool get_cpu(bool has_vcpu, int vcpu, CPUState **cpu, Error **errp)
> +{
> + if (has_vcpu) {
> + *cpu = qemu_get_cpu(vcpu);
> + if (*cpu == NULL) {
> + error_setg(errp, "invalid vCPU index %u", vcpu);
> + return false;
> + }
> + } else {
> + *cpu = NULL;
> + }
> + return true;
> +}
This returns three things: a bool (via return value), a CPUState *
(via @cpu) and an Error (via customary @errp). Possible combinations:
* Success with has_vcpu: true, non-null CPU *, no error
* Failure with has_vcpu: false, null CPU *, error set
* Success without has_vcpu: true, null CPU *, no error
Usage:
if (!get_cpu(has_vcpu, vcpu, &cpu, errp)) {
error out...
}
If you dispense with the bool and return the pointer instead, you'd get:
Error *err = NULL;
cpu = get_cpu(has_vcpu, vcpu, &err);
if (err) {
error_propagate(errp, err);
error out...
}
This is more in line with how we use Error elsewhere. Needs more code,
though. Since it's just a local helper function, the choice is yours.
> +
> +static bool check_events(bool has_vcpu, bool ignore_unavailable, bool is_pattern,
> + const char *name, Error **errp)
> +{
> + if (!is_pattern) {
> + TraceEvent *ev = trace_event_name(name);
> +
> + /* error for non-existing event */
> + if (ev == NULL) {
> + error_setg(errp, "unknown event \"%s\"", name);
> + return false;
> + }
> +
> + /* error for non-vcpu event */
> + if (has_vcpu && trace_event_is_vcpu(ev)) {
> + error_setg(errp, "event \"%s\" is not vCPU-specific", name);
> + return false;
> + }
> +
> + /* error for unavailable event */
> + if (!ignore_unavailable && !trace_event_get_state_static(ev)) {
> + error_setg(errp, "event \"%s\" is disabled", name);
> + return false;
> + }
> +
> + return true;
> + } else {
> + /* error for unavailable events */
> + TraceEvent *ev = NULL;
> + while ((ev = trace_event_pattern(name, ev)) != NULL) {
> + if (!ignore_unavailable && !trace_event_get_state_static(ev)) {
> + error_setg(errp, "event \"%s\" is disabled", trace_event_get_name(ev));
> + return false;
> + }
> + }
> + return true;
> + }
> +}
> +
> +TraceEventInfoList *qmp_trace_event_get_state(const char *name,
> + bool has_vcpu, int64_t vcpu,
> + Error **errp)
> {
> TraceEventInfoList *events = NULL;
> - bool found = false;
> TraceEvent *ev;
> + bool is_pattern = trace_event_is_pattern(name);
> + CPUState *cpu;
>
> + /* Check provided vcpu */
> + if (!get_cpu(has_vcpu, vcpu, &cpu, errp)) {
> + return NULL;
> + }
> +
> + /* Check events */
> + if (!check_events(has_vcpu, true, is_pattern, name, errp)) {
> + return NULL;
> + }
> +
> + /* Get states (all errors checked above) */
> ev = NULL;
> while ((ev = trace_event_pattern(name, ev)) != NULL) {
> - TraceEventInfoList *elem = g_new(TraceEventInfoList, 1);
> + TraceEventInfoList *elem;
> + bool is_vcpu = trace_event_is_vcpu(ev);
> + if (has_vcpu && !is_vcpu) {
> + continue;
> + }
> +
> + elem = g_new(TraceEventInfoList, 1);
> elem->value = g_new(TraceEventInfo, 1);
> + elem->value->vcpu = is_vcpu;
> elem->value->name = g_strdup(trace_event_get_name(ev));
> +
> if (!trace_event_get_state_static(ev)) {
> elem->value->state = TRACE_EVENT_STATE_UNAVAILABLE;
> - } else if (!trace_event_get_state_dynamic(ev)) {
> - elem->value->state = TRACE_EVENT_STATE_DISABLED;
> } else {
> - elem->value->state = TRACE_EVENT_STATE_ENABLED;
> + if (has_vcpu) {
> + if (is_vcpu) {
> + if (trace_event_get_vcpu_state_dynamic(cpu, ev)) {
> + elem->value->state = TRACE_EVENT_STATE_ENABLED;
> + } else {
> + elem->value->state = TRACE_EVENT_STATE_DISABLED;
> + }
> + }
> + /* else: already skipped above */
> + } else {
> + if (trace_event_get_state_dynamic(ev)) {
> + elem->value->state = TRACE_EVENT_STATE_ENABLED;
> + } else {
> + elem->value->state = TRACE_EVENT_STATE_DISABLED;
> + }
> + }
> }
> elem->next = events;
> events = elem;
> - found = true;
> - }
> -
> - if (!found && !trace_event_is_pattern(name)) {
> - error_setg(errp, "unknown event \"%s\"", name);
> }
>
> return events;
> }
>
> void qmp_trace_event_set_state(const char *name, bool enable,
> - bool has_ignore_unavailable,
> - bool ignore_unavailable, Error **errp)
> + bool has_ignore_unavailable, bool ignore_unavailable,
> + bool has_vcpu, int64_t vcpu,
> + Error **errp)
> {
> - bool found = false;
> TraceEvent *ev;
> + bool is_pattern = trace_event_is_pattern(name);
> + CPUState *cpu;
>
> - /* Check all selected events are dynamic */
> - ev = NULL;
> - while ((ev = trace_event_pattern(name, ev)) != NULL) {
> - found = true;
> - if (!(has_ignore_unavailable && ignore_unavailable) &&
> - !trace_event_get_state_static(ev)) {
> - error_setg(errp, "cannot set dynamic tracing state for \"%s\"",
> - trace_event_get_name(ev));
> - return;
> - }
> + /* Check provided vcpu */
> + if (!get_cpu(has_vcpu, vcpu, &cpu, errp)) {
> + return;
> }
> - if (!found && !trace_event_is_pattern(name)) {
> - error_setg(errp, "unknown event \"%s\"", name);
> +
> + /* Check events */
> + if (!check_events(has_vcpu, has_ignore_unavailable && ignore_unavailable,
> + is_pattern, name, errp)) {
> return;
> }
>
> - /* Apply changes */
> + /* Apply changes (all errors checked above) */
> ev = NULL;
> while ((ev = trace_event_pattern(name, ev)) != NULL) {
> - if (trace_event_get_state_static(ev)) {
> + if (!trace_event_get_state_static(ev) ||
> + (has_vcpu && trace_event_is_vcpu(ev))) {
> + continue;
> + }
> + if (has_vcpu) {
> + trace_event_set_vcpu_state_dynamic(cpu, ev, enable);
> + } else {
> trace_event_set_state_dynamic(ev, enable);
> }
> }
Looks okay.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state
2016-06-20 7:23 ` Markus Armbruster
@ 2016-06-20 10:48 ` Lluís Vilanova
2016-06-20 12:13 ` Markus Armbruster
2016-06-20 10:53 ` Lluís Vilanova
1 sibling, 1 reply; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-20 10:48 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, Luiz Capitulino, qemu-devel, Stefan Hajnoczi,
Eduardo Habkost
Markus Armbruster writes:
> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>> monitor.c | 4 +-
>> qapi/trace.json | 20 ++++++--
>> qmp-commands.hx | 17 ++++++-
>> trace/qmp.c | 143 ++++++++++++++++++++++++++++++++++++++++++++-----------
>> 4 files changed, 147 insertions(+), 37 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index a27e115..bb89877 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -910,7 +910,7 @@ static void hmp_trace_event(Monitor *mon, const QDict *qdict)
>> bool new_state = qdict_get_bool(qdict, "option");
>> Error *local_err = NULL;
>>
>> - qmp_trace_event_set_state(tp_name, new_state, true, true, &local_err);
>> + qmp_trace_event_set_state(tp_name, new_state, true, true, false, 0, &local_err);
>> if (local_err) {
>> error_report_err(local_err);
>> }
>> @@ -1069,7 +1069,7 @@ static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
>>
>> static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
>> {
>> - TraceEventInfoList *events = qmp_trace_event_get_state("*", NULL);
>> + TraceEventInfoList *events = qmp_trace_event_get_state("*", false, 0, NULL);
>> TraceEventInfoList *elem;
>>
>> for (elem = events; elem != NULL; elem = elem->next) {
> The new feature remains inaccessible in HMP. Any plans to extend HMP?
> Any reasons not to?
My bad, I forgot about it. I'll see to adding it.
>> diff --git a/qapi/trace.json b/qapi/trace.json
>> index 01b0a52..25d8095 100644
>> --- a/qapi/trace.json
>> +++ b/qapi/trace.json
>> @@ -1,6 +1,6 @@
>> # -*- mode: python -*-
>> #
>> -# Copyright (C) 2011-2014 Lluís Vilanova <vilanova@ac.upc.edu>
>> +# Copyright (C) 2011-2016 Lluís Vilanova <vilanova@ac.upc.edu>
>> #
>> # This work is licensed under the terms of the GNU GPL, version 2 or later.
>> # See the COPYING file in the top-level directory.
>> @@ -29,11 +29,12 @@
>> #
>> # @name: Event name.
>> # @state: Tracing state.
>> +# @vcpu: Whether this is a per-vCPU event (since 2.7).
>> #
>> # Since 2.2
>> ##
>> { 'struct': 'TraceEventInfo',
>> - 'data': {'name': 'str', 'state': 'TraceEventState'} }
>> + 'data': {'name': 'str', 'state': 'TraceEventState', 'vcpu': 'bool'} }
>>
>> ##
>> # @trace-event-get-state:
>> @@ -41,13 +42,18 @@
>> # Query the state of events.
>> #
>> # @name: Event name pattern (case-sensitive glob).
>> +# @vcpu: #optional The vCPU to check (any by default; since 2.7).
>> #
>> # Returns: a list of @TraceEventInfo for the matching events
>> #
>> +# For any event without the "vcpu" property:
> All events have the vcpu property, but for some of them the property's
> value is false.
Hmmm, do you mean in the generated event array? Here, I was referring to wether
an event has the "vcpu" property in "trace-events". I can clarify that.
>> +# - If @name is a pattern and @vcpu is set, events are ignored.
> I figure "ignored" means they're not included in the return value.
> Correct?
Exactly.
>> +# - If @name is not a pattern and @vcpu is set, an error is raised.
> Perhaps we could clarify as follows:
> # Returns: a list of @TraceEventInfo for the matching events
> #
> # An event matches if
> # - its name matches the @name pattern, and
> # - if @vcpu is given, its vCPU equals @vcpu.
> # It follows that with @vcpu given, the query can match only per-vCPU
> # events. Special case: if the name is an exact match, you get an error
> # instead of an empty list.
Doesn't sound entirely right to me:
# Returns: a list of @TraceEventInfo for the matching events
#
# An event matches if
# - its name matches the @name pattern, and
# - if @vcpu is given, the event is a per-vCPU one (has the "vcpu" property in
# "trace-events").
#
# Therefore, if @vcpu is given, the query will only match per-vCPU events.
# Special case: if @name is an exact match and @vcpu is given, you will get an
# error if the event does not have the "vcpu" property.
>> +#
>> # Since 2.2
>> ##
>> { 'command': 'trace-event-get-state',
>> - 'data': {'name': 'str'},
>> + 'data': {'name': 'str', '*vcpu': 'int'},
>> 'returns': ['TraceEventInfo'] }
>>
>> ##
>> @@ -58,8 +64,14 @@
>> # @name: Event name pattern (case-sensitive glob).
>> # @enable: Whether to enable tracing.
>> # @ignore-unavailable: #optional Do not match unavailable events with @name.
>> +# @vcpu: #optional The vCPU to act upon (all by default; since 2.7).
>> +#
>> +# For any event without the "vcpu" property:
>> +# - If @name is a pattern and @vcpu is set, events are ignored.
>> +# - If @name is not a pattern and @vcpu is set, an error is raised.
> Likewise.
Ok.
>> #
>> # Since 2.2
>> ##
>> { 'command': 'trace-event-set-state',
>> - 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool'} }
>> + 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool',
>> + '*vcpu': 'int'} }
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 28801a2..c9eb25c 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -4676,7 +4676,7 @@ EQMP
>>
>> {
>> .name = "trace-event-get-state",
>> - .args_type = "name:s",
>> + .args_type = "name:s,vcpu:i?",
>> .mhandler.cmd_new = qmp_marshal_trace_event_get_state,
>> },
>>
>> @@ -4686,6 +4686,11 @@ trace-event-get-state
>>
>> Query the state of events.
>>
>> +Arguments:
>> +
>> +- "name": Event name pattern (json-string).
>> +- "vcpu": Specific vCPU to query, any vCPU by default (json-int, optional).
>> +
> Less information than you give in the schema. Easy enough to fix.
I guess you mean extending the docs here to cover the same info given in the
JSON file.
>> Example:
>>
-> { "execute": "trace-event-get-state", "arguments": { "name": "qemu_memalign" } }
>> @@ -4694,7 +4699,7 @@ EQMP
>>
>> {
>> .name = "trace-event-set-state",
>> - .args_type = "name:s,enable:b,ignore-unavailable:b?",
>> + .args_type = "name:s,enable:b,ignore-unavailable:b?,vcpu:i?",
>> .mhandler.cmd_new = qmp_marshal_trace_event_set_state,
>> },
>>
>> @@ -4704,6 +4709,14 @@ trace-event-set-state
>>
>> Set the state of events.
>>
>> +Arguments:
>> +
>> +- "name": Event name pattern (json-string).
>> +- "enable": Whether to enable or disable the event (json-bool).
>> +- "ignore-unavailable": Whether to ignore errors for events that cannot be
>> + changed (json-bool, optional).
>> +- "vcpu": Specific vCPU to set, all vCPUs by default (json-int, optional).
>> +
> Likewise.
Ok.
>> Example:
>>
-> { "execute": "trace-event-set-state", "arguments": { "name": "qemu_memalign", "enable": "true" } }
>> diff --git a/trace/qmp.c b/trace/qmp.c
>> index 8aa2660..c18e750 100644
>> --- a/trace/qmp.c
>> +++ b/trace/qmp.c
>> @@ -1,7 +1,7 @@
>> /*
>> * QMP commands for tracing events.
>> *
>> - * Copyright (C) 2014 Lluís Vilanova <vilanova@ac.upc.edu>
>> + * Copyright (C) 2014-2016 Lluís Vilanova <vilanova@ac.upc.edu>
>> *
>> * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> * See the COPYING file in the top-level directory.
>> @@ -12,63 +12,148 @@
>> #include "trace/control.h"
>>
>>
>> -TraceEventInfoList *qmp_trace_event_get_state(const char *name, Error **errp)
>> +static bool get_cpu(bool has_vcpu, int vcpu, CPUState **cpu, Error **errp)
>> +{
>> + if (has_vcpu) {
>> + *cpu = qemu_get_cpu(vcpu);
>> + if (*cpu == NULL) {
>> + error_setg(errp, "invalid vCPU index %u", vcpu);
>> + return false;
>> + }
>> + } else {
>> + *cpu = NULL;
>> + }
>> + return true;
>> +}
> This returns three things: a bool (via return value), a CPUState *
> (via @cpu) and an Error (via customary @errp). Possible combinations:
> * Success with has_vcpu: true, non-null CPU *, no error
> * Failure with has_vcpu: false, null CPU *, error set
> * Success without has_vcpu: true, null CPU *, no error
> Usage:
> if (!get_cpu(has_vcpu, vcpu, &cpu, errp)) {
> error out...
> }
> If you dispense with the bool and return the pointer instead, you'd get:
> Error *err = NULL;
> cpu = get_cpu(has_vcpu, vcpu, &err);
> if (err) {
> error_propagate(errp, err);
> error out...
> }
> This is more in line with how we use Error elsewhere. Needs more code,
> though. Since it's just a local helper function, the choice is yours.
Oh, I didn't think of using err to check errors. That seems more readable.
Thanks,
Lluis
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state
2016-06-20 7:23 ` Markus Armbruster
2016-06-20 10:48 ` Lluís Vilanova
@ 2016-06-20 10:53 ` Lluís Vilanova
2016-06-20 12:17 ` Markus Armbruster
1 sibling, 1 reply; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-20 10:53 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, Luiz Capitulino, qemu-devel, Stefan Hajnoczi,
Eduardo Habkost
Markus Armbruster writes:
> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>> monitor.c | 4 +-
>> qapi/trace.json | 20 ++++++--
>> qmp-commands.hx | 17 ++++++-
>> trace/qmp.c | 143 ++++++++++++++++++++++++++++++++++++++++++++-----------
>> 4 files changed, 147 insertions(+), 37 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index a27e115..bb89877 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -910,7 +910,7 @@ static void hmp_trace_event(Monitor *mon, const QDict *qdict)
>> bool new_state = qdict_get_bool(qdict, "option");
>> Error *local_err = NULL;
>>
>> - qmp_trace_event_set_state(tp_name, new_state, true, true, &local_err);
>> + qmp_trace_event_set_state(tp_name, new_state, true, true, false, 0, &local_err);
>> if (local_err) {
>> error_report_err(local_err);
>> }
>> @@ -1069,7 +1069,7 @@ static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
>>
>> static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
>> {
>> - TraceEventInfoList *events = qmp_trace_event_get_state("*", NULL);
>> + TraceEventInfoList *events = qmp_trace_event_get_state("*", false, 0, NULL);
>> TraceEventInfoList *elem;
>>
>> for (elem = events; elem != NULL; elem = elem->next) {
> The new feature remains inaccessible in HMP. Any plans to extend HMP?
> Any reasons not to?
BTW, I was just looking at "info trace-events" and it only shows the state for
all events (there's no name pattern argument). Is it worth updating HMP knowing
it should be replaced in favour of the newer QAPI interface?
Thanks,
Lluis
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property
2016-06-17 20:16 ` Lluís Vilanova
@ 2016-06-20 12:05 ` Stefan Hajnoczi
2016-06-20 12:40 ` Lluís Vilanova
0 siblings, 1 reply; 20+ messages in thread
From: Stefan Hajnoczi @ 2016-06-20 12:05 UTC (permalink / raw)
To: Stefan Hajnoczi, Eduardo Habkost, Peter Crosthwaite, Riku Voipio,
qemu-devel, Blue Swirl, Paolo Bonzini, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 819 bytes --]
On Fri, Jun 17, 2016 at 10:16:25PM +0200, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
>
> > On Tue, Jun 14, 2016 at 03:11:12PM +0200, Lluís Vilanova wrote:
> >> @@ -1116,6 +1117,7 @@ int main(int argc, char **argv)
> >> gdbserver_start (gdbstub_port);
> >> gdb_handlesig(cpu, 0);
> >> }
> >> + trace_init_vcpu_events();
>
> > Do vcpu events make sense in *-user builds? I thought this feature is
> > only available in *-softmmu.
>
> It's here for completeness, although it's currently useless. AFAIK, QEMU has
> multiple vCPUs when the user application is multi-threaded, but user QEMU has no
> interface to interactively control event states.
Yes, please drop the *-user changes. I see no trace_init_*() calls in
*-user so it won't be useful to call trace_init_vcpu_events().
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state
2016-06-20 10:48 ` Lluís Vilanova
@ 2016-06-20 12:13 ` Markus Armbruster
2016-06-20 12:44 ` Lluís Vilanova
0 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2016-06-20 12:13 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Luiz Capitulino, qemu-devel, Stefan Hajnoczi, Eduardo Habkost
Lluís Vilanova <vilanova@ac.upc.edu> writes:
> Markus Armbruster writes:
>
>> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>> monitor.c | 4 +-
>>> qapi/trace.json | 20 ++++++--
>>> qmp-commands.hx | 17 ++++++-
>>> trace/qmp.c | 143 ++++++++++++++++++++++++++++++++++++++++++++-----------
>>> 4 files changed, 147 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/monitor.c b/monitor.c
>>> index a27e115..bb89877 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -910,7 +910,7 @@ static void hmp_trace_event(Monitor *mon, const QDict *qdict)
>>> bool new_state = qdict_get_bool(qdict, "option");
>>> Error *local_err = NULL;
>>>
>>> - qmp_trace_event_set_state(tp_name, new_state, true, true, &local_err);
>>> + qmp_trace_event_set_state(tp_name, new_state, true, true, false, 0, &local_err);
>>> if (local_err) {
>>> error_report_err(local_err);
>>> }
>>> @@ -1069,7 +1069,7 @@ static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
>>>
>>> static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
>>> {
>>> - TraceEventInfoList *events = qmp_trace_event_get_state("*", NULL);
>>> + TraceEventInfoList *events = qmp_trace_event_get_state("*", false, 0, NULL);
>>> TraceEventInfoList *elem;
>>>
>>> for (elem = events; elem != NULL; elem = elem->next) {
>
>> The new feature remains inaccessible in HMP. Any plans to extend HMP?
>> Any reasons not to?
>
> My bad, I forgot about it. I'll see to adding it.
>
>
>>> diff --git a/qapi/trace.json b/qapi/trace.json
>>> index 01b0a52..25d8095 100644
>>> --- a/qapi/trace.json
>>> +++ b/qapi/trace.json
>>> @@ -1,6 +1,6 @@
>>> # -*- mode: python -*-
>>> #
>>> -# Copyright (C) 2011-2014 Lluís Vilanova <vilanova@ac.upc.edu>
>>> +# Copyright (C) 2011-2016 Lluís Vilanova <vilanova@ac.upc.edu>
>>> #
>>> # This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> # See the COPYING file in the top-level directory.
>>> @@ -29,11 +29,12 @@
>>> #
>>> # @name: Event name.
>>> # @state: Tracing state.
>>> +# @vcpu: Whether this is a per-vCPU event (since 2.7).
>>> #
>>> # Since 2.2
>>> ##
>>> { 'struct': 'TraceEventInfo',
>>> - 'data': {'name': 'str', 'state': 'TraceEventState'} }
>>> + 'data': {'name': 'str', 'state': 'TraceEventState', 'vcpu': 'bool'} }
>>>
>>> ##
>>> # @trace-event-get-state:
>>> @@ -41,13 +42,18 @@
>>> # Query the state of events.
>>> #
>>> # @name: Event name pattern (case-sensitive glob).
>>> +# @vcpu: #optional The vCPU to check (any by default; since 2.7).
>>> #
>>> # Returns: a list of @TraceEventInfo for the matching events
>>> #
>>> +# For any event without the "vcpu" property:
>
>> All events have the vcpu property, but for some of them the property's
>> value is false.
>
> Hmmm, do you mean in the generated event array? Here, I was referring to wether
> an event has the "vcpu" property in "trace-events". I can clarify that.
What is "trace-events"? The file in the source tree?
>>> +# - If @name is a pattern and @vcpu is set, events are ignored.
>
>> I figure "ignored" means they're not included in the return value.
>> Correct?
>
> Exactly.
>
>
>>> +# - If @name is not a pattern and @vcpu is set, an error is raised.
>
>> Perhaps we could clarify as follows:
>
>> # Returns: a list of @TraceEventInfo for the matching events
>> #
>> # An event matches if
>> # - its name matches the @name pattern, and
>> # - if @vcpu is given, its vCPU equals @vcpu.
>> # It follows that with @vcpu given, the query can match only per-vCPU
>> # events. Special case: if the name is an exact match, you get an error
>> # instead of an empty list.
>
> Doesn't sound entirely right to me:
>
> # Returns: a list of @TraceEventInfo for the matching events
> #
> # An event matches if
> # - its name matches the @name pattern, and
> # - if @vcpu is given, the event is a per-vCPU one (has the "vcpu" property in
> # "trace-events").
Fails to mention that @vcpu selects the TraceEventInfo for that
particular VCPU. That's what I tried to express by "its vCPU equals
@vcpu".
> #
> # Therefore, if @vcpu is given, the query will only match per-vCPU events.
> # Special case: if @name is an exact match and @vcpu is given, you will get an
> # error if the event does not have the "vcpu" property.
>
>
>>> +#
>>> # Since 2.2
>>> ##
>>> { 'command': 'trace-event-get-state',
>>> - 'data': {'name': 'str'},
>>> + 'data': {'name': 'str', '*vcpu': 'int'},
>>> 'returns': ['TraceEventInfo'] }
>>>
>>> ##
>>> @@ -58,8 +64,14 @@
>>> # @name: Event name pattern (case-sensitive glob).
>>> # @enable: Whether to enable tracing.
>>> # @ignore-unavailable: #optional Do not match unavailable events with @name.
>>> +# @vcpu: #optional The vCPU to act upon (all by default; since 2.7).
>>> +#
>>> +# For any event without the "vcpu" property:
>>> +# - If @name is a pattern and @vcpu is set, events are ignored.
>>> +# - If @name is not a pattern and @vcpu is set, an error is raised.
>
>> Likewise.
>
> Ok.
>
>
>>> #
>>> # Since 2.2
>>> ##
>>> { 'command': 'trace-event-set-state',
>>> - 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool'} }
>>> + 'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool',
>>> + '*vcpu': 'int'} }
>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>>> index 28801a2..c9eb25c 100644
>>> --- a/qmp-commands.hx
>>> +++ b/qmp-commands.hx
>>> @@ -4676,7 +4676,7 @@ EQMP
>>>
>>> {
>>> .name = "trace-event-get-state",
>>> - .args_type = "name:s",
>>> + .args_type = "name:s,vcpu:i?",
>>> .mhandler.cmd_new = qmp_marshal_trace_event_get_state,
>>> },
>>>
>>> @@ -4686,6 +4686,11 @@ trace-event-get-state
>>>
>>> Query the state of events.
>>>
>>> +Arguments:
>>> +
>>> +- "name": Event name pattern (json-string).
>>> +- "vcpu": Specific vCPU to query, any vCPU by default (json-int, optional).
>>> +
>
>> Less information than you give in the schema. Easy enough to fix.
>
> I guess you mean extending the docs here to cover the same info given in the
> JSON file.
Yes.
I hope we can get rid of the duplication some day. Marc-André has
patches.
[...]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state
2016-06-20 10:53 ` Lluís Vilanova
@ 2016-06-20 12:17 ` Markus Armbruster
2016-06-20 12:37 ` Lluís Vilanova
0 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2016-06-20 12:17 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Luiz Capitulino, qemu-devel, Stefan Hajnoczi, Eduardo Habkost
Lluís Vilanova <vilanova@ac.upc.edu> writes:
> Markus Armbruster writes:
>
>> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>> monitor.c | 4 +-
>>> qapi/trace.json | 20 ++++++--
>>> qmp-commands.hx | 17 ++++++-
>>> trace/qmp.c | 143 ++++++++++++++++++++++++++++++++++++++++++++-----------
>>> 4 files changed, 147 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/monitor.c b/monitor.c
>>> index a27e115..bb89877 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -910,7 +910,7 @@ static void hmp_trace_event(Monitor *mon, const QDict *qdict)
>>> bool new_state = qdict_get_bool(qdict, "option");
>>> Error *local_err = NULL;
>>>
>>> - qmp_trace_event_set_state(tp_name, new_state, true, true, &local_err);
>>> + qmp_trace_event_set_state(tp_name, new_state, true, true, false, 0, &local_err);
>>> if (local_err) {
>>> error_report_err(local_err);
>>> }
>>> @@ -1069,7 +1069,7 @@ static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
>>>
>>> static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
>>> {
>>> - TraceEventInfoList *events = qmp_trace_event_get_state("*", NULL);
>>> + TraceEventInfoList *events = qmp_trace_event_get_state("*", false, 0, NULL);
>>> TraceEventInfoList *elem;
>>>
>>> for (elem = events; elem != NULL; elem = elem->next) {
>
>> The new feature remains inaccessible in HMP. Any plans to extend HMP?
>> Any reasons not to?
>
> BTW, I was just looking at "info trace-events" and it only shows the state for
> all events (there's no name pattern argument). Is it worth updating HMP knowing
> it should be replaced in favour of the newer QAPI interface?
QMP is not meant as replacement for HMP. QMP is a stable interface for
machines, HMP is a convenient interface for humans. New functionality
is usually added to both. HMP sometimes gets additional convenience
features. In rare cases, a feature is deemed of no interest to humans,
and left out of HMP. Is filtering by vCPU such a case?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state
2016-06-20 12:17 ` Markus Armbruster
@ 2016-06-20 12:37 ` Lluís Vilanova
0 siblings, 0 replies; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-20 12:37 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, Eduardo Habkost, qemu-devel, Stefan Hajnoczi,
Luiz Capitulino
Markus Armbruster writes:
> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>> Markus Armbruster writes:
>>
>>> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>> ---
>>>> monitor.c | 4 +-
>>>> qapi/trace.json | 20 ++++++--
>>>> qmp-commands.hx | 17 ++++++-
>>>> trace/qmp.c | 143 ++++++++++++++++++++++++++++++++++++++++++++-----------
>>>> 4 files changed, 147 insertions(+), 37 deletions(-)
>>>>
>>>> diff --git a/monitor.c b/monitor.c
>>>> index a27e115..bb89877 100644
>>>> --- a/monitor.c
>>>> +++ b/monitor.c
>>>> @@ -910,7 +910,7 @@ static void hmp_trace_event(Monitor *mon, const QDict *qdict)
>>>> bool new_state = qdict_get_bool(qdict, "option");
>>>> Error *local_err = NULL;
>>>>
>>>> - qmp_trace_event_set_state(tp_name, new_state, true, true, &local_err);
>>>> + qmp_trace_event_set_state(tp_name, new_state, true, true, false, 0, &local_err);
>>>> if (local_err) {
>>>> error_report_err(local_err);
>>>> }
>>>> @@ -1069,7 +1069,7 @@ static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
>>>>
>>>> static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
>>>> {
>>>> - TraceEventInfoList *events = qmp_trace_event_get_state("*", NULL);
>>>> + TraceEventInfoList *events = qmp_trace_event_get_state("*", false, 0, NULL);
>>>> TraceEventInfoList *elem;
>>>>
>>>> for (elem = events; elem != NULL; elem = elem->next) {
>>
>>> The new feature remains inaccessible in HMP. Any plans to extend HMP?
>>> Any reasons not to?
>>
>> BTW, I was just looking at "info trace-events" and it only shows the state for
>> all events (there's no name pattern argument). Is it worth updating HMP knowing
>> it should be replaced in favour of the newer QAPI interface?
> QMP is not meant as replacement for HMP. QMP is a stable interface for
> machines, HMP is a convenient interface for humans. New functionality
> is usually added to both. HMP sometimes gets additional convenience
> features. In rare cases, a feature is deemed of no interest to humans,
> and left out of HMP. Is filtering by vCPU such a case?
Oh, I thought there were plans to provide an HMP-like interface from the events
in the QAPI files. But maybe I just mixed this with the plans for dropping the
QMP commands file in favour of the QAPI command definitions.
I'll then add the proper HMP commands.
Thanks,
Lluis
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property
2016-06-20 12:05 ` Stefan Hajnoczi
@ 2016-06-20 12:40 ` Lluís Vilanova
2016-06-21 9:21 ` Stefan Hajnoczi
0 siblings, 1 reply; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-20 12:40 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Stefan Hajnoczi, Eduardo Habkost, Peter Crosthwaite, Riku Voipio,
qemu-devel, Blue Swirl, Paolo Bonzini, Richard Henderson
Stefan Hajnoczi writes:
> On Fri, Jun 17, 2016 at 10:16:25PM +0200, Lluís Vilanova wrote:
>> Stefan Hajnoczi writes:
>>
>> > On Tue, Jun 14, 2016 at 03:11:12PM +0200, Lluís Vilanova wrote:
>> >> @@ -1116,6 +1117,7 @@ int main(int argc, char **argv)
>> >> gdbserver_start (gdbstub_port);
>> >> gdb_handlesig(cpu, 0);
>> >> }
>> >> + trace_init_vcpu_events();
>>
>> > Do vcpu events make sense in *-user builds? I thought this feature is
>> > only available in *-softmmu.
>>
>> It's here for completeness, although it's currently useless. AFAIK, QEMU has
>> multiple vCPUs when the user application is multi-threaded, but user QEMU has no
>> interface to interactively control event states.
> Yes, please drop the *-user changes. I see no trace_init_*() calls in
> *-user so it won't be useful to call trace_init_vcpu_events().
Actually, I was thinking on extending the *-user command line to accept
something similar to softmmu to control what and where to trace. I guess this
should go onto a separate series (or can I fold it into this one?).
Cheers,
Lluis
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state
2016-06-20 12:13 ` Markus Armbruster
@ 2016-06-20 12:44 ` Lluís Vilanova
0 siblings, 0 replies; 20+ messages in thread
From: Lluís Vilanova @ 2016-06-20 12:44 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, Eduardo Habkost, qemu-devel, Stefan Hajnoczi,
Luiz Capitulino
Markus Armbruster writes:
> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>> Markus Armbruster writes:
>>
>>> Lluís Vilanova <vilanova@ac.upc.edu> writes:
[...]
>>>> diff --git a/qapi/trace.json b/qapi/trace.json
>>>> index 01b0a52..25d8095 100644
>>>> --- a/qapi/trace.json
>>>> +++ b/qapi/trace.json
>>>> @@ -1,6 +1,6 @@
>>>> # -*- mode: python -*-
>>>> #
>>>> -# Copyright (C) 2011-2014 Lluís Vilanova <vilanova@ac.upc.edu>
>>>> +# Copyright (C) 2011-2016 Lluís Vilanova <vilanova@ac.upc.edu>
>>>> #
>>>> # This work is licensed under the terms of the GNU GPL, version 2 or later.
>>>> # See the COPYING file in the top-level directory.
>>>> @@ -29,11 +29,12 @@
>>>> #
>>>> # @name: Event name.
>>>> # @state: Tracing state.
>>>> +# @vcpu: Whether this is a per-vCPU event (since 2.7).
>>>> #
>>>> # Since 2.2
>>>> ##
>>>> { 'struct': 'TraceEventInfo',
>>>> - 'data': {'name': 'str', 'state': 'TraceEventState'} }
>>>> + 'data': {'name': 'str', 'state': 'TraceEventState', 'vcpu': 'bool'} }
>>>>
>>>> ##
>>>> # @trace-event-get-state:
>>>> @@ -41,13 +42,18 @@
>>>> # Query the state of events.
>>>> #
>>>> # @name: Event name pattern (case-sensitive glob).
>>>> +# @vcpu: #optional The vCPU to check (any by default; since 2.7).
>>>> #
>>>> # Returns: a list of @TraceEventInfo for the matching events
>>>> #
>>>> +# For any event without the "vcpu" property:
>>
>>> All events have the vcpu property, but for some of them the property's
>>> value is false.
>>
>> Hmmm, do you mean in the generated event array? Here, I was referring to wether
>> an event has the "vcpu" property in "trace-events". I can clarify that.
> What is "trace-events"? The file in the source tree?
Exactly. Adding the "tcgv" property to an event on that file is what defines
whether the event is per-vCPU.
[...]
>>>> +# - If @name is not a pattern and @vcpu is set, an error is raised.
>>
>>> Perhaps we could clarify as follows:
>>
>>> # Returns: a list of @TraceEventInfo for the matching events
>>> #
>>> # An event matches if
>>> # - its name matches the @name pattern, and
>>> # - if @vcpu is given, its vCPU equals @vcpu.
>>> # It follows that with @vcpu given, the query can match only per-vCPU
>>> # events. Special case: if the name is an exact match, you get an error
>>> # instead of an empty list.
>>
>> Doesn't sound entirely right to me:
>>
>> # Returns: a list of @TraceEventInfo for the matching events
>> #
>> # An event matches if
>> # - its name matches the @name pattern, and
>> # - if @vcpu is given, the event is a per-vCPU one (has the "vcpu" property in
>> # "trace-events").
> Fails to mention that @vcpu selects the TraceEventInfo for that
> particular VCPU. That's what I tried to express by "its vCPU equals
> @vcpu".
[...]
Right.
Thanks,
Lluis
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property
2016-06-20 12:40 ` Lluís Vilanova
@ 2016-06-21 9:21 ` Stefan Hajnoczi
0 siblings, 0 replies; 20+ messages in thread
From: Stefan Hajnoczi @ 2016-06-21 9:21 UTC (permalink / raw)
To: Stefan Hajnoczi, Eduardo Habkost, Peter Crosthwaite, Riku Voipio,
qemu-devel, Blue Swirl, Paolo Bonzini, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1290 bytes --]
On Mon, Jun 20, 2016 at 02:40:35PM +0200, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
>
> > On Fri, Jun 17, 2016 at 10:16:25PM +0200, Lluís Vilanova wrote:
> >> Stefan Hajnoczi writes:
> >>
> >> > On Tue, Jun 14, 2016 at 03:11:12PM +0200, Lluís Vilanova wrote:
> >> >> @@ -1116,6 +1117,7 @@ int main(int argc, char **argv)
> >> >> gdbserver_start (gdbstub_port);
> >> >> gdb_handlesig(cpu, 0);
> >> >> }
> >> >> + trace_init_vcpu_events();
> >>
> >> > Do vcpu events make sense in *-user builds? I thought this feature is
> >> > only available in *-softmmu.
> >>
> >> It's here for completeness, although it's currently useless. AFAIK, QEMU has
> >> multiple vCPUs when the user application is multi-threaded, but user QEMU has no
> >> interface to interactively control event states.
>
> > Yes, please drop the *-user changes. I see no trace_init_*() calls in
> > *-user so it won't be useful to call trace_init_vcpu_events().
>
> Actually, I was thinking on extending the *-user command line to accept
> something similar to softmmu to control what and where to trace. I guess this
> should go onto a separate series (or can I fold it into this one?).
Please do that in a separate series. Tracing could indeed be useful for
*-user.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-06-21 9:21 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-14 13:10 [Qemu-devel] [PATCH v4 0/6] trace: Per-vCPU tracing states Lluís Vilanova
2016-06-14 13:10 ` [Qemu-devel] [PATCH v4 1/6] trace: Identify events with the 'vcpu' property Lluís Vilanova
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 2/6] disas: Remove unused macro '_' Lluís Vilanova
2016-06-17 15:25 ` Stefan Hajnoczi
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 3/6] [trivial] trace: Cosmetic changes on fast-path tracing Lluís Vilanova
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property Lluís Vilanova
2016-06-17 15:24 ` Stefan Hajnoczi
2016-06-17 20:16 ` Lluís Vilanova
2016-06-20 12:05 ` Stefan Hajnoczi
2016-06-20 12:40 ` Lluís Vilanova
2016-06-21 9:21 ` Stefan Hajnoczi
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 5/6] trace: Conditionally trace events based on their per-vCPU state Lluís Vilanova
2016-06-14 13:11 ` [Qemu-devel] [PATCH v4 6/6] trace: Add QAPI/QMP interfaces to query and control per-vCPU tracing state Lluís Vilanova
2016-06-20 7:23 ` Markus Armbruster
2016-06-20 10:48 ` Lluís Vilanova
2016-06-20 12:13 ` Markus Armbruster
2016-06-20 12:44 ` Lluís Vilanova
2016-06-20 10:53 ` Lluís Vilanova
2016-06-20 12:17 ` Markus Armbruster
2016-06-20 12:37 ` Lluís Vilanova
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).