[Qemu-devel] [PATCH for-2.8 v1 10/60] trace: remove fixed global event state arrays

["Daniel P. Berrange" <berrange@redhat.com>]

There are currently two global event state arrays,
'uint16 dstate[]' and 'bool dstate_init[]' whose
sized is defined based on the TRACE_EVENT_COUNT
enum value.

When multiple event groups are enabled, it won't
be possible to used a fixed global event state.
This change thus expands the event group register
method so that the event state arrays can be passed
in when the group is registered.

When calling the methods for setting/getting the
trace event state, the caller also needs to pass
in the corresponding 'dstate' array. While it
would be possible to look up the array in the
group list, these methods are in the hot-path,
so passing in the array directly is hugely more
efficient. The extra parameter is no real burden
on callers, since all the code is auto-generated,
with the exception of a call in hw/usb/hcd-ohci.c

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 hw/usb/hcd-ohci.c                    |  4 +--
 scripts/tracetool/backend/simple.py  |  2 +-
 scripts/tracetool/format/events_c.py |  7 +++--
 scripts/tracetool/format/events_h.py |  3 ++
 stubs/trace-control.c                |  8 ++----
 trace/control-internal.h             | 17 ++++++-----
 trace/control-target.c               | 14 ++++-----
 trace/control.c                      | 56 ++++++++++++++++++++++++++----------
 trace/control.h                      | 15 ++++++----
 trace/qmp.c                          | 17 +++++++----
 10 files changed, 93 insertions(+), 50 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index fa57038..87508a7 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -937,8 +937,8 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
 #ifdef trace_event_get_state
 static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len)
 {
-    bool print16 = !!trace_event_get_state(TRACE_USB_OHCI_TD_PKT_SHORT);
-    bool printall = !!trace_event_get_state(TRACE_USB_OHCI_TD_PKT_FULL);
+    bool print16 = !!trace_event_get_state(dstate, TRACE_USB_OHCI_TD_PKT_SHORT);
+    bool printall = !!trace_event_get_state(dstate, TRACE_USB_OHCI_TD_PKT_FULL);
     const int width = 16;
     int i;
     char tmp[3 * width + 1];
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 1bccada..c2a8468 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -73,7 +73,7 @@ def generate_c(event):
         # already checked on the generic format code
         cond = "true"
     else:
-        cond = "trace_event_get_state(%s)" % event_id
+        cond = "trace_event_get_state(dstate, %s)" % event_id
 
     out('',
         '    if (!%(cond)s) {',
diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/format/events_c.py
index 9203377..bab6404 100644
--- a/scripts/tracetool/format/events_c.py
+++ b/scripts/tracetool/format/events_c.py
@@ -25,7 +25,10 @@ def generate(events, backend):
         '#include "trace/control.h"',
         '')
 
-    out('TraceEvent trace_events[TRACE_EVENT_COUNT] = {')
+    out('uint16_t dstate[TRACE_EVENT_COUNT];')
+    out('bool dstate_init[TRACE_EVENT_COUNT];')
+
+    out('static TraceEvent trace_events[TRACE_EVENT_COUNT] = {')
 
     for e in events:
         if "vcpu" in e.properties:
@@ -45,6 +48,6 @@ def generate(events, backend):
 
     out('void trace_register_events(void)',
         '{',
-        '    trace_event_register_group(trace_events, TRACE_EVENT_COUNT);',
+        '    trace_event_register_group(trace_events, TRACE_EVENT_COUNT, dstate, dstate_init);',
         '}',
         'trace_init(trace_register_events)')
diff --git a/scripts/tracetool/format/events_h.py b/scripts/tracetool/format/events_h.py
index 291c183..d6465d4 100644
--- a/scripts/tracetool/format/events_h.py
+++ b/scripts/tracetool/format/events_h.py
@@ -55,6 +55,9 @@ def generate(events, backend):
                 enabled=enabled)
         out('#define TRACE_%s_ENABLED %d' % (e.name.upper(), enabled))
 
+    out('extern uint16_t dstate[TRACE_EVENT_COUNT];')
+    out('extern bool dstate_init[TRACE_EVENT_COUNT];')
+
     out('#include "trace/event-internal.h"',
         'void trace_register_events(void);',
         '',
diff --git a/stubs/trace-control.c b/stubs/trace-control.c
index fe59836..31566c2 100644
--- a/stubs/trace-control.c
+++ b/stubs/trace-control.c
@@ -11,16 +11,12 @@
 #include "trace/control.h"
 
 
-void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
+void trace_event_set_state_dynamic(uint16_t *dstate, 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,
+void trace_event_set_vcpu_state_dynamic(uint16_t *dstate, CPUState *vcpu,
                                         TraceEvent *ev, bool state)
 {
     /* should never be called on non-target binaries */
diff --git a/trace/control-internal.h b/trace/control-internal.h
index e95b031..634effe 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -15,7 +15,6 @@
 #include "qom/cpu.h"
 
 
-extern uint16_t trace_events_dstate[];
 extern int trace_events_enabled_count;
 
 
@@ -53,22 +52,24 @@ static inline bool trace_event_get_state_static(TraceEvent *ev)
     return ev->sstate;
 }
 
-static inline bool trace_event_get_state_dynamic_by_id(TraceEventID id)
+static inline bool trace_event_get_state_dynamic_by_id(
+    uint16_t *trace_events_dstate, 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)
+static inline bool trace_event_get_state_dynamic(
+    uint16_t *trace_events_dstate, TraceEvent *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);
+    return trace_event_get_state_dynamic_by_id(trace_events_dstate, id);
 }
 
-static inline bool trace_event_get_vcpu_state_dynamic_by_vcpu_id(CPUState *vcpu,
-                                                                 TraceEventVCPUID id)
+static inline bool trace_event_get_vcpu_state_dynamic_by_vcpu_id(
+    CPUState *vcpu, TraceEventVCPUID id)
 {
     /* it's on fast path, avoid consistency checks (asserts) */
     if (unlikely(trace_events_enabled_count)) {
@@ -89,6 +90,8 @@ static inline bool trace_event_get_vcpu_state_dynamic(CPUState *vcpu,
 
 
 void trace_event_register_group(TraceEvent *events,
-                                size_t nevents);
+                                size_t nevents,
+                                uint16_t *dstate,
+                                bool *dstate_init);
 
 #endif /* TRACE__CONTROL_INTERNAL_H */
diff --git a/trace/control-target.c b/trace/control-target.c
index 74c029a..da326b4 100644
--- a/trace/control-target.c
+++ b/trace/control-target.c
@@ -13,22 +13,22 @@
 #include "translate-all.h"
 
 
-void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
+void trace_event_set_state_dynamic(uint16_t *dstate, 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);
+            trace_event_set_vcpu_state_dynamic(dstate, 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;
+        trace_events_enabled_count += state - dstate[id];
+        dstate[id] = state;
     }
 }
 
-void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
+void trace_event_set_vcpu_state_dynamic(uint16_t *dstate, CPUState *vcpu,
                                         TraceEvent *ev, bool state)
 {
     TraceEventID id;
@@ -43,11 +43,11 @@ void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
         if (state) {
             trace_events_enabled_count++;
             set_bit(vcpu_id, vcpu->trace_dstate);
-            trace_events_dstate[id]++;
+            dstate[id]++;
         } else {
             trace_events_enabled_count--;
             clear_bit(vcpu_id, vcpu->trace_dstate);
-            trace_events_dstate[id]--;
+            dstate[id]--;
         }
     }
 }
diff --git a/trace/control.c b/trace/control.c
index d0aa075..29659a1 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -29,20 +29,22 @@ int trace_events_enabled_count;
 typedef struct TraceEventGroup {
     TraceEvent *events;
     size_t nevents;
+    /*
+     * 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 *dstate;
+    /* Marks events for late vCPU state init */
+    bool *dstate_init;
 } TraceEventGroup;
 
 static bool have_vcpu_events;
 static TraceEventGroup *event_groups;
 static size_t nevent_groups;
 
-/*
- * 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];
+static bool pattern_glob(const char *pat, const char *ev);
 
 QemuOptsList qemu_trace_opts = {
     .name = "trace",
@@ -66,7 +68,9 @@ QemuOptsList qemu_trace_opts = {
 
 
 void trace_event_register_group(TraceEvent *events,
-                                size_t nevents)
+                                size_t nevents,
+                                uint16_t *dstate,
+                                bool *dstate_init)
 {
     size_t nvcpuevents = 0;
     for (size_t i = 0; i < nevents; i++) {
@@ -85,6 +89,8 @@ void trace_event_register_group(TraceEvent *events,
     event_groups = g_renew(TraceEventGroup, event_groups, nevent_groups + 1);
     event_groups[nevent_groups].events = events;
     event_groups[nevent_groups].nevents = nevents;
+    event_groups[nevent_groups].dstate = dstate;
+    event_groups[nevent_groups].dstate_init = dstate_init;
     nevent_groups++;
 }
 
@@ -145,6 +151,14 @@ void trace_event_iter_init(TraceEventIter *iter, const char *pattern)
 
 TraceEvent *trace_event_iter_next(TraceEventIter *iter)
 {
+    return trace_event_iter_next_full(iter, NULL, NULL);
+}
+
+
+TraceEvent *trace_event_iter_next_full(TraceEventIter *iter,
+                                       uint16_t **dstate,
+                                       bool **dstate_init)
+{
     TraceEvent *ev;
 
     if (iter->group >= nevent_groups ||
@@ -153,6 +167,12 @@ TraceEvent *trace_event_iter_next(TraceEventIter *iter)
     }
 
     ev = &(event_groups[iter->group].events[iter->event]);
+    if (dstate) {
+        *dstate = event_groups[iter->group].dstate;
+    }
+    if (dstate_init) {
+        *dstate_init = event_groups[iter->group].dstate_init;
+    }
 
     do {
         iter->event++;
@@ -188,9 +208,12 @@ static void do_trace_enable_events(const char *line_buf)
     TraceEventIter iter;
     TraceEvent *ev;
     bool is_pattern = trace_event_is_pattern(line_ptr);
+    uint16_t *dstate;
+    bool *dstate_init;
 
     trace_event_iter_init(&iter, is_pattern ? line_ptr : NULL);
-    while ((ev = trace_event_iter_next(&iter)) != NULL) {
+    while ((ev = trace_event_iter_next_full(&iter, &dstate, &dstate_init)) !=
+           NULL) {
         bool match = false;
         if (is_pattern) {
             if (trace_event_get_state_static(ev)) {
@@ -209,9 +232,9 @@ static void do_trace_enable_events(const char *line_buf)
         }
         if (match) {
             /* start tracing */
-            trace_event_set_state_dynamic(ev, enable);
+            trace_event_set_state_dynamic(dstate, ev, enable);
             /* mark for late vCPU init */
-            trace_events_dstate_init[ev->id] = true;
+            dstate_init[ev->id] = true;
             if (!is_pattern) {
                 return;
             }
@@ -334,12 +357,15 @@ void trace_init_vcpu_events(void)
 {
     TraceEventIter iter;
     TraceEvent *ev;
+    uint16_t *dstate;
+    bool *dstate_init;
     trace_event_iter_init(&iter, NULL);
-    while ((ev = trace_event_iter_next(&iter)) != NULL) {
+    while ((ev = trace_event_iter_next_full(&iter, &dstate, &dstate_init)) !=
+           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);
+            dstate_init[ev->id]) {
+            trace_event_set_state_dynamic(dstate, ev, true);
         }
     }
 }
diff --git a/trace/control.h b/trace/control.h
index c9ea2f6..a5cb7f3 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -35,6 +35,9 @@ enum TraceEventID;
 void trace_event_iter_init(TraceEventIter *iter, const char *pattern);
 
 TraceEvent *trace_event_iter_next(TraceEventIter *iter);
+TraceEvent *trace_event_iter_next_full(TraceEventIter *iter,
+                                       uint16_t **dstate,
+                                       bool **dstate_init);
 
 
 /**
@@ -97,8 +100,8 @@ static const char * trace_event_get_name(TraceEvent *ev);
  *
  * As a down side, you must always use an immediate #TraceEventID value.
  */
-#define trace_event_get_state(id)                       \
-    ((id ##_ENABLED) && trace_event_get_state_dynamic_by_id(id))
+#define trace_event_get_state(dstate, id)                                \
+    ((id ##_ENABLED) && trace_event_get_state_dynamic_by_id(dstate, id))
 
 /**
  * trace_event_get_vcpu_state:
@@ -135,7 +138,8 @@ static bool trace_event_get_state_static(TraceEvent *ev);
  *
  * If the event has the 'vcpu' property, gets the OR'ed state of all vCPUs.
  */
-static bool trace_event_get_state_dynamic(TraceEvent *ev);
+static bool trace_event_get_state_dynamic(uint16_t *dstate,
+                                          TraceEvent *ev);
 
 /**
  * trace_event_get_vcpu_state_dynamic:
@@ -154,7 +158,8 @@ static bool trace_event_get_vcpu_state_dynamic(CPUState *vcpu, TraceEvent *ev);
  *
  * Pre-condition: trace_event_get_state_static(ev) == true
  */
-void trace_event_set_state_dynamic(TraceEvent *ev, bool state);
+void trace_event_set_state_dynamic(uint16_t *dstate, TraceEvent *ev,
+                                   bool state);
 
 /**
  * trace_event_set_vcpu_state_dynamic:
@@ -163,7 +168,7 @@ void trace_event_set_state_dynamic(TraceEvent *ev, bool state);
  *
  * Pre-condition: trace_event_get_vcpu_state_static(ev) == true
  */
-void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
+void trace_event_set_vcpu_state_dynamic(uint16_t *dstate, CPUState *vcpu,
                                         TraceEvent *ev, bool state);
 
 
diff --git a/trace/qmp.c b/trace/qmp.c
index 88a907b..190049a 100644
--- a/trace/qmp.c
+++ b/trace/qmp.c
@@ -73,6 +73,8 @@ TraceEventInfoList *qmp_trace_event_get_state(const char *name,
     TraceEventInfoList *events = NULL;
     TraceEventIter iter;
     TraceEvent *ev;
+    uint16_t *dstate;
+    bool *dstate_init;
     bool is_pattern = trace_event_is_pattern(name);
     CPUState *cpu;
 
@@ -90,9 +92,11 @@ TraceEventInfoList *qmp_trace_event_get_state(const char *name,
 
     /* Get states (all errors checked above) */
     trace_event_iter_init(&iter, is_pattern ? name : NULL);
-    while ((ev = trace_event_iter_next(&iter)) != NULL) {
+    while ((ev = trace_event_iter_next_full(&iter, &dstate, &dstate_init)) !=
+           NULL) {
         TraceEventInfoList *elem;
         bool is_vcpu = trace_event_is_vcpu(ev);
+        g_assert(dstate);
         if (has_vcpu && !is_vcpu) {
             continue;
         }
@@ -115,7 +119,7 @@ TraceEventInfoList *qmp_trace_event_get_state(const char *name,
                 }
                 /* else: already skipped above */
             } else {
-                if (trace_event_get_state_dynamic(ev)) {
+                if (trace_event_get_state_dynamic(dstate, ev)) {
                     elem->value->state = TRACE_EVENT_STATE_ENABLED;
                 } else {
                     elem->value->state = TRACE_EVENT_STATE_DISABLED;
@@ -139,6 +143,8 @@ void qmp_trace_event_set_state(const char *name, bool enable,
     TraceEvent *ev;
     bool is_pattern = trace_event_is_pattern(name);
     CPUState *cpu;
+    uint16_t *dstate;
+    bool *dstate_init;
 
     /* Check provided vcpu */
     cpu = get_cpu(has_vcpu, vcpu, &err);
@@ -155,15 +161,16 @@ void qmp_trace_event_set_state(const char *name, bool enable,
 
     /* Apply changes (all errors checked above) */
     trace_event_iter_init(&iter, name);
-    while ((ev = trace_event_iter_next(&iter)) != NULL) {
+    while ((ev = trace_event_iter_next_full(&iter, &dstate, &dstate_init)) !=
+           NULL) {
         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);
+            trace_event_set_vcpu_state_dynamic(dstate, cpu, ev, enable);
         } else {
-            trace_event_set_state_dynamic(ev, enable);
+            trace_event_set_state_dynamic(dstate, ev, enable);
         }
     }
 }
-- 
2.7.4