From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44475) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsS6X-000459-59 for qemu-devel@nongnu.org; Fri, 07 Oct 2016 06:10:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bsS6T-0004ZF-PV for qemu-devel@nongnu.org; Fri, 07 Oct 2016 06:10:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45436) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsS6T-0004Ya-Dv for qemu-devel@nongnu.org; Fri, 07 Oct 2016 06:10:09 -0400 From: Stefan Hajnoczi Date: Fri, 7 Oct 2016 11:09:28 +0100 Message-Id: <1475834979-4980-10-git-send-email-stefanha@redhat.com> In-Reply-To: <1475834979-4980-1-git-send-email-stefanha@redhat.com> References: <1475834979-4980-1-git-send-email-stefanha@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [PULL 09/20] trace: remove the TraceEventID and TraceEventVCPUID enums List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Peter Maydell , "Daniel P. Berrange" , Stefan Hajnoczi From: "Daniel P. Berrange" 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=C3=ADs 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/back= end/simple.py index 1bccada..1114e35 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=3Dcond, - event_id=3Devent_id, + event_obj=3Devent.api(event.QEMU_EVENT), size_str=3Dsizestr) =20 if len(event.args) > 0: diff --git a/scripts/tracetool/format/events_c.py b/scripts/tracetool/for= mat/events_c.py index a97054f..40ae395 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)) =20 + next_id =3D 0 + next_vcpu_id =3D 0 for e in events: + id =3D next_id + next_id +=3D 1 if "vcpu" in e.properties: - vcpu_id =3D "TRACE_VCPU_" + e.name.upper() + vcpu_id =3D next_vcpu_id + next_vcpu_id +=3D 1 else: - vcpu_id =3D "TRACE_VCPU_EVENT_COUNT" + vcpu_id =3D "TRACE_VCPU_EVENT_NONE" out('TraceEvent %(event)s =3D {', ' .id =3D %(id)s,', ' .vcpu_id =3D %(vcpu_id)s,', @@ -41,16 +46,17 @@ def generate(events, backend): ' .dstate =3D &%(dstate)s ', '};', event =3D e.api(e.QEMU_EVENT), - id =3D "TRACE_" + e.name.upper(), + id =3D id, vcpu_id =3D vcpu_id, name =3D e.name, sstate =3D "TRACE_%s_ENABLED" % e.name.upper(), dstate =3D e.api(e.QEMU_DSTATE)) =20 - out('TraceEvent *trace_events[TRACE_EVENT_COUNT] =3D {') + out('TraceEvent *trace_events[] =3D {') =20 for e in events: out(' &%(event)s,', event =3D e.api(e.QEMU_EVENT)) =20 - out('};', + out(' NULL,', + '};', '') diff --git a/scripts/tracetool/format/events_h.py b/scripts/tracetool/for= mat/events_h.py index 80a66c5..ca6d730 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 =3D e.api(e.QEMU_EVENT)) =20 - # 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)) =20 - # per-vCPU event identifiers - out('typedef enum {') + numvcpu =3D len([e for e in events if "vcpu" in e.properties]) =20 - 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) =20 # static state for e in events: diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.p= y index 3763e9a..64a6680 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 =3D next(iter(e.args))[1] cond =3D "trace_event_get_vcpu_state(%(cpu)s,"\ - " TRACE_%(id)s,"\ - " TRACE_VCPU_%(id)s)"\ + " TRACE_%(id)s)"\ % dict( cpu=3Dtrace_cpu, id=3De.name.upper()) diff --git a/trace/control-internal.h b/trace/control-internal.h index 808f85d..9abbc96 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, '*') !=3D NULL; } =20 -static inline TraceEventID trace_event_get_id(TraceEvent *ev) +static inline uint32_t trace_event_get_id(TraceEvent *ev) { assert(ev !=3D NULL); return ev->id; } =20 -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; } =20 static inline bool trace_event_is_vcpu(TraceEvent *ev) { - return ev->vcpu_id !=3D TRACE_VCPU_EVENT_COUNT; + return ev->vcpu_id !=3D TRACE_VCPU_EVENT_NONE; } =20 static inline const char * trace_event_get_name(TraceEvent *ev) @@ -62,12 +62,13 @@ static inline bool trace_event_get_state_dynamic(Trac= eEvent *ev) return unlikely(trace_events_enabled_count) && *ev->dstate; } =20 -static inline bool trace_event_get_vcpu_state_dynamic_by_vcpu_id(CPUStat= e *vcpu, - TraceEv= entVCPUID 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 =3D trace_event_get_vcpu_id(ev); - return trace_event_get_vcpu_state_dynamic_by_vcpu_id(vcpu, id); + vcpu_id =3D trace_event_get_vcpu_id(ev); + return trace_event_get_vcpu_state_dynamic_by_vcpu_id(vcpu, vcpu_id); } =20 #endif /* TRACE__CONTROL_INTERNAL_H */ diff --git a/trace/control-target.c b/trace/control-target.c index 3b55941..7ebf6e0 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 9035846..6b32511 100644 --- a/trace/control.c +++ b/trace/control.c @@ -105,7 +105,7 @@ void trace_event_iter_init(TraceEventIter *iter, cons= t char *pattern) =20 TraceEvent *trace_event_iter_next(TraceEventIter *iter) { - while (iter->event < TRACE_EVENT_COUNT) { + while (trace_events[iter->event] !=3D NULL) { TraceEvent *ev =3D trace_events[iter->event]; iter->event++; if (!iter->pattern || diff --git a/trace/control.h b/trace/control.h index 10d4aba..cccd2a2 100644 --- a/trace/control.h +++ b/trace/control.h @@ -18,17 +18,6 @@ typedef struct TraceEventIter { const char *pattern; } TraceEventIter; =20 -/** - * TraceEventID: - * - * Unique tracing event identifier. - * - * These are named as 'TRACE_${EVENT_NAME}'. - * - * See also: "trace/generated-events.h" - */ -enum TraceEventID; - =20 /** * 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); =20 /** * 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-spe= cific + * Special value #TRACE_VCPU_EVENT_NONE means the event is not vCPU-spec= ific * (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); =20 /** * trace_event_is_vcpu: @@ -104,14 +93,12 @@ static const char * trace_event_get_name(TraceEvent = *ev); =20 /** * 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 perfor= mance * 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 g= iven * vCPU. * * If the event has the disabled property, the check will have no perfor= mance * 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(vc= pu, 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)) =20 /** * trace_event_get_state_static: diff --git a/trace/event-internal.h b/trace/event-internal.h index 58f0551..f63500b 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 =20 +/* + * 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 2f09daf..2ec32e1 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -17,8 +17,8 @@ #include "trace/control.h" #include "trace/simple.h" =20 -/** Trace file header event ID */ -#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with TraceE= ventIDs */ +/** Trace file header event ID, picked to avoid conflict with real event= IDs */ +#define HEADER_EVENT_ID (~(uint64_t)0) =20 /** Trace file magic number */ #define HEADER_MAGIC 0xf2b177cb0aa429b4ULL @@ -58,7 +58,7 @@ static char *trace_file_name; =20 /* * 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, c= onst char *s, uint32_t slen rec->rec_off =3D write_to_buffer(rec->rec_off, (void*)s, slen); } =20 -int trace_record_start(TraceBufferRecord *rec, TraceEventID event, size_= t datasize) +int trace_record_start(TraceBufferRecord *rec, uint32_t event, size_t da= tasize) { unsigned int idx, rec_off, old_idx, new_idx; uint32_t rec_len =3D sizeof(TraceRecord) + datasize; diff --git a/trace/simple.h b/trace/simple.h index 1e7de45..17ce472 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 a= rglen); +int trace_record_start(TraceBufferRecord *rec, uint32_t id, size_t argle= n); =20 /** * Append a 64-bit argument to a trace record --=20 2.7.4