From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>,
Markus Armbruster <armbru@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
qemu-devel@nongnu.org
Subject: Re: [PATCH v3 0/8] [RfC] fix tracing for modules
Date: Mon, 22 Mar 2021 12:03:42 +0000 [thread overview]
Message-ID: <YFiHnr/uguP8/Vtz@redhat.com> (raw)
In-Reply-To: <20210222151332.vea6cszd4pwtkeno@sirius.home.kraxel.org>
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 :|
next prev parent reply other threads:[~2021-03-22 12:05 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-21 12:50 [PATCH v3 0/8] [RfC] fix tracing for modules Gerd Hoffmann
2021-01-21 12:50 ` [PATCH v3 1/8] meson: add trace_events_config[] Gerd Hoffmann
2021-01-27 14:48 ` Stefan Hajnoczi
2021-01-21 12:50 ` [PATCH v3 2/8] meson: move up hw subdir (specifically before trace subdir) Gerd Hoffmann
2021-01-27 14:49 ` Stefan Hajnoczi
2021-01-21 12:50 ` [PATCH v3 3/8] meson: add module_trace & module_trace_src Gerd Hoffmann
2021-01-27 15:03 ` Stefan Hajnoczi
2021-01-21 12:50 ` [PATCH v3 4/8] meson: move qxl trace events to separate file Gerd Hoffmann
2021-01-27 15:33 ` Stefan Hajnoczi
2021-03-22 11:43 ` Daniel P. Berrangé
2021-01-21 12:50 ` [PATCH v3 5/8] trace: iter init tweaks Gerd Hoffmann
2021-02-03 16:13 ` Stefan Hajnoczi
2021-01-21 12:50 ` [PATCH v3 6/8] trace: add trace_event_iter_init_group Gerd Hoffmann
2021-02-03 16:16 ` Stefan Hajnoczi
2021-01-21 12:50 ` [PATCH v3 7/8] trace/simple: pass iter to st_write_event_mapping Gerd Hoffmann
2021-02-03 16:17 ` Stefan Hajnoczi
2021-01-21 12:50 ` [PATCH v3 8/8] trace/simple: add st_init_group Gerd Hoffmann
2021-02-03 16:22 ` Stefan Hajnoczi
2021-02-03 16:32 ` [PATCH v3 0/8] [RfC] fix tracing for modules Stefan Hajnoczi
2021-02-22 15:13 ` Gerd Hoffmann
2021-03-22 11:36 ` Stefan Hajnoczi
2021-03-22 11:46 ` Daniel P. Berrangé
2021-03-29 16:32 ` Stefan Hajnoczi
2021-03-22 12:03 ` Daniel P. Berrangé [this message]
2021-03-26 12:47 ` Gerd Hoffmann
2021-03-29 9:23 ` Daniel P. Berrangé
2021-03-29 9:48 ` Gerd Hoffmann
2021-03-29 10:02 ` Daniel P. Berrangé
2021-03-31 10:55 ` Gerd Hoffmann
2021-04-09 9:10 ` Gerd Hoffmann
2021-04-09 13:12 ` Gerd Hoffmann
2021-04-09 13:17 ` Daniel P. Berrangé
2021-04-12 13:07 ` Gerd Hoffmann
2021-04-15 13:10 ` Daniel P. Berrangé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YFiHnr/uguP8/Vtz@redhat.com \
--to=berrange@redhat.com \
--cc=armbru@redhat.com \
--cc=dgilbert@redhat.com \
--cc=kraxel@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).