Re: [Qemu-devel] [PATCH] trace: Remove 'trace_events_dstate_init'

[Lluís Vilanova]

Daniel P Berrange writes:

> On Wed, Aug 10, 2016 at 06:57:34PM +0200, Lluís Vilanova wrote:
>> Removes the event state array used for early initialization. Since only
>> events with the "vcpu" property need a late initialization fixup,
>> threats their initialization specially.
>> 
>> Assumes that the user won't touch the state of "vcpu" events between
>> early and late initialization (e.g., through QMP).
>> 
>> Signed-off-by: Lluís Vilanova <address@hidden>
>> ---
>> stubs/trace-control.c  |    5 +++++
>> trace/control-target.c |    9 +++++++++
>> trace/control.c        |   21 ++++++++-------------
>> trace/control.h        |    3 +++
>> trace/event-internal.h |    1 +
>> 5 files changed, 26 insertions(+), 13 deletions(-)
>> 
>> diff --git a/stubs/trace-control.c b/stubs/trace-control.c
>> index fe59836..3740c38 100644
>> --- a/stubs/trace-control.c
>> +++ b/stubs/trace-control.c
>> @@ -11,6 +11,11 @@
>> #include "trace/control.h"
>> 
>> 
>> +void trace_event_set_state_dynamic_init(TraceEvent *ev, bool state)
>> +{
>> +    trace_event_set_state_dynamic(ev, state);
>> +}
>> +
>> void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
>> {
>> TraceEventID id;
>> diff --git a/trace/control-target.c b/trace/control-target.c
>> index 74c029a..4ee3733 100644
>> --- a/trace/control-target.c
>> +++ b/trace/control-target.c
>> @@ -13,6 +13,15 @@
>> #include "translate-all.h"
>> 
>> 
>> +void trace_event_set_state_dynamic_init(TraceEvent *ev, bool state)
>> +{
>> +    TraceEventID id = trace_event_get_id(ev);
>> +    assert(trace_event_get_state_static(ev));
>> +    /* Ignore "vcpu" property, since no vCPUs have been created yet */
>> +    trace_events_enabled_count += state - trace_events_dstate[id];
>> +    trace_events_dstate[id] = state;
>> +}
>> +
>> void trace_event_set_state_dynamic(TraceEvent *ev, bool state)
>> {
>> CPUState *vcpu;
>> diff --git a/trace/control.c b/trace/control.c
>> index d173c09..027adc7 100644
>> --- a/trace/control.c
>> +++ b/trace/control.c
>> @@ -31,8 +31,6 @@ int trace_events_enabled_count;
>> * - 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];
>> 
>> QemuOptsList qemu_trace_opts = {
>> .name = "trace",
>> @@ -142,10 +140,7 @@ 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;
>> +                trace_event_set_state_dynamic_init(ev, enable);
>> }
>> }
>> } else {
>> @@ -157,10 +152,7 @@ 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;
>> +            trace_event_set_state_dynamic_init(ev, enable);
>> }
>> }
>> }
>> @@ -275,9 +267,12 @@ 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]) {
>> +        if (trace_event_is_vcpu(ev) && trace_event_get_state_dynamic(ev)) {
>> +            TraceEventID id = trace_event_get_id(ev);
>> +            /* disable early-init state ... */
>> +            trace_events_dstate[id] = false;

> This variable is uint16_t, so assigning false to it is bogus.

Woops, sorry. I'll re-send with this fix.


Thanks,
  Lluis