perf build: Fix LIBTRACEEVENT_DYNAMIC

Message ID 20221116224631.207631-1-irogers@google.com
State New
Headers
Series perf build: Fix LIBTRACEEVENT_DYNAMIC |

Commit Message

Ian Rogers Nov. 16, 2022, 10:46 p.m. UTC
  The tools/lib includes fixes break LIBTRACEVENT_DYNAMIC as the
makefile erroneously had depdendencies on building libtraceevent even
when not linking with it. This change fixes the issues with
LIBTRACEEVENT_DYNAMIC by making the built files optional.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Makefile.perf | 25 ++++++++++++++++++++++---
 tools/perf/util/setup.py |  3 ++-
 2 files changed, 24 insertions(+), 4 deletions(-)
  

Comments

Arnaldo Carvalho de Melo Nov. 17, 2022, 4:21 p.m. UTC | #1
Em Wed, Nov 16, 2022 at 02:46:31PM -0800, Ian Rogers escreveu:
> The tools/lib includes fixes break LIBTRACEVENT_DYNAMIC as the
> makefile erroneously had depdendencies on building libtraceevent even
> when not linking with it. This change fixes the issues with
> LIBTRACEEVENT_DYNAMIC by making the built files optional.

I had noticed this but didn't come around to fix it, thanks!

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/Makefile.perf | 25 ++++++++++++++++++++++---
>  tools/perf/util/setup.py |  3 ++-
>  2 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index dd096aba4430..59c4e8f9b014 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -242,8 +242,10 @@ sub-make: fixdep
>  else # force_fixdep
>  
>  LIBAPI_DIR      = $(srctree)/tools/lib/api/
> +ifndef LIBTRACEEVENT_DYNAMIC
>  LIBTRACEEVENT_DIR = $(srctree)/tools/lib/traceevent/
>  LIBTRACEEVENT_PLUGINS_DIR = $(LIBTRACEEVENT_DIR)/plugins
> +endif
>  LIBBPF_DIR      = $(srctree)/tools/lib/bpf/
>  LIBSUBCMD_DIR   = $(srctree)/tools/lib/subcmd/
>  LIBSYMBOL_DIR   = $(srctree)/tools/lib/symbol/
> @@ -293,6 +295,7 @@ SCRIPT_SH += perf-iostat.sh
>  grep-libs = $(filter -l%,$(1))
>  strip-libs = $(filter-out -l%,$(1))
>  
> +ifndef LIBTRACEEVENT_DYNAMIC
>  ifneq ($(OUTPUT),)
>    LIBTRACEEVENT_OUTPUT = $(abspath $(OUTPUT))/libtraceevent
>  else
> @@ -306,13 +309,16 @@ LIBTRACEEVENT = $(LIBTRACEEVENT_OUTPUT)/libtraceevent.a
>  export LIBTRACEEVENT
>  LIBTRACEEVENT_DYNAMIC_LIST = $(LIBTRACEEVENT_PLUGINS_OUTPUT)/libtraceevent-dynamic-list
>  CFLAGS += -I$(LIBTRACEEVENT_OUTPUT)/include
> -
>  #
>  # The static build has no dynsym table, so this does not work for
>  # static build. Looks like linker starts to scream about that now
>  # (in Fedora 26) so we need to switch it off for static build.
>  DYNAMIC_LIST_LDFLAGS               = -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
>  LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS = $(if $(findstring -static,$(LDFLAGS)),,$(DYNAMIC_LIST_LDFLAGS))
> +else
> +LIBTRACEEVENT_DYNAMIC_LIST =
> +LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS =
> +endif
>  
>  ifneq ($(OUTPUT),)
>    LIBAPI_OUTPUT = $(abspath $(OUTPUT))/libapi
> @@ -375,7 +381,11 @@ export PYTHON_EXTBUILD_LIB PYTHON_EXTBUILD_TMP
>  python-clean := $(call QUIET_CLEAN, python) $(RM) -r $(PYTHON_EXTBUILD) $(OUTPUT)python/perf*.so
>  
>  PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
> +ifndef LIBTRACEEVENT_DYNAMIC
>  PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBTRACEEVENT) $(LIBAPI)
> +else
> +PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBAPI)
> +endif
>  
>  SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH))
>  
> @@ -785,9 +795,12 @@ prepare: $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h archheaders $(drm_ioc
>  	$(LIBPERF) \
>  	$(LIBSUBCMD) \
>  	$(LIBSYMBOL) \
> -	$(LIBTRACEEVENT) \
>  	bpf-skel
>  
> +ifndef LIBTRACEEVENT_DYNAMIC
> +prepare: $(LIBTRACEEVENT)
> +endif
> +
>  $(OUTPUT)%.o: %.c prepare FORCE
>  	$(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=$(build-dir) $@
>  
> @@ -843,6 +856,7 @@ endif
>  
>  $(patsubst perf-%,%.o,$(PROGRAMS)): $(wildcard */*.h)
>  
> +ifndef LIBTRACEEVENT_DYNAMIC
>  LIBTRACEEVENT_FLAGS += plugin_dir=$(plugindir_SQ) 'EXTRA_CFLAGS=$(EXTRA_CFLAGS)' 'LDFLAGS=$(filter-out -static,$(LDFLAGS))'
>  
>  $(LIBTRACEEVENT): FORCE | $(LIBTRACEEVENT_OUTPUT)
> @@ -872,6 +886,7 @@ install-traceevent-plugins: libtraceevent_plugins
>  	$(Q)$(MAKE) -C $(LIBTRACEEVENT_PLUGINS_DIR) O=$(LIBTRACEEVENT_PLUGINS_OUTPUT) \
>  		DESTDIR=$(LIBTRACEEVENT_PLUGINS_DESTDIR) prefix= \
>  		$(LIBTRACEEVENT_FLAGS) install
> +endif
>  
>  $(LIBAPI): FORCE | $(LIBAPI_OUTPUT)
>  	$(Q)$(MAKE) -C $(LIBAPI_DIR) O=$(LIBAPI_OUTPUT) \
> @@ -1152,7 +1167,7 @@ endif # BUILD_BPF_SKEL
>  bpf-skel-clean:
>  	$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
>  
> -clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean libtraceevent_plugins-clean
> +clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean
>  	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
>  	$(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
>  	$(Q)$(RM) $(OUTPUT).config-detected
> @@ -1192,6 +1207,10 @@ clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clea
>  	$(call QUIET_CLEAN, Documentation) \
>  	$(MAKE) -C $(DOC_DIR) O=$(OUTPUT) clean >/dev/null
>  
> +ifndef LIBTRACEEVENT_DYNAMIC
> +clean:: $(LIBTRACEEVENT)-clean libtraceevent_plugins-clean
> +endif
> +
>  #
>  # To provide FEATURE-DUMP into $(FEATURE_DUMP_COPY)
>  # file if defined, with no further action.
> diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
> index 5b1e6468d5e8..43e7ca40b2ec 100644
> --- a/tools/perf/util/setup.py
> +++ b/tools/perf/util/setup.py
> @@ -77,7 +77,8 @@ perf = Extension('perf',
>  		  include_dirs = ['util/include'],
>  		  libraries = extra_libraries,
>  		  extra_compile_args = cflags,
> -		  extra_objects = [libtraceevent, libapikfs, libperf],
> +		  extra_objects = [ x for x in [libtraceevent, libapikfs, libperf]
> +                                    if x is not None],
>                   )
>  
>  setup(name='perf',
> -- 
> 2.38.1.431.g37b22c650d-goog
  
Alexander Gordeev Nov. 25, 2022, 10:41 a.m. UTC | #2
On Wed, Nov 16, 2022 at 02:46:31PM -0800, Ian Rogers wrote:

Hi Ian,

> The tools/lib includes fixes break LIBTRACEVENT_DYNAMIC as the
> makefile erroneously had depdendencies on building libtraceevent even
> when not linking with it. This change fixes the issues with
> LIBTRACEEVENT_DYNAMIC by making the built files optional.

It kernel-next on s390 somehow libtraceevent seems to be bypassed, which leads 
to make -C tools/perf install-bin failure. While it does not hit in every
envoronment, could you please hint what could be a reason for that?

...
make FIXDEP=1 -f Makefile.perf install-bin
Makefile.config:1030: No numa.h found, disables 'perf bench numa mem' benchmark, please install numactl-devel/libnuma-devel/libnuma-dev
Auto-detecting system features:
...                                   dwarf: [ on  ]
...                      dwarf_getlocations: [ on  ]
...                                   glibc: [ on  ]
...                                  libbfd: [ on  ]
...                          libbfd-buildid: [ on  ]
...                                  libcap: [ on  ]
...                                  libelf: [ on  ]
...                                 libnuma: [ OFF ]
...                  numa_num_possible_cpus: [ OFF ]
...                                 libperl: [ on  ]
...                               libpython: [ on  ]
...                               libcrypto: [ on  ]
...                               libunwind: [ OFF ]
...                      libdw-dwarf-unwind: [ on  ]
...                                    zlib: [ on  ]
...                                    lzma: [ on  ]
...                               get_cpuid: [ OFF ]
...                                     bpf: [ on  ]
...                                  libaio: [ on  ]
...                                 libzstd: [ on  ]
$(:)
make -C /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/lib/api/ O=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libapi \
 DESTDIR=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libapi prefix= \
 /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libapi/libapi.a install_headers
make -C /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/lib/perf/ O=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libperf \
 DESTDIR=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libperf prefix= \
 /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libperf/libperf.a install_headers
make -C /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/lib/subcmd/ O=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libsubcmd \
 DESTDIR=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libsubcmd prefix= \
 /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libsubcmd/libsubcmd.a install_headers
make -C /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/lib/symbol/ O=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libsymbol \
 DESTDIR=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libsymbol prefix= \
 /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libsymbol/libsymbol.a install_headers
make -C /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/lib/bpf/ FEATURES_DUMP=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/FEATURE-DUMP \
 O= OUTPUT=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libbpf/ DESTDIR=/builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libbpf prefix= \
 /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/libbpf/libbpf.a install_headers
/bin/sh util/PERF-VERSION-GEN
make -C /builddir/build/BUILD/kernel-6.0.fc36/linux-6.0/tools/perf/tests/shell/coresight
make[2]: *** No rule to make target 'install-traceevent-plugins', needed by 'install-bin'.  Stop.


Thanks!
  
Alexander Gordeev Nov. 25, 2022, 1:31 p.m. UTC | #3
On Fri, Nov 25, 2022 at 11:41:40AM +0100, Alexander Gordeev wrote:
> > The tools/lib includes fixes break LIBTRACEVENT_DYNAMIC as the
> > makefile erroneously had depdendencies on building libtraceevent even
> > when not linking with it. This change fixes the issues with
> > LIBTRACEEVENT_DYNAMIC by making the built files optional.
> 
> It kernel-next on s390 somehow libtraceevent seems to be bypassed, which leads 
> to make -C tools/perf install-bin failure. While it does not hit in every
> envoronment, could you please hint what could be a reason for that?

And unless I am missing something install-traceevent-plugins target
is only awailable when LIBTRACEEVENT_DYNAMIC is undefined..
  
Ian Rogers Nov. 29, 2022, 7:33 p.m. UTC | #4
On Fri, Nov 25, 2022 at 5:31 AM Alexander Gordeev
<agordeev@linux.ibm.com> wrote:
>
> On Fri, Nov 25, 2022 at 11:41:40AM +0100, Alexander Gordeev wrote:
> > > The tools/lib includes fixes break LIBTRACEVENT_DYNAMIC as the
> > > makefile erroneously had depdendencies on building libtraceevent even
> > > when not linking with it. This change fixes the issues with
> > > LIBTRACEEVENT_DYNAMIC by making the built files optional.
> >
> > It kernel-next on s390 somehow libtraceevent seems to be bypassed, which leads
> > to make -C tools/perf install-bin failure. While it does not hit in every
> > envoronment, could you please hint what could be a reason for that?
>
> And unless I am missing something install-traceevent-plugins target
> is only awailable when LIBTRACEEVENT_DYNAMIC is undefined..

Thanks Alexander, I'm guessing you've hit this in your builds as
LIBTRACEEVENT_DYNAMIC is defined. I've set out a fix in:
https://lore.kernel.org/lkml/20221129192924.1580537-1-irogers@google.com/T/#u

Thanks,
Ian
  

Patch

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index dd096aba4430..59c4e8f9b014 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -242,8 +242,10 @@  sub-make: fixdep
 else # force_fixdep
 
 LIBAPI_DIR      = $(srctree)/tools/lib/api/
+ifndef LIBTRACEEVENT_DYNAMIC
 LIBTRACEEVENT_DIR = $(srctree)/tools/lib/traceevent/
 LIBTRACEEVENT_PLUGINS_DIR = $(LIBTRACEEVENT_DIR)/plugins
+endif
 LIBBPF_DIR      = $(srctree)/tools/lib/bpf/
 LIBSUBCMD_DIR   = $(srctree)/tools/lib/subcmd/
 LIBSYMBOL_DIR   = $(srctree)/tools/lib/symbol/
@@ -293,6 +295,7 @@  SCRIPT_SH += perf-iostat.sh
 grep-libs = $(filter -l%,$(1))
 strip-libs = $(filter-out -l%,$(1))
 
+ifndef LIBTRACEEVENT_DYNAMIC
 ifneq ($(OUTPUT),)
   LIBTRACEEVENT_OUTPUT = $(abspath $(OUTPUT))/libtraceevent
 else
@@ -306,13 +309,16 @@  LIBTRACEEVENT = $(LIBTRACEEVENT_OUTPUT)/libtraceevent.a
 export LIBTRACEEVENT
 LIBTRACEEVENT_DYNAMIC_LIST = $(LIBTRACEEVENT_PLUGINS_OUTPUT)/libtraceevent-dynamic-list
 CFLAGS += -I$(LIBTRACEEVENT_OUTPUT)/include
-
 #
 # The static build has no dynsym table, so this does not work for
 # static build. Looks like linker starts to scream about that now
 # (in Fedora 26) so we need to switch it off for static build.
 DYNAMIC_LIST_LDFLAGS               = -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
 LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS = $(if $(findstring -static,$(LDFLAGS)),,$(DYNAMIC_LIST_LDFLAGS))
+else
+LIBTRACEEVENT_DYNAMIC_LIST =
+LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS =
+endif
 
 ifneq ($(OUTPUT),)
   LIBAPI_OUTPUT = $(abspath $(OUTPUT))/libapi
@@ -375,7 +381,11 @@  export PYTHON_EXTBUILD_LIB PYTHON_EXTBUILD_TMP
 python-clean := $(call QUIET_CLEAN, python) $(RM) -r $(PYTHON_EXTBUILD) $(OUTPUT)python/perf*.so
 
 PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
+ifndef LIBTRACEEVENT_DYNAMIC
 PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBTRACEEVENT) $(LIBAPI)
+else
+PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBAPI)
+endif
 
 SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH))
 
@@ -785,9 +795,12 @@  prepare: $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h archheaders $(drm_ioc
 	$(LIBPERF) \
 	$(LIBSUBCMD) \
 	$(LIBSYMBOL) \
-	$(LIBTRACEEVENT) \
 	bpf-skel
 
+ifndef LIBTRACEEVENT_DYNAMIC
+prepare: $(LIBTRACEEVENT)
+endif
+
 $(OUTPUT)%.o: %.c prepare FORCE
 	$(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=$(build-dir) $@
 
@@ -843,6 +856,7 @@  endif
 
 $(patsubst perf-%,%.o,$(PROGRAMS)): $(wildcard */*.h)
 
+ifndef LIBTRACEEVENT_DYNAMIC
 LIBTRACEEVENT_FLAGS += plugin_dir=$(plugindir_SQ) 'EXTRA_CFLAGS=$(EXTRA_CFLAGS)' 'LDFLAGS=$(filter-out -static,$(LDFLAGS))'
 
 $(LIBTRACEEVENT): FORCE | $(LIBTRACEEVENT_OUTPUT)
@@ -872,6 +886,7 @@  install-traceevent-plugins: libtraceevent_plugins
 	$(Q)$(MAKE) -C $(LIBTRACEEVENT_PLUGINS_DIR) O=$(LIBTRACEEVENT_PLUGINS_OUTPUT) \
 		DESTDIR=$(LIBTRACEEVENT_PLUGINS_DESTDIR) prefix= \
 		$(LIBTRACEEVENT_FLAGS) install
+endif
 
 $(LIBAPI): FORCE | $(LIBAPI_OUTPUT)
 	$(Q)$(MAKE) -C $(LIBAPI_DIR) O=$(LIBAPI_OUTPUT) \
@@ -1152,7 +1167,7 @@  endif # BUILD_BPF_SKEL
 bpf-skel-clean:
 	$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
 
-clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean libtraceevent_plugins-clean
+clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean
 	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
 	$(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
 	$(Q)$(RM) $(OUTPUT).config-detected
@@ -1192,6 +1207,10 @@  clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clea
 	$(call QUIET_CLEAN, Documentation) \
 	$(MAKE) -C $(DOC_DIR) O=$(OUTPUT) clean >/dev/null
 
+ifndef LIBTRACEEVENT_DYNAMIC
+clean:: $(LIBTRACEEVENT)-clean libtraceevent_plugins-clean
+endif
+
 #
 # To provide FEATURE-DUMP into $(FEATURE_DUMP_COPY)
 # file if defined, with no further action.
diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
index 5b1e6468d5e8..43e7ca40b2ec 100644
--- a/tools/perf/util/setup.py
+++ b/tools/perf/util/setup.py
@@ -77,7 +77,8 @@  perf = Extension('perf',
 		  include_dirs = ['util/include'],
 		  libraries = extra_libraries,
 		  extra_compile_args = cflags,
-		  extra_objects = [libtraceevent, libapikfs, libperf],
+		  extra_objects = [ x for x in [libtraceevent, libapikfs, libperf]
+                                    if x is not None],
                  )
 
 setup(name='perf',