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.

Signed-off -by: Anthony Xu <anthony.xu@intel.com>


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
                                                           

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
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

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