| Message ID | 4712D8F4B26E034E80552F30A67BE0B1A3D97E@ORSMSX112.amr.corp.intel.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Mar 24, 2017 at 04:28:32PM +0000, Xu, Anthony wrote: > Create libqemutrace.a for all trace.o > Currently all trace.o are linked into qemu-system, qemu-img, > qemu-nbd, qemu-io etc., even the corresponding components > are not included. > Create a libqemutrace.a that the linker would only pull in .o > files containing symbols that are actually referenced by the > program. > ./trace.o, ./qapi/trace.o and ./util/trace.o are added into > libqemuutil.a to avoid recursive dependencies between > libqemuutil.a and libqemutrace.a. Why would libqemutrace.a depend on libqemuutil.a? Tracing code shouldn't call other QEMU code. That would could create infinite recursion when a trace event is fired.
> > ./trace.o, ./qapi/trace.o and ./util/trace.o are added into > > libqemuutil.a to avoid recursive dependencies between > > libqemuutil.a and libqemutrace.a. > > Why would libqemutrace.a depend on libqemuutil.a? Each trace.c calls trace_event_register_group to register events, trace_event_register_group is defined in trace/control.c , which is linked into libqemuutil.a. > Tracing code shouldn't call other QEMU code. That would could create > infinite recursion when a trace event is fired. If all trace.o needed by libqemuutil.a are linked into libqemuutil.a, libqemuutil.a will not depend on libqemutrace.a. This is what this patch take to break the infinite recursion. Or we can link trace/*.o to libqemutrace.a, hope it breaks the infinite recursion. But trace/*.o may still depend on libqemuutil.a Or we can just link all trace.o to libqemuutil.a. Anthony
On 27/03/2017 20:21, Xu, Anthony wrote: >>> ./trace.o, ./qapi/trace.o and ./util/trace.o are added into >>> libqemuutil.a to avoid recursive dependencies between >>> libqemuutil.a and libqemutrace.a. >> Why would libqemutrace.a depend on libqemuutil.a? > Each trace.c calls trace_event_register_group to register events, > trace_event_register_group is defined in trace/control.c , which > is linked into libqemuutil.a. Ah: util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o util-obj-y += control.o util-obj-y += qmp.o With the introduction of libqemutrace.a, I believe these should be moved into libqemutrace.a. Paolo > >> Tracing code shouldn't call other QEMU code. That would could create >> infinite recursion when a trace event is fired. > If all trace.o needed by libqemuutil.a are linked into libqemuutil.a, > libqemuutil.a will not depend on libqemutrace.a. This is what this patch > take to break the infinite recursion. > > Or we can link trace/*.o to libqemutrace.a, hope it breaks the infinite > recursion. But trace/*.o may still depend on libqemuutil.a > > Or we can just link all trace.o to libqemuutil.a.
> >>> ./trace.o, ./qapi/trace.o and ./util/trace.o are added into > >>> libqemuutil.a to avoid recursive dependencies between > >>> libqemuutil.a and libqemutrace.a. > >> Why would libqemutrace.a depend on libqemuutil.a? > > Each trace.c calls trace_event_register_group to register events, > > trace_event_register_group is defined in trace/control.c , which > > is linked into libqemuutil.a. > > Ah: > > util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o > util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o > util-obj-y += control.o > util-obj-y += qmp.o > > With the introduction of libqemutrace.a, I believe these should be moved > into libqemutrace.a. Agreed, But it doesn't solve infinite recursion issue. register_module_init is needed by libqemutrace.a, which is defined util/module.c. it is hard to remove libqemutrace.a dependency on libqemuutil.a. Removing libqemuutil.a dependency on libqemutrace.a is feasible. Just like what I did in this patch, include all util related trace.o to libqemuutila. The other simple way is to include all trace.o into libqemuutil.a What's your opinion? Thanks Anthony
On Tue, Mar 28, 2017 at 07:00:34PM +0000, Xu, Anthony wrote: > > >>> ./trace.o, ./qapi/trace.o and ./util/trace.o are added into > > >>> libqemuutil.a to avoid recursive dependencies between > > >>> libqemuutil.a and libqemutrace.a. > > >> Why would libqemutrace.a depend on libqemuutil.a? > > > Each trace.c calls trace_event_register_group to register events, > > > trace_event_register_group is defined in trace/control.c , which > > > is linked into libqemuutil.a. > > > > Ah: > > > > util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o > > util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o > > util-obj-y += control.o > > util-obj-y += qmp.o > > > > With the introduction of libqemutrace.a, I believe these should be moved > > into libqemutrace.a. > > Agreed, > But it doesn't solve infinite recursion issue. register_module_init is > needed by libqemutrace.a, which is defined util/module.c. > > it is hard to remove libqemutrace.a dependency on libqemuutil.a. > > Removing libqemuutil.a dependency on libqemutrace.a is feasible. > Just like what I did in this patch, include all util related trace.o > to libqemuutila. > > The other simple way is to include all trace.o into libqemuutil.a Yeah, I'd suggest doing that - i don't see any benefit in having them in a separate libqemutrace.a Regards, Daniel
On Tue, Mar 28, 2017 at 07:00:34PM +0000, Xu, Anthony wrote: > > >>> ./trace.o, ./qapi/trace.o and ./util/trace.o are added into > > >>> libqemuutil.a to avoid recursive dependencies between > > >>> libqemuutil.a and libqemutrace.a. > > >> Why would libqemutrace.a depend on libqemuutil.a? > > > Each trace.c calls trace_event_register_group to register events, > > > trace_event_register_group is defined in trace/control.c , which > > > is linked into libqemuutil.a. > > > > Ah: > > > > util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o > > util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o > > util-obj-y += control.o > > util-obj-y += qmp.o > > > > With the introduction of libqemutrace.a, I believe these should be moved > > into libqemutrace.a. > > Agreed, > But it doesn't solve infinite recursion issue. register_module_init is > needed by libqemutrace.a, which is defined util/module.c. > > it is hard to remove libqemutrace.a dependency on libqemuutil.a. > > Removing libqemuutil.a dependency on libqemutrace.a is feasible. > Just like what I did in this patch, include all util related trace.o > to libqemuutila. > > The other simple way is to include all trace.o into libqemuutil.a > > What's your opinion? There's no harm in adding all trace objects into libqemuutil.a. I'd go that route. Stefan
diff --git a/Makefile b/Makefile index 6c359b2..565f5c7 100644 --- a/Makefile +++ b/Makefile @@ -345,8 +345,8 @@ subdir-dtc:dtc/libfdt dtc/tests dtc/%: mkdir -p $@ -$(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y) $(chardev-obj-y) \ - $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY)) $(trace-obj-y) +$(SUBDIR_RULES): libqemutrace.a libqemuutil.a libqemustub.a $(common-obj-y) $(chardev-obj-y) \ + $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY)) ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS)) # Only keep -O and -g cflags @@ -367,10 +367,11 @@ Makefile: $(version-obj-y) libqemustub.a: $(stub-obj-y) libqemuutil.a: $(util-obj-y) +libqemutrace.a: $(trace-obj-y) ###################################################################### -COMMON_LDADDS = $(trace-obj-y) libqemuutil.a libqemustub.a +COMMON_LDADDS = libqemutrace.a libqemuutil.a libqemustub.a qemu-img.o: qemu-img-cmds.h diff --git a/Makefile.objs b/Makefile.objs index 6167e7b..4289ef9 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -3,6 +3,7 @@ stub-obj-y = stubs/ crypto/ util-obj-y = util/ qobject/ qapi/ util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o +util-obj-y += $(trace-root-obj-y) chardev-obj-y = chardev/ @@ -167,8 +168,9 @@ trace-events-subdirs += qapi trace-events-files = $(SRC_PATH)/trace-events $(trace-events-subdirs:%=$(SRC_PATH)/%/trace-events) -trace-obj-y = trace-root.o +trace-root-obj-y = trace-root.o +trace-root-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o +trace-obj-y = $(trace-root-obj-y) trace-obj-y += $(trace-events-subdirs:%=%/trace.o) trace-obj-$(CONFIG_TRACE_UST) += trace-ust-all.o -trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o trace-obj-$(CONFIG_TRACE_DTRACE) += $(trace-events-subdirs:%=%/trace-dtrace.o) diff --git a/Makefile.target b/Makefile.target index 7df2b8c..6f508c8 100644 --- a/Makefile.target +++ b/Makefile.target @@ -201,7 +201,7 @@ all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y) $(QEMU_PROG_BUILD): config-devices.mak -COMMON_LDADDS = $(trace-obj-y) ../libqemuutil.a ../libqemustub.a +COMMON_LDADDS = ../libqemutrace.a ../libqemuutil.a ../libqemustub.a # build either PROG or PROGW $(QEMU_PROG_BUILD): $(all-obj-y) $(COMMON_LDADDS) diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs index 33906ff..d543d56 100644 --- a/qapi/Makefile.objs +++ b/qapi/Makefile.objs @@ -4,3 +4,5 @@ util-obj-y += string-input-visitor.o string-output-visitor.o util-obj-y += opts-visitor.o qapi-clone-visitor.o util-obj-y += qmp-event.o util-obj-y += qapi-util.o +util-obj-y += trace.o +util-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace.o diff --git a/tests/Makefile.include b/tests/Makefile.include index f3de81f..6cbd602 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -519,7 +519,7 @@ QEMU_CFLAGS += -I$(SRC_PATH)/tests # Deps that are common to various different sets of tests below -test-util-obj-y = $(trace-obj-y) libqemuutil.a libqemustub.a +test-util-obj-y = libqemutrace.a libqemuutil.a libqemustub.a test-qom-obj-y = $(qom-obj-y) $(test-util-obj-y) test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \ tests/test-qapi-event.o tests/test-qmp-introspect.o \ diff --git a/util/Makefile.objs b/util/Makefile.objs index c6205eb..e38b91e 100644 --- a/util/Makefile.objs +++ b/util/Makefile.objs @@ -43,3 +43,5 @@ util-obj-y += qdist.o util-obj-y += qht.o util-obj-y += range.o util-obj-y += systemd.o +util-obj-y += trace.o +util-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace.o