Traceevent plugins need dynamic symbols exported from libtraceevent.a,
otherwise a dlopen error will occur during plugins loading.
This patch uses dynamic-list-file to export dynamic symbols which will
be used in plugins to perf executable.
The problem is covered up if feature-libpython is enabled, because
PYTHON_EMBED_LDOPTS contains '-Xlinker --export-dynamic' which adds all
symbols to the dynamic symbol table. So we should reproduce the problem
by setting NO_LIBPYTHON=1.
Before this patch:
(Prepare plugins)
$ ls /root/.traceevent/plugins/
plugin_sched_switch.so
plugin_function.so
...
$ perf record -e 'ftrace:function' ls
$ perf script
Warning: could not load plugin '/mnt/data/root/.traceevent/plugins/plugin_sched_switch.so'
/root/.traceevent/plugins/plugin_sched_switch.so: undefined symbol: pevent_unregister_event_handler
Warning: could not load plugin '/root/.traceevent/plugins/plugin_function.so'
/root/.traceevent/plugins/plugin_function.so: undefined symbol: warning
...
:1049 1049 [000] 9666.754487: ftrace:function: ffffffff8118bc50 <-- ffffffff8118c5b3
:1049 1049 [000] 9666.754487: ftrace:function: ffffffff818e2440 <-- ffffffff8118bc75
:1049 1049 [000] 9666.754487: ftrace:function: ffffffff8106eee0 <-- ffffffff811212e2
After this patch:
$ perf record -e 'ftrace:function' ls
$ perf script
:1049 1049 [000] 9666.754487: ftrace:function: __set_task_comm
:1049 1049 [000] 9666.754487: ftrace:function: _raw_spin_lock
:1049 1049 [000] 9666.754487: ftrace:function: task_tgid_nr_ns
...
Signed-off-by: He Kuang <[email protected]>
---
tools/lib/traceevent/Makefile | 14 +++++++++++++-
tools/perf/Makefile.perf | 15 ++++++++++++---
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index d410da3..2cbac13 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -23,6 +23,7 @@ endef
# Allow setting CC and AR, or setting CROSS_COMPILE as a prefix.
$(call allow-override,CC,$(CROSS_COMPILE)gcc)
$(call allow-override,AR,$(CROSS_COMPILE)ar)
+$(call allow-override,NM,$(CROSS_COMPILE)nm)
EXT = -std=gnu99
INSTALL = install
@@ -151,8 +152,9 @@ PLUGINS_IN := $(PLUGINS:.so=-in.o)
TE_IN := $(OUTPUT)libtraceevent-in.o
LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE))
+DYNAMIC_LIST_FILE := $(OUTPUT)libtraceevent-dynamic-list
-CMD_TARGETS = $(LIB_FILE) $(PLUGINS)
+CMD_TARGETS = $(LIB_FILE) $(PLUGINS) $(DYNAMIC_LIST_FILE)
TARGETS = $(CMD_TARGETS)
@@ -169,6 +171,9 @@ $(OUTPUT)libtraceevent.so: $(TE_IN)
$(OUTPUT)libtraceevent.a: $(TE_IN)
$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
+$(OUTPUT)libtraceevent-dynamic-list: $(PLUGINS)
+ $(QUIET_GEN)$(call do_generate_dynamic_list_file, $(PLUGINS), $@)
+
plugins: $(PLUGINS)
__plugin_obj = $(notdir $@)
@@ -238,6 +243,13 @@ define do_install_plugins
done
endef
+define do_generate_dynamic_list_file
+ (echo '{'; \
+ $(NM) -u -D $1 | awk 'NF>1 {print "\t"$$2";"}' | sort -u; \
+ echo '};'; \
+ ) > $2
+endef
+
install_lib: all_cmd install_plugins
$(call QUIET_INSTALL, $(LIB_FILE)) \
$(call do_install,$(LIB_FILE),$(bindir_SQ))
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 03409cc..338c426 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -172,6 +172,7 @@ endif
LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
export LIBTRACEEVENT
+LIBTRACEEVENT_DYNAMIC_LIST = $(TE_PATH)libtraceevent-dynamic-list
LIBAPI = $(LIB_PATH)libapi.a
export LIBAPI
@@ -278,8 +279,9 @@ build := -f $(srctree)/tools/build/Makefile.build dir=. obj
$(PERF_IN): $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h FORCE
$(Q)$(MAKE) $(build)=perf
-$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN)
- $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(PERF_IN) $(LIBS) -o $@
+LD_LIBTRACEEVENT_FLAGS += -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
+$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(LIBTRACEEVENT_DYNAMIC_LIST)
+ $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(LD_LIBTRACEEVENT_FLAGS) $(PERF_IN) $(LIBS) -o $@
$(GTK_IN): FORCE
$(Q)$(MAKE) $(build)=gtk
@@ -373,7 +375,13 @@ $(LIB_FILE): $(LIBPERF_IN)
LIBTRACEEVENT_FLAGS += plugin_dir=$(plugindir_SQ)
$(LIBTRACEEVENT): FORCE
- $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a plugins
+ $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a
+
+libtraceevent_plugins: FORCE
+ $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) plugins
+
+$(LIBTRACEEVENT_DYNAMIC_LIST): libtraceevent_plugins
+ $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent-dynamic-list
$(LIBTRACEEVENT)-clean:
$(call QUIET_CLEAN, libtraceevent)
@@ -551,4 +559,5 @@ FORCE:
.PHONY: all install clean config-clean strip install-gtk
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
.PHONY: $(GIT-HEAD-PHONY) TAGS tags cscope FORCE single_dep
+.PHONY: libtraceevent_plugins
--
1.8.5.2
The libtrace-dynamic-list file is used to export symbols used by
traceevent plugins.
Signed-off-by: He Kuang <[email protected]>
---
tools/lib/traceevent/.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/lib/traceevent/.gitignore b/tools/lib/traceevent/.gitignore
index 35f56be..3c60335 100644
--- a/tools/lib/traceevent/.gitignore
+++ b/tools/lib/traceevent/.gitignore
@@ -1 +1,2 @@
TRACEEVENT-CFLAGS
+libtraceevent-dynamic-list
--
1.8.5.2
On Tue, May 12, 2015 at 06:41:56AM +0000, He Kuang wrote:
SNIP
> $(Q)$(MAKE) $(build)=perf
>
> -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN)
> - $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(PERF_IN) $(LIBS) -o $@
> +LD_LIBTRACEEVENT_FLAGS += -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
> +$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(LIBTRACEEVENT_DYNAMIC_LIST)
> + $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(LD_LIBTRACEEVENT_FLAGS) $(PERF_IN) $(LIBS) -o $@
>
> $(GTK_IN): FORCE
> $(Q)$(MAKE) $(build)=gtk
> @@ -373,7 +375,13 @@ $(LIB_FILE): $(LIBPERF_IN)
> LIBTRACEEVENT_FLAGS += plugin_dir=$(plugindir_SQ)
>
> $(LIBTRACEEVENT): FORCE
> - $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a plugins
> + $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a
> +
> +libtraceevent_plugins: FORCE
> + $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) plugins
> +
> +$(LIBTRACEEVENT_DYNAMIC_LIST): libtraceevent_plugins
> + $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent-dynamic-list
I thought the idea of v2 was not to introduce new target,
something like in attached patch (not completely tested)
jirka
---
diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index d410da335e3d..57eed0403e6a 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -23,6 +23,7 @@ endef
# Allow setting CC and AR, or setting CROSS_COMPILE as a prefix.
$(call allow-override,CC,$(CROSS_COMPILE)gcc)
$(call allow-override,AR,$(CROSS_COMPILE)ar)
+$(call allow-override,NM,$(CROSS_COMPILE)nm)
EXT = -std=gnu99
INSTALL = install
@@ -169,7 +170,10 @@ $(OUTPUT)libtraceevent.so: $(TE_IN)
$(OUTPUT)libtraceevent.a: $(TE_IN)
$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
-plugins: $(PLUGINS)
+$(OUTPUT)libtraceevent-dynamic-list: $(PLUGINS)
+ $(QUIET_GEN)$(call do_generate_dynamic_list_file, $(PLUGINS), $@)
+
+plugins: $(OUTPUT)libtraceevent-dynamic-list
__plugin_obj = $(notdir $@)
plugin_obj = $(__plugin_obj:-in.o=)
@@ -238,6 +242,13 @@ define do_install_plugins
done
endef
+define do_generate_dynamic_list_file
+ (echo '{'; \
+ $(NM) -u -D $1 | awk 'NF>1 {print "\t"$$2";"}' | sort -u; \
+ echo '};'; \
+ ) > $2
+endef
+
install_lib: all_cmd install_plugins
$(call QUIET_INSTALL, $(LIB_FILE)) \
$(call do_install,$(LIB_FILE),$(bindir_SQ))
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 03409cc02117..a8db98649ea3 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -173,6 +173,8 @@ endif
LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
export LIBTRACEEVENT
+LDFLAGS += -Xlinker --dynamic-list=$(TE_PATH)libtraceevent-dynamic-list
+
LIBAPI = $(LIB_PATH)libapi.a
export LIBAPI
Hi, jirka
On 05/12/2015 08:37 PM, Jiri Olsa wrote:
> On Tue, May 12, 2015 at 06:41:56AM +0000, He Kuang wrote:
>
> SNIP
>
>> $(Q)$(MAKE) $(build)=perf
>>
>> -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN)
>> - $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(PERF_IN) $(LIBS) -o $@
>> +LD_LIBTRACEEVENT_FLAGS += -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
>> +$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(LIBTRACEEVENT_DYNAMIC_LIST)
>> + $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(LD_LIBTRACEEVENT_FLAGS) $(PERF_IN) $(LIBS) -o $@
>>
>> $(GTK_IN): FORCE
>> $(Q)$(MAKE) $(build)=gtk
>> @@ -373,7 +375,13 @@ $(LIB_FILE): $(LIBPERF_IN)
>> LIBTRACEEVENT_FLAGS += plugin_dir=$(plugindir_SQ)
>>
>> $(LIBTRACEEVENT): FORCE
>> - $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a plugins
>> + $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a
>> +
>> +libtraceevent_plugins: FORCE
>> + $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) plugins
>> +
>> +$(LIBTRACEEVENT_DYNAMIC_LIST): libtraceevent_plugins
>> + $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent-dynamic-list
> I thought the idea of v2 was not to introduce new target,
> something like in attached patch (not completely tested)
>
> jirka
There is a problem, the target perf executable is dependent on
the dynamic-list-file, so we should add plugins or the
dynamic-list-file to perf's dependencies.
As your patch below, the dynamic-list-file is built implictly
when building plugins, so we should not add it directly to the
dependency list of perf.
It seems new targets are needed. In the v2 patch,
dynamic-list-file is extracted out from plugins as perf's
dependncy.
>
> ---
> diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
> index d410da335e3d..57eed0403e6a 100644
> --- a/tools/lib/traceevent/Makefile
> +++ b/tools/lib/traceevent/Makefile
> @@ -23,6 +23,7 @@ endef
> # Allow setting CC and AR, or setting CROSS_COMPILE as a prefix.
> $(call allow-override,CC,$(CROSS_COMPILE)gcc)
> $(call allow-override,AR,$(CROSS_COMPILE)ar)
> +$(call allow-override,NM,$(CROSS_COMPILE)nm)
>
> EXT = -std=gnu99
> INSTALL = install
> @@ -169,7 +170,10 @@ $(OUTPUT)libtraceevent.so: $(TE_IN)
> $(OUTPUT)libtraceevent.a: $(TE_IN)
> $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
>
> -plugins: $(PLUGINS)
> +$(OUTPUT)libtraceevent-dynamic-list: $(PLUGINS)
> + $(QUIET_GEN)$(call do_generate_dynamic_list_file, $(PLUGINS), $@)
> +
> +plugins: $(OUTPUT)libtraceevent-dynamic-list
>
> __plugin_obj = $(notdir $@)
> plugin_obj = $(__plugin_obj:-in.o=)
> @@ -238,6 +242,13 @@ define do_install_plugins
> done
> endef
>
> +define do_generate_dynamic_list_file
> + (echo '{'; \
> + $(NM) -u -D $1 | awk 'NF>1 {print "\t"$$2";"}' | sort -u; \
> + echo '};'; \
> + ) > $2
> +endef
> +
> install_lib: all_cmd install_plugins
> $(call QUIET_INSTALL, $(LIB_FILE)) \
> $(call do_install,$(LIB_FILE),$(bindir_SQ))
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 03409cc02117..a8db98649ea3 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -173,6 +173,8 @@ endif
> LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
> export LIBTRACEEVENT
>
> +LDFLAGS += -Xlinker --dynamic-list=$(TE_PATH)libtraceevent-dynamic-list
> +
> LIBAPI = $(LIB_PATH)libapi.a
> export LIBAPI
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>
On Tue, May 12, 2015 at 11:25:45PM +0800, hekuang wrote:
> Hi, jirka
>
> On 05/12/2015 08:37 PM, Jiri Olsa wrote:
> >On Tue, May 12, 2015 at 06:41:56AM +0000, He Kuang wrote:
> >
> >SNIP
> >
> >> $(Q)$(MAKE) $(build)=perf
> >>-$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN)
> >>- $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(PERF_IN) $(LIBS) -o $@
> >>+LD_LIBTRACEEVENT_FLAGS += -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
> >>+$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(LIBTRACEEVENT_DYNAMIC_LIST)
> >>+ $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(LD_LIBTRACEEVENT_FLAGS) $(PERF_IN) $(LIBS) -o $@
> >> $(GTK_IN): FORCE
> >> $(Q)$(MAKE) $(build)=gtk
> >>@@ -373,7 +375,13 @@ $(LIB_FILE): $(LIBPERF_IN)
> >> LIBTRACEEVENT_FLAGS += plugin_dir=$(plugindir_SQ)
> >> $(LIBTRACEEVENT): FORCE
> >>- $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a plugins
> >>+ $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a
> >>+
> >>+libtraceevent_plugins: FORCE
> >>+ $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) plugins
> >>+
> >>+$(LIBTRACEEVENT_DYNAMIC_LIST): libtraceevent_plugins
> >>+ $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent-dynamic-list
> >I thought the idea of v2 was not to introduce new target,
> >something like in attached patch (not completely tested)
> >
> >jirka
>
> There is a problem, the target perf executable is dependent on
> the dynamic-list-file, so we should add plugins or the
> dynamic-list-file to perf's dependencies.
>
> As your patch below, the dynamic-list-file is built implictly
> when building plugins, so we should not add it directly to the
> dependency list of perf.
>
> It seems new targets are needed. In the v2 patch,
hum, I dont get it.. why ?
dynamic-list-file gets rebuilt any time plugins are rebuilt..
why not keep just the 'plugins' dependency?
jirka
Hi, jirka
On 2015/5/13 22:50, Jiri Olsa wrote:
> On Tue, May 12, 2015 at 11:25:45PM +0800, hekuang wrote:
>> Hi, jirka
>>
>> On 05/12/2015 08:37 PM, Jiri Olsa wrote:
>>> On Tue, May 12, 2015 at 06:41:56AM +0000, He Kuang wrote:
>>>
>>> SNIP
>>>
>>>> $(Q)$(MAKE) $(build)=perf
>>>> -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN)
>>>> - $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(PERF_IN) $(LIBS) -o $@
>>>> +LD_LIBTRACEEVENT_FLAGS += -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
>>>> +$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(LIBTRACEEVENT_DYNAMIC_LIST)
>>>> + $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(LD_LIBTRACEEVENT_FLAGS) $(PERF_IN) $(LIBS) -o $@
>>>> $(GTK_IN): FORCE
>>>> $(Q)$(MAKE) $(build)=gtk
>>>> @@ -373,7 +375,13 @@ $(LIB_FILE): $(LIBPERF_IN)
>>>> LIBTRACEEVENT_FLAGS += plugin_dir=$(plugindir_SQ)
>>>> $(LIBTRACEEVENT): FORCE
>>>> - $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a plugins
>>>> + $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a
>>>> +
>>>> +libtraceevent_plugins: FORCE
>>>> + $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) plugins
>>>> +
>>>> +$(LIBTRACEEVENT_DYNAMIC_LIST): libtraceevent_plugins
>>>> + $(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent-dynamic-list
>>> I thought the idea of v2 was not to introduce new target,
>>> something like in attached patch (not completely tested)
>>>
>>> jirka
>>
>> There is a problem, the target perf executable is dependent on
>> the dynamic-list-file, so we should add plugins or the
>> dynamic-list-file to perf's dependencies.
>>
>> As your patch below, the dynamic-list-file is built implictly
>> when building plugins, so we should not add it directly to the
>> dependency list of perf.
>>
>> It seems new targets are needed. In the v2 patch,
>
> hum, I dont get it.. why ?
>
> dynamic-list-file gets rebuilt any time plugins are rebuilt..
> why not keep just the 'plugins' dependency?
You can test your patch as following steps:
$ touch ../lib/traceevent/plugin_function.c
$ make
CC plugin_function.o
LD plugin_function-in.o
LINK plugin_function.so
GEN libtraceevent-dynamic-list
perf is not rebuilt. There should be a 'GEN perf', right?
>
> jirka
>
On Thu, May 14, 2015 at 08:56:15PM +0800, He Kuang wrote:
SNIP
> >>It seems new targets are needed. In the v2 patch,
> >
> >hum, I dont get it.. why ?
> >
> >dynamic-list-file gets rebuilt any time plugins are rebuilt..
> >why not keep just the 'plugins' dependency?
>
> You can test your patch as following steps:
>
> $ touch ../lib/traceevent/plugin_function.c
> $ make
> CC plugin_function.o
> LD plugin_function-in.o
> LINK plugin_function.so
> GEN libtraceevent-dynamic-list
>
> perf is not rebuilt. There should be a 'GEN perf', right?
hum, right.. so this is separated bug that was there even
without your change. I tried to kick my change to address
that and ended up with what you sent in v2 ;-)
I have another comment for your v2, which I'll send right away
thanks,
jirka
On Tue, May 12, 2015 at 06:41:56AM +0000, He Kuang wrote:
SNIP
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 03409cc..338c426 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -172,6 +172,7 @@ endif
>
> LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
> export LIBTRACEEVENT
> +LIBTRACEEVENT_DYNAMIC_LIST = $(TE_PATH)libtraceevent-dynamic-list
>
> LIBAPI = $(LIB_PATH)libapi.a
> export LIBAPI
> @@ -278,8 +279,9 @@ build := -f $(srctree)/tools/build/Makefile.build dir=. obj
> $(PERF_IN): $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h FORCE
> $(Q)$(MAKE) $(build)=perf
>
> -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN)
> - $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(PERF_IN) $(LIBS) -o $@
> +LD_LIBTRACEEVENT_FLAGS += -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
Could you please just update LDFLAGS directly? I dont think
there's a need for separated variable..
otherwise it seems ok to me
thanks,
jirka
On 2015/5/14 21:31, Jiri Olsa wrote:
> On Thu, May 14, 2015 at 08:56:15PM +0800, He Kuang wrote:
>
> SNIP
>
>>>> It seems new targets are needed. In the v2 patch,
>>>
>>> hum, I dont get it.. why ?
>>>
>>> dynamic-list-file gets rebuilt any time plugins are rebuilt..
>>> why not keep just the 'plugins' dependency?
>>
>> You can test your patch as following steps:
>>
>> $ touch ../lib/traceevent/plugin_function.c
>> $ make
>> CC plugin_function.o
>> LD plugin_function-in.o
>> LINK plugin_function.so
>> GEN libtraceevent-dynamic-list
>>
>> perf is not rebuilt. There should be a 'GEN perf', right?
>
> hum, right.. so this is separated bug that was there even
> without your change. I tried to kick my change to address
> that and ended up with what you sent in v2 ;-)
>
> I have another comment for your v2, which I'll send right away
>
> thanks,
> jirka
>
Ok, please review the new version.