Re: [Qemu-devel] [RFC] tracing: consistent usage of "disable" in "trace-events"

[Lluís]

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.

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".


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth