Re: [PATCH v3 0/8] [RfC] fix tracing for modules

[Daniel P . Berrangé]

On Mon, Feb 22, 2021 at 04:13:32PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > > TODO:
> > > Enabling modular tracepoints via -trace cmd line doesn't work yet.
> > > Guess we need to store the list somewhere for later re-processing.
> > > Error handling is tricky, specifically the "tracepoint doesn't exist"
> > > error.  Suggestions / ideas are welcome.
> > 
> > Two ideas:
> > 
> > Global trace event name list
> > ----------------------------
> > Build *some* global information about all trace events, including
> > modules, into the main QEMU binary. For example, generate an array of
> > all trace event names so QEMU can always print an error if a
> > non-existent trace event name is used. (This is similar to the
> > trace-events-all file, which is a global list of all trace events.)
> > 
> > Module name prefixes
> > --------------------
> > Allow an optional module/group prefix like qxl:my_trace_event. When the
> > user says:
> > 
> >   --trace qxl:my_trace_event
> > 
> > QEMU knows that this trace event belongs to the "qxl" module/group. It
> > will not attempt to load it until the qxl module registers itself.
> > 
> > If "my_trace_event" doesn't exist in the qxl module:
> > 1. If the qxl module is not loaded we don't hit an error. Nevermind.
> > 2. When the qxl module is loaded pending events are resolved and an
> >    error is printed.
> 
> Finally found the time to look at this again... 
> 
> So, we already have a "group".  Which is basically the sub-directory of
> the trace-events file right now, and it seems to be mostly a build system
> thing.  We get many small lists instead of one huge, but there seems to
> be no other effect.  We could change that though, by giving each group
> an (optional?) prefix.
> 
> There also is a probe prefix, apparently used by dtrace only.  Not sure
> how to deal with that.  It prefix is qemu-<target-type>-<target-name>.
> Giving qemu modules its own dtrace prefix looks sensible to me.  That
> would probably something like "qemu-module-<name>".

The prefix is used by the systemtap backend. It lets us define
friendly probes, scoped for each QEMU emulator target.

eg a trace point "dma_map_wait" gets mapped to probes in many
.stp files, once per target, because we need to match based on
the executable path:

  probe qemu.system.x86_64.dma_map_wait = 
process("/usr/libexec/qemu-system-x86_64").mark("dma_map_wait")
  probe qemu.system.x86_64.dma_map_wait = 
process("/usr/libexec/qemu-system-ppc64").mark("dma_map_wait")
  probe qemu.system.x86_64.dma_map_wait = 
process("/usr/libexec/qemu-system-aarch64").mark("dma_map_wait")

there is nothing here that reqiures us to change the prefix for
a module - we can carry on using the same systemtap probe name
whether modules are used or not.


Currently we kind of unofficially have a convention that the name
of a trace point should reflect its functional area. So all QXL
related probes have a name prefix "qxl_".

We could make that explicit, by having the trace-events files
support a group, IOW instead of

  qxl_destroy_primary(int qid) "%d"
  qxl_enter_vga_mode(int qid) "%d"
  qxl_exit_vga_mode(int qid) "%d"

We could say that a dot separates the group from the probe
name, and thus have


  qxl.destroy_primary(int qid) "%d"
  qxl.enter_vga_mode(int qid) "%d"
  qxl.exit_vga_mode(int qid) "%d"

this would be backwards compatible, as we can turn the "." back into
a "_" for all existing trace backends, except stp.

NB this is all tagential to use of modules at a build system /runtime
level. I think this explicit grouping of probes would make sense for
everything, to make the naming convention explicit instead of implicit.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|