[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 0/8] [RfC] fix tracing for modules
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v3 0/8] [RfC] fix tracing for modules |
Date: |
Mon, 22 Mar 2021 12:03:42 +0000 |
User-agent: |
Mutt/2.0.5 (2021-01-21) |
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 :|