mbox series

[v2,0/4,RfC] fix tracing for modules

Message ID 20201124160255.28111-1-kraxel@redhat.com
Headers show
Series fix tracing for modules | expand

Message

Gerd Hoffmann Nov. 24, 2020, 4:02 p.m. UTC 
First version that actually works.  Only qxl covered for this RfC, other
modules will follow once the basics are hashed out.

More context:
  https://bugzilla.redhat.com/show_bug.cgi?id=1898700
  https://bugzilla.redhat.com/show_bug.cgi?id=1869642

take care,
  Gerd

Gerd Hoffmann (4):
  meson: add trace_events_config[]
  meson: move up hw subdir (specifically before trace subdir)
  meson: add module_trace & module_trace_src
  meson: move qxl trace events to separate file

 hw/display/qxl-render.c     |  1 +
 hw/display/qxl.c            |  1 +
 hw/display/meson.build      |  5 +++
 hw/display/trace-events     | 67 -------------------------------------
 hw/display/trace-events-qxl | 66 ++++++++++++++++++++++++++++++++++++
 meson.build                 |  7 ++--
 trace/meson.build           | 37 +++++++++++++++-----
 7 files changed, 106 insertions(+), 78 deletions(-)
 create mode 100644 hw/display/trace-events-qxl

Comments

Stefan Hajnoczi Nov. 30, 2020, 4:56 p.m. UTC | #1 
On Tue, Nov 24, 2020 at 05:02:51PM +0100, Gerd Hoffmann wrote:
> First version that actually works.  Only qxl covered for this RfC, other
> modules will follow once the basics are hashed out.
> 
> More context:
>   https://bugzilla.redhat.com/show_bug.cgi?id=1898700
>   https://bugzilla.redhat.com/show_bug.cgi?id=1869642
> 
> take care,
>   Gerd
> 
> Gerd Hoffmann (4):
>   meson: add trace_events_config[]
>   meson: move up hw subdir (specifically before trace subdir)
>   meson: add module_trace & module_trace_src
>   meson: move qxl trace events to separate file

Awesome, thank you for working on this!

I noticed an issue with simpletrace: the trace file does not contain
qxl_* TRACE_RECORD_TYPE_MAPPING records when ./configure
--enable-modules is used. This happens because st_write_event_mapping()
is called before the qxl module calls trace_event_register_group().

(The mapping records describe the integer ID to string name mapping used
in a simpletrace file.)

You can check this using "grep -a qxl_ trace-$LAST_QEMU_PID" after
running qemu --device qxl built with ./configure --enable-modules
--enable-trace-backend=simple.

Remove --enable-modules and the file will contain the qxl_ trace events.

This means the trace file is broken because the simpletrace records
cannot be mapped to a trace event name.

One way to solve this is by modifying trace_event_register_group() to
call into trace/simple.c (maybe with a TraceEventIter initialized to
iterate over the newly registered trace events group?).

Alternatively, simpletrace.c might be able to emit
TRACE_RECORD_TYPE_MAPPING on demand instead of ahead of time.

Stefan
Gerd Hoffmann Dec. 3, 2020, 10:39 a.m. UTC | #2 
Hi,

> I noticed an issue with simpletrace: the trace file does not contain
> qxl_* TRACE_RECORD_TYPE_MAPPING records when ./configure
> --enable-modules is used. This happens because st_write_event_mapping()
> is called before the qxl module calls trace_event_register_group().
> 
> (The mapping records describe the integer ID to string name mapping used
> in a simpletrace file.)

It's not the only initialization issue.  "qemu-system-x86_64 -vga qxl
-trace qxl*" doesn't work either for simliar reasons.

> One way to solve this is by modifying trace_event_register_group() to
> call into trace/simple.c (maybe with a TraceEventIter initialized to
> iterate over the newly registered trace events group?).
> 
> Alternatively, simpletrace.c might be able to emit
> TRACE_RECORD_TYPE_MAPPING on demand instead of ahead of time.

I guess I'll have a look at the first option, or more general at
per-group initialization, that looks like the better approach on
a quick glance.

take care,
  Gerd
Gerd Hoffmann Dec. 15, 2020, 4:36 p.m. UTC | #3 
Hi,

> One way to solve this is by modifying trace_event_register_group() to
> call into trace/simple.c (maybe with a TraceEventIter initialized to
> iterate over the newly registered trace events group?).

https://git.kraxel.org/cgit/qemu/log/?h=sirius/trace-modules

Not solved yet: -trace "events=qxl*" not working, because
trace_enable_events runs before modules are loaded.

take care,
  Gerd