On Mon, Nov 08, 2010 at 04:55:10PM +0100, Lluís wrote:
> Daniel P Berrange writes:
>
> > On Mon, Nov 08, 2010 at 03:42:15PM +0100, Lluís wrote:
> >> On the current implementation, the "disable" keyword in "trace-events"
> >> has different semantics, depending on the backend:
> >>
> >> * nop : ignored (not a problem)
> >> * simple : enables tracing, but sets dynamic state to disable
> >> * ust : disables tracing (uses nop backend)
> >> * dtrace : same as simple
> >>
> >> Would it be possible to just use nop whenever the event is disabled in
> >> trace-events? If you agree I can cook the patch, as it's pretty simple.
>
> > I don't particularly see the point of the 'disable' keyword existing at
> > all, unless there are performance implications for a particular trace
> > backend. For the DTrace backend I strip & ignore the disable keyword
> > because probes that are compiled in, reduce to a inline conditional
> > check that has no serious overhead when no trace client is active.
>
> I think the same is applicable to the UST backend.
>
> But it is not true when tracing guest events (e.g., memory
> accesses). Not because of the backend, but because of the frequency of
> appearance of such events.
>
> In this case, the auto-generated code is on the lines of (I haven't yet
> posted the patch series producing this):
>
> [called during TCG code generation -- e.g., translate.c]
>
> #define TRACE_CURR_CPU_STATE_SET (cpu_single_env->trace_state_set)
> #define trace_guest_vmem_cpu_event 0 // number of per-CPU trace event
>
> static inline void trace_gen_guest_vmem (TCGv_i64 addr, uint32_t size,
> uint32_t write)
> {
> if (TRACE_CURR_CPU_STATE_SET & (1 << trace_guest_vmem_cpu_event)) {
> gen_helper_proxy_guest_vmem(addr, tcg_const_i32(size),
> tcg_const_i32(write));
> }
> }
>
> [extra helper functions -- declared at helper.h]
>
> void helper_proxy_guest_vmem (uint64_t addr, uint32_t size, uint32_t write)
> {
> trace_guest_vmem(addr, size, write);
> }
>
> (*) A state set is a bitset with the events that have been declared with
> the "gen" keyword in "trace-events".
>
> This code has indeed a performance cost, so I opted to follow the
> approach taken by the UST backend ("disable" produces a trace event with
> "nop"). When I opted for this, only simple and ust where in 'tracetool'.
>
> In any case, there might appear other events that could have performance
> implications, although I understand the ease of usage of having all
> trace events available by default.
Ok, I agree that if we have tracepoints in the kind of places in TCG
you describe, then this could have measurable performance impact.
> That's why I would rather declare all trace-events without the "disable"
> keyword, and leave it only on those events that are known to have a high
> frequency, as no backend should have so poor performance as to force
> events to "disappear".
This sounds like a reasonable plan to me.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
