From ef4c9fc8542e06b1d567172c04b0c0377c7ab0c5 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 4 Oct 2016 14:35:49 +0100 Subject: [PATCH] trace: remove the TraceEventID and TraceEventVCPUID enums MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TraceEventID and TraceEventVCPUID enums constants are no longer actually used for anything critical. The TRACE_EVENT_COUNT limit is used to determine the size of the TraceEvents array, and can be removed if we just NULL terminate the array instead. The TRACE_VCPU_EVENT_COUNT limit is used as a magic value for marking non-vCPU events, and also for declaring the size of the trace dstate mask in the CPUState struct. The former usage can be replaced by a dedicated constant TRACE_EVENT_VCPU_NONE, defined as (uint32_t)-1. For the latter usage, we can simply define a constant for the number of VCPUs, avoiding the need for the full enum. The only other usages of the enum values can be replaced by accesing the id/vcpu_id fields via the named TraceEvent structs. Reviewed-by: Lluís Vilanova Reviewed-by: Stefan Hajnoczi Signed-off-by: Daniel P. Berrange Message-id: 1475588159-30598-11-git-send-email-berrange@redhat.com Signed-off-by: Stefan Hajnoczi --- scripts/tracetool/backend/simple.py | 4 ++-- scripts/tracetool/format/events_c.py | 16 +++++++++----- scripts/tracetool/format/events_h.py | 19 ++--------------- scripts/tracetool/format/h.py | 3 +-- trace/control-internal.h | 19 +++++++++-------- trace/control-target.c | 2 +- trace/control.c | 2 +- trace/control.h | 32 ++++++++-------------------- trace/event-internal.h | 6 ++++++ trace/simple.c | 8 +++---- trace/simple.h | 2 +- 11 files changed, 48 insertions(+), 65 deletions(-) diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py index 1bccada63d..1114e3550b 100644 --- a/scripts/tracetool/backend/simple.py +++ b/scripts/tracetool/backend/simple.py @@ -80,11 +80,11 @@ def generate_c(event): ' return;', ' }', '', - ' if (trace_record_start(&rec, %(event_id)s, %(size_str)s)) {', + ' if (trace_record_start(&rec, %(event_obj)s.id, %(size_str)s)) {', ' return; /* Trace Buffer Full, Event Dropped ! */', ' }', cond=cond, - event_id=event_id, + event_obj=event.api(event.QEMU_EVENT), size_str=sizestr) if len(event.args) > 0: diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/format/events_c.py index a97054fd58..40ae39568e 100644 --- a/scripts/tracetool/format/events_c.py +++ b/scripts/tracetool/format/events_c.py @@ -28,11 +28,16 @@ def generate(events, backend): for e in events: out('uint16_t %s;' % e.api(e.QEMU_DSTATE)) + next_id = 0 + next_vcpu_id = 0 for e in events: + id = next_id + next_id += 1 if "vcpu" in e.properties: - vcpu_id = "TRACE_VCPU_" + e.name.upper() + vcpu_id = next_vcpu_id + next_vcpu_id += 1 else: - vcpu_id = "TRACE_VCPU_EVENT_COUNT" + vcpu_id = "TRACE_VCPU_EVENT_NONE" out('TraceEvent %(event)s = {', ' .id = %(id)s,', ' .vcpu_id = %(vcpu_id)s,', @@ -41,16 +46,17 @@ def generate(events, backend): ' .dstate = &%(dstate)s ', '};', event = e.api(e.QEMU_EVENT), - id = "TRACE_" + e.name.upper(), + id = id, vcpu_id = vcpu_id, name = e.name, sstate = "TRACE_%s_ENABLED" % e.name.upper(), dstate = e.api(e.QEMU_DSTATE)) - out('TraceEvent *trace_events[TRACE_EVENT_COUNT] = {') + out('TraceEvent *trace_events[] = {') for e in events: out(' &%(event)s,', event = e.api(e.QEMU_EVENT)) - out('};', + out(' NULL,', + '};', '') diff --git a/scripts/tracetool/format/events_h.py b/scripts/tracetool/format/events_h.py index 80a66c57d5..ca6d730519 100644 --- a/scripts/tracetool/format/events_h.py +++ b/scripts/tracetool/format/events_h.py @@ -29,27 +29,12 @@ def generate(events, backend): out('extern TraceEvent %(event)s;', event = e.api(e.QEMU_EVENT)) - # event identifiers - out('typedef enum {') - - for e in events: - out(' TRACE_%s,' % e.name.upper()) - - out(' TRACE_EVENT_COUNT', - '} TraceEventID;') - for e in events: out('extern uint16_t %s;' % e.api(e.QEMU_DSTATE)) - # per-vCPU event identifiers - out('typedef enum {') + numvcpu = len([e for e in events if "vcpu" in e.properties]) - for e in events: - if "vcpu" in e.properties: - out(' TRACE_VCPU_%s,' % e.name.upper()) - - out(' TRACE_VCPU_EVENT_COUNT', - '} TraceEventVCPUID;') + out("#define TRACE_VCPU_EVENT_COUNT %d" % numvcpu) # static state for e in events: diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py index 3763e9aecb..64a6680fdc 100644 --- a/scripts/tracetool/format/h.py +++ b/scripts/tracetool/format/h.py @@ -32,8 +32,7 @@ def generate(events, backend): 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)"\ + " TRACE_%(id)s)"\ % dict( cpu=trace_cpu, id=e.name.upper()) diff --git a/trace/control-internal.h b/trace/control-internal.h index 808f85dc06..9abbc96bcb 100644 --- a/trace/control-internal.h +++ b/trace/control-internal.h @@ -25,20 +25,20 @@ static inline bool trace_event_is_pattern(const char *str) return strchr(str, '*') != NULL; } -static inline TraceEventID trace_event_get_id(TraceEvent *ev) +static inline uint32_t trace_event_get_id(TraceEvent *ev) { assert(ev != NULL); return ev->id; } -static inline TraceEventVCPUID trace_event_get_vcpu_id(TraceEvent *ev) +static inline uint32_t 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; + return ev->vcpu_id != TRACE_VCPU_EVENT_NONE; } static inline const char * trace_event_get_name(TraceEvent *ev) @@ -62,12 +62,13 @@ static inline bool trace_event_get_state_dynamic(TraceEvent *ev) return unlikely(trace_events_enabled_count) && *ev->dstate; } -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, + uint32_t vcpu_id) { /* it's on fast path, avoid consistency checks (asserts) */ if (unlikely(trace_events_enabled_count)) { - return test_bit(id, vcpu->trace_dstate); + return test_bit(vcpu_id, vcpu->trace_dstate); } else { return false; } @@ -76,10 +77,10 @@ static inline bool trace_event_get_vcpu_state_dynamic_by_vcpu_id(CPUState *vcpu, static inline bool trace_event_get_vcpu_state_dynamic(CPUState *vcpu, TraceEvent *ev) { - TraceEventVCPUID id; + uint32_t vcpu_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); + vcpu_id = trace_event_get_vcpu_id(ev); + return trace_event_get_vcpu_state_dynamic_by_vcpu_id(vcpu, vcpu_id); } #endif /* TRACE__CONTROL_INTERNAL_H */ diff --git a/trace/control-target.c b/trace/control-target.c index 3b55941414..7ebf6e0bcb 100644 --- a/trace/control-target.c +++ b/trace/control-target.c @@ -60,7 +60,7 @@ void trace_event_set_state_dynamic(TraceEvent *ev, bool state) void trace_event_set_vcpu_state_dynamic(CPUState *vcpu, TraceEvent *ev, bool state) { - TraceEventVCPUID vcpu_id; + uint32_t vcpu_id; bool state_pre; assert(trace_event_get_state_static(ev)); assert(trace_event_is_vcpu(ev)); diff --git a/trace/control.c b/trace/control.c index 9035846b26..6b32511d60 100644 --- a/trace/control.c +++ b/trace/control.c @@ -105,7 +105,7 @@ void trace_event_iter_init(TraceEventIter *iter, const char *pattern) TraceEvent *trace_event_iter_next(TraceEventIter *iter) { - while (iter->event < TRACE_EVENT_COUNT) { + while (trace_events[iter->event] != NULL) { TraceEvent *ev = trace_events[iter->event]; iter->event++; if (!iter->pattern || diff --git a/trace/control.h b/trace/control.h index 10d4aba78c..cccd2a295e 100644 --- a/trace/control.h +++ b/trace/control.h @@ -18,17 +18,6 @@ typedef struct TraceEventIter { const char *pattern; } TraceEventIter; -/** - * TraceEventID: - * - * Unique tracing event identifier. - * - * These are named as 'TRACE_${EVENT_NAME}'. - * - * See also: "trace/generated-events.h" - */ -enum TraceEventID; - /** * trace_event_iter_init: @@ -76,17 +65,17 @@ static bool trace_event_is_pattern(const char *str); * * Get the identifier of an event. */ -static TraceEventID trace_event_get_id(TraceEvent *ev); +static uint32_t 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 + * Special value #TRACE_VCPU_EVENT_NONE means the event is not vCPU-specific * (does not have the "vcpu" property). */ -static TraceEventVCPUID trace_event_get_vcpu_id(TraceEvent *ev); +static uint32_t trace_event_get_vcpu_id(TraceEvent *ev); /** * trace_event_is_vcpu: @@ -104,14 +93,12 @@ static const char * trace_event_get_name(TraceEvent *ev); /** * trace_event_get_state: - * @id: Event identifier. + * @id: Event identifier name. * * Get the tracing state of an event (both static and dynamic). * * 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_state(id) \ ((id ##_ENABLED) && trace_event_get_state_dynamic_by_id(id)) @@ -119,19 +106,18 @@ static const char * trace_event_get_name(TraceEvent *ev); /** * trace_event_get_vcpu_state: * @vcpu: Target vCPU. - * @id: Event identifier (TraceEventID). - * @vcpu_id: Per-vCPU event identifier (TraceEventVCPUID). + * @id: Event identifier name. * * 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)) +#define trace_event_get_vcpu_state(vcpu, id) \ + ((id ##_ENABLED) && \ + trace_event_get_vcpu_state_dynamic_by_vcpu_id( \ + vcpu, _ ## id ## _EVENT.vcpu_id)) /** * trace_event_get_state_static: diff --git a/trace/event-internal.h b/trace/event-internal.h index 58f0551745..f63500b37e 100644 --- a/trace/event-internal.h +++ b/trace/event-internal.h @@ -10,6 +10,12 @@ #ifndef TRACE__EVENT_INTERNAL_H #define TRACE__EVENT_INTERNAL_H +/* + * Special value for TraceEvent.vcpu_id field to indicate + * that the event is not VCPU specific + */ +#define TRACE_VCPU_EVENT_NONE ((uint32_t)-1) + /** * TraceEvent: * @id: Unique event identifier. diff --git a/trace/simple.c b/trace/simple.c index 2f09dafcbc..2ec32e159c 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -17,8 +17,8 @@ #include "trace/control.h" #include "trace/simple.h" -/** Trace file header event ID */ -#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with TraceEventIDs */ +/** Trace file header event ID, picked to avoid conflict with real event IDs */ +#define HEADER_EVENT_ID (~(uint64_t)0) /** Trace file magic number */ #define HEADER_MAGIC 0xf2b177cb0aa429b4ULL @@ -58,7 +58,7 @@ static char *trace_file_name; /* * Trace buffer entry */ typedef struct { - uint64_t event; /* TraceEventID */ + uint64_t event; /* event ID value */ uint64_t timestamp_ns; uint32_t length; /* in bytes */ uint32_t pid; @@ -202,7 +202,7 @@ void trace_record_write_str(TraceBufferRecord *rec, const char *s, uint32_t slen rec->rec_off = write_to_buffer(rec->rec_off, (void*)s, slen); } -int trace_record_start(TraceBufferRecord *rec, TraceEventID event, size_t datasize) +int trace_record_start(TraceBufferRecord *rec, uint32_t event, size_t datasize) { unsigned int idx, rec_off, old_idx, new_idx; uint32_t rec_len = sizeof(TraceRecord) + datasize; diff --git a/trace/simple.h b/trace/simple.h index 1e7de45575..17ce47260f 100644 --- a/trace/simple.h +++ b/trace/simple.h @@ -33,7 +33,7 @@ typedef struct { * * @arglen number of bytes required for arguments */ -int trace_record_start(TraceBufferRecord *rec, TraceEventID id, size_t arglen); +int trace_record_start(TraceBufferRecord *rec, uint32_t id, size_t arglen); /** * Append a 64-bit argument to a trace record