| Message ID | 20201119084448.24397-5-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | try fix tracing for modules | expand |
On Thu, Nov 19, 2020 at 09:44:48AM +0100, Gerd Hoffmann wrote: > Add source set to trace_events_config, use it in trace/meson.build > so the trace objects are linked to the module not core qemu. > > Not working as intended. > /me looks puzzled. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/display/meson.build | 1 + > trace/meson.build | 9 +++++---- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/hw/display/meson.build b/hw/display/meson.build > index c2fc36e19d3e..94e355ac1e81 100644 > --- a/hw/display/meson.build > +++ b/hw/display/meson.build > @@ -46,6 +46,7 @@ if config_all_devices.has_key('CONFIG_QXL') > trace_events_config += { > 'file' : meson.source_root() / 'hw' / 'display' / 'trace-events-qxl', > 'group' : 'hw_display_qxl', > + 'ss' : qxl_ss, > } > hw_display_modules += {'qxl': qxl_ss} > endif > diff --git a/trace/meson.build b/trace/meson.build > index 66395d3e2ba7..3f0fe7b7b74c 100644 > --- a/trace/meson.build > +++ b/trace/meson.build > @@ -18,6 +18,7 @@ foreach c : trace_events_config > trace_events_files += [ trace_events_file ] > group = '--group=' + c.get('group') > fmt = '@0@-' + c.get('group') + '.@1@' > + module_ss = c.get('ss', trace_ss) One idea: module_ss is already used in other files. Are you sure there isn't an identifier naming conflict?
Hi, > > diff --git a/trace/meson.build b/trace/meson.build > > index 66395d3e2ba7..3f0fe7b7b74c 100644 > > --- a/trace/meson.build > > +++ b/trace/meson.build > > @@ -18,6 +18,7 @@ foreach c : trace_events_config > > trace_events_files += [ trace_events_file ] > > group = '--group=' + c.get('group') > > fmt = '@0@-' + c.get('group') + '.@1@' > > + module_ss = c.get('ss', trace_ss) > > One idea: module_ss is already used in other files. Are you sure there > isn't an identifier naming conflict? Nope. Tried s/module_ss/kraxel_ss/, still not working. I get tons of "undefined reference to `_TRACE_something'" errors (*not* qxl). Seems trace_ss is not updated as intended. take care, Gerd
On Thu, Nov 19, 2020 at 12:23:23PM +0100, Gerd Hoffmann wrote: > Hi, > > > > diff --git a/trace/meson.build b/trace/meson.build > > > index 66395d3e2ba7..3f0fe7b7b74c 100644 > > > --- a/trace/meson.build > > > +++ b/trace/meson.build > > > @@ -18,6 +18,7 @@ foreach c : trace_events_config > > > trace_events_files += [ trace_events_file ] > > > group = '--group=' + c.get('group') > > > fmt = '@0@-' + c.get('group') + '.@1@' > > > + module_ss = c.get('ss', trace_ss) > > > > One idea: module_ss is already used in other files. Are you sure there > > isn't an identifier naming conflict? > > Nope. Tried s/module_ss/kraxel_ss/, still not working. > > I get tons of "undefined reference to `_TRACE_something'" > errors (*not* qxl). Seems trace_ss is not updated as intended. Okay. There is a workaround available: [PATCH v2] trace: use STAP_SDT_V2 to work around symbol visibility We can take time to figure out how to extend the build system to handle modules. Would you like me to try to debug this? I'm also on #qemu IRC if you want to discuss. Stefan
Hi, > > Nope. Tried s/module_ss/kraxel_ss/, still not working. > > > > I get tons of "undefined reference to `_TRACE_something'" > > errors (*not* qxl). Seems trace_ss is not updated as intended. > > Okay. There is a workaround available: > [PATCH v2] trace: use STAP_SDT_V2 to work around symbol visibility > > We can take time to figure out how to extend the build system to handle > modules. IMHO it isn't 5.2 material anyway. > Would you like me to try to debug this? Guess I'll go dig into the meson documentation, this looks more like a build system than a tracing problem to me. Nevertheless any ideas/hints are welcome of course. take care, Gerd
Hi, > Guess I'll go dig into the meson documentation, this looks more like a > build system than a tracing problem to me. Looking at https://mesonbuild.com/Syntax.html ... "all objects are immutable". So "var2 = var1" creates a copy not a reference I guess? Which implies that ... foo_ss.add(something) ... is different from ... bar_ss = foo_ss bar_ss.add(something) ... which in turn explains why the patch doesn't work at all. Now I'm wondering how I can make trace/meson.build add the trace objects to the module source sets if I can't pass around references to the module source sets? Paolo? Any hints how to tackle this the meson way? thanks, Gerd
On 11/20/20 12:25 PM, Gerd Hoffmann wrote: > Hi, > >> Guess I'll go dig into the meson documentation, this looks more like a >> build system than a tracing problem to me. > > Looking at https://mesonbuild.com/Syntax.html ... > > "all objects are immutable". > > So "var2 = var1" creates a copy not a reference I guess? > > Which implies that ... > > foo_ss.add(something) > > ... is different from ... > > bar_ss = foo_ss > bar_ss.add(something) > > ... which in turn explains why the patch doesn't work at all. > > Now I'm wondering how I can make trace/meson.build add the trace > objects to the module source sets if I can't pass around references > to the module source sets? > > Paolo? Any hints how to tackle this the meson way? Maybe managing it all in the main meson.build, like the e28ab096bf8..da33fc09873 cleanup? ("Move the creation of the library to the main meson.build") > > thanks, > Gerd > >
On 20/11/20 12:25, Gerd Hoffmann wrote: > Hi, > >> Guess I'll go dig into the meson documentation, this looks more like a >> build system than a tracing problem to me. > > Looking at https://mesonbuild.com/Syntax.html ... > > "all objects are immutable". > > So "var2 = var1" creates a copy not a reference I guess? > > Which implies that ... > > foo_ss.add(something) > > ... is different from ... > > bar_ss = foo_ss > bar_ss.add(something) > > ... which in turn explains why the patch doesn't work at all. > > Now I'm wondering how I can make trace/meson.build add the trace > objects to the module source sets if I can't pass around references > to the module source sets? > > Paolo? Any hints how to tackle this the meson way? You could build a separate dictionary in trace/meson.build. Instead of using the 'hw_display_qxl' group, you use the module name i.e. 'hw-display-qxl'. trace/meson.build does: module_trace = {} and in trace/meson.build module_trace_src = [] foreach c : ... ... group = '--group=' + c['name'].underscorify() module_trace_src += [trace_h, trace_c] ... module_trace += { c['name']: module_trace_src } endforeach Then when building the shared_module you add the trace files to the sources, like module_trace_src = module_trace.get(d + '-' + m, []) sl = static_library(d + '-' + m, [genh, module_ss.sources(), module_trace_src], dependencies: ...) Paolo
On 20/11/20 13:30, Philippe Mathieu-Daudé wrote: > Maybe managing it all in the main meson.build, like the > e28ab096bf8..da33fc09873 cleanup? > ("Move the creation of the library to the main meson.build") That was a different issue due to variables being defined in many different meson.build files. But I think in this case the ordering is not an issue, and that's actually because of your patch. :) Paolo
> > So "var2 = var1" creates a copy not a reference I guess? > > > > Which implies that ... > > > > foo_ss.add(something) > > > > ... is different from ... > > > > bar_ss = foo_ss > > bar_ss.add(something) > > > > ... which in turn explains why the patch doesn't work at all. > > > > Now I'm wondering how I can make trace/meson.build add the trace > > objects to the module source sets if I can't pass around references > > to the module source sets? > > > > Paolo? Any hints how to tackle this the meson way? > > Maybe managing it all in the main meson.build, like the > e28ab096bf8..da33fc09873 cleanup? > ("Move the creation of the library to the main meson.build") I don't see how that'll help. The fundamental idea is hw/*/meson.build stores the source set in the new trace_events_config, then trace/meson.build can pick it up there when it loops over the trace_events_config array, adding the generated trace source files to the correct source set. Whenever that loop is in trace/meson.build or the toplevel meson.build doesn't make much of a difference I think. I think the problem is meson stores a copy not a reference so the trace_events_config loop doesn't update the original source set. When meson loops over the module source sets it doesn't see the updates because those changes where done on a throw-away copy. I don't see an easy way out :( take care, Gerd
> You could build a separate dictionary in trace/meson.build. Instead of > using the 'hw_display_qxl' group, you use the module name i.e. > 'hw-display-qxl'. trace/meson.build does: > > module_trace = {} > > and in trace/meson.build > > module_trace_src = [] > foreach c : ... > ... > group = '--group=' + c['name'].underscorify() > module_trace_src += [trace_h, trace_c] > ... > module_trace += { c['name']: module_trace_src } > endforeach > > Then when building the shared_module you add the trace files to the sources, > like > > module_trace_src = module_trace.get(d + '-' + m, []) > sl = static_library(d + '-' + m, > [genh, module_ss.sources(), module_trace_src], > dependencies: ...) So basically keep track of the objects separately. Got that working for the modular builds. Progress!!! Non-modular builds fail due to missing qxl tracepoints. Tried to fix that with a simple 'softmmu_ss.add(module_trace_src)'. Now I get: ../../meson.build:1802:2: ERROR: Object extraction arguments must be strings or Files. /me looks surprised. Doing trace_ss.add(module_trace_src) in trace/meson.buikd works just fine ... Branch available at git://git.kraxel.org/qemu sirius/trace-modules Running "qemu -device qxl" segfaults. Not investigated yet, but I guess this is just modular tracepoint not being properly initialized. Will check later, have to run pick up my daughter now. take care, Gerd
On 23/11/20 12:38, Gerd Hoffmann wrote: > So basically keep track of the objects separately. Got that working > for the modular builds. Progress!!! > > Non-modular builds fail due to missing qxl tracepoints. Tried to fix > that with a simple 'softmmu_ss.add(module_trace_src)'. Mentioned in https://wiki.qemu.org/Features/Meson#Pending_Meson_changes as "extract_objects does not support generated file (not needed yet but could be surprising)". Just use util_ss instead, confirming both points in the parentheses. Paolo
diff --git a/hw/display/meson.build b/hw/display/meson.build index c2fc36e19d3e..94e355ac1e81 100644 --- a/hw/display/meson.build +++ b/hw/display/meson.build @@ -46,6 +46,7 @@ if config_all_devices.has_key('CONFIG_QXL') trace_events_config += { 'file' : meson.source_root() / 'hw' / 'display' / 'trace-events-qxl', 'group' : 'hw_display_qxl', + 'ss' : qxl_ss, } hw_display_modules += {'qxl': qxl_ss} endif diff --git a/trace/meson.build b/trace/meson.build index 66395d3e2ba7..3f0fe7b7b74c 100644 --- a/trace/meson.build +++ b/trace/meson.build @@ -18,6 +18,7 @@ foreach c : trace_events_config trace_events_files += [ trace_events_file ] group = '--group=' + c.get('group') fmt = '@0@-' + c.get('group') + '.@1@' + module_ss = c.get('ss', trace_ss) trace_h = custom_target(fmt.format('trace', 'h'), output: fmt.format('trace', 'h'), @@ -36,10 +37,10 @@ foreach c : trace_events_config input: trace_events_file, command: [ tracetool, group, '--format=ust-events-h', '@INPUT@' ], capture: true) - trace_ss.add(trace_ust_h, lttng, urcubp) + module_ss.add(trace_ust_h, lttng, urcubp) genh += trace_ust_h endif - trace_ss.add(trace_h, trace_c) + module_ss.add(trace_h, trace_c) if 'CONFIG_TRACE_DTRACE' in config_host trace_dtrace = custom_target(fmt.format('trace-dtrace', 'dtrace'), output: fmt.format('trace-dtrace', 'dtrace'), @@ -50,13 +51,13 @@ foreach c : trace_events_config output: fmt.format('trace-dtrace', 'h'), input: trace_dtrace, command: [ 'dtrace', '-o', '@OUTPUT@', '-h', '-s', '@INPUT@' ]) - trace_ss.add(trace_dtrace_h) + module_ss.add(trace_dtrace_h) if host_machine.system() != 'darwin' trace_dtrace_o = custom_target(fmt.format('trace-dtrace', 'o'), output: fmt.format('trace-dtrace', 'o'), input: trace_dtrace, command: [ 'dtrace', '-o', '@OUTPUT@', '-G', '-s', '@INPUT@' ]) - trace_ss.add(trace_dtrace_o) + module_ss.add(trace_dtrace_o) endif genh += trace_dtrace_h
Add source set to trace_events_config, use it in trace/meson.build so the trace objects are linked to the module not core qemu. Not working as intended. /me looks puzzled. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/display/meson.build | 1 + trace/meson.build | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-)