Message ID | 147084825411.21266.1977426703494992563.stgit@fimbulvetr.bsc.es |
---|---|
State | New |
Headers | show |
Lluís Vilanova writes: > 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). Forgot to cc Daniel, sorry.
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 <vilanova@ac.upc.edu> > --- > 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. > + trace_events_enabled_count--; > + /* ... and properly re-enable */ > trace_event_set_state_dynamic(ev, true); > } > } Regards, Daniel
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 <vilanova@ac.upc.edu> >> --- >> 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
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; + trace_events_enabled_count--; + /* ... and properly re-enable */ trace_event_set_state_dynamic(ev, true); } } diff --git a/trace/control.h b/trace/control.h index 0413b28..27a16fc 100644 --- a/trace/control.h +++ b/trace/control.h @@ -274,6 +274,9 @@ char *trace_opt_parse(const char *optarg); * * Re-synchronize initial event state with vCPUs (which can be created after * trace_init_events()). + * + * Precondition: event states won't be changed between trace_enable_events() and + * trace_init_vcpu_events() (e.g., through QMP). */ void trace_init_vcpu_events(void); diff --git a/trace/event-internal.h b/trace/event-internal.h index 5d8fa97..074faf6 100644 --- a/trace/event-internal.h +++ b/trace/event-internal.h @@ -29,5 +29,6 @@ typedef struct TraceEvent { const bool sstate; } TraceEvent; +void trace_event_set_state_dynamic_init(TraceEvent *ev, bool state); #endif /* TRACE__EVENT_INTERNAL_H */
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 <vilanova@ac.upc.edu> --- 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(-)