| Message ID | 20211105020244.6869-1-quentin@isovalent.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] perf build: Install libbpf headers locally when building | expand |
On Thu, Nov 4, 2021 at 7:02 PM Quentin Monnet <quentin@isovalent.com> wrote: > > API headers from libbpf should not be accessed directly from the > library's source directory. Instead, they should be exported with "make > install_headers". Let's adjust perf's Makefile to install those headers > locally when building libbpf. > > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > --- > Note: Sending to bpf-next because it's directly related to libbpf, and > to similar patches merged through bpf-next, but maybe Arnaldo prefers to > take it? Arnaldo would know better how to thoroughly test it, so I'd prefer to route this through perf tree. Any objections, Arnaldo? > --- > tools/perf/Makefile.perf | 24 +++++++++++++----------- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf > index b856afa6eb52..3a81b6c712a9 100644 > --- a/tools/perf/Makefile.perf > +++ b/tools/perf/Makefile.perf > @@ -241,7 +241,7 @@ else # force_fixdep > > LIB_DIR = $(srctree)/tools/lib/api/ > TRACE_EVENT_DIR = $(srctree)/tools/lib/traceevent/ > -BPF_DIR = $(srctree)/tools/lib/bpf/ > +LIBBPF_DIR = $(srctree)/tools/lib/bpf/ > SUBCMD_DIR = $(srctree)/tools/lib/subcmd/ > LIBPERF_DIR = $(srctree)/tools/lib/perf/ > DOC_DIR = $(srctree)/tools/perf/Documentation/ > @@ -293,7 +293,6 @@ strip-libs = $(filter-out -l%,$(1)) > ifneq ($(OUTPUT),) > TE_PATH=$(OUTPUT) > PLUGINS_PATH=$(OUTPUT) > - BPF_PATH=$(OUTPUT) > SUBCMD_PATH=$(OUTPUT) > LIBPERF_PATH=$(OUTPUT) > ifneq ($(subdir),) > @@ -305,7 +304,6 @@ else > TE_PATH=$(TRACE_EVENT_DIR) > PLUGINS_PATH=$(TRACE_EVENT_DIR)plugins/ > API_PATH=$(LIB_DIR) > - BPF_PATH=$(BPF_DIR) > SUBCMD_PATH=$(SUBCMD_DIR) > LIBPERF_PATH=$(LIBPERF_DIR) > endif > @@ -324,7 +322,10 @@ LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS = $(if $(findstring -static,$(LDFLAGS)),,$(DY > LIBAPI = $(API_PATH)libapi.a > export LIBAPI > > -LIBBPF = $(BPF_PATH)libbpf.a > +LIBBPF_OUTPUT = $(OUTPUT)libbpf > +LIBBPF_DESTDIR = $(LIBBPF_OUTPUT) > +LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include > +LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a > > LIBSUBCMD = $(SUBCMD_PATH)libsubcmd.a > > @@ -829,12 +830,14 @@ $(LIBAPI)-clean: > $(call QUIET_CLEAN, libapi) > $(Q)$(MAKE) -C $(LIB_DIR) O=$(OUTPUT) clean >/dev/null > > -$(LIBBPF): FORCE > - $(Q)$(MAKE) -C $(BPF_DIR) O=$(OUTPUT) $(OUTPUT)libbpf.a FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) > +$(LIBBPF): FORCE | $(LIBBPF_OUTPUT) > + $(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \ > + O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \ > + $@ install_headers > > $(LIBBPF)-clean: > $(call QUIET_CLEAN, libbpf) > - $(Q)$(MAKE) -C $(BPF_DIR) O=$(OUTPUT) clean >/dev/null > + $(Q)$(RM) -r -- $(LIBBPF_OUTPUT) > > $(LIBPERF): FORCE > $(Q)$(MAKE) -C $(LIBPERF_DIR) EXTRA_CFLAGS="$(LIBPERF_CFLAGS)" O=$(OUTPUT) $(OUTPUT)libperf.a > @@ -1036,14 +1039,13 @@ SKELETONS += $(SKEL_OUT)/bperf_cgroup.skel.h > > ifdef BUILD_BPF_SKEL > BPFTOOL := $(SKEL_TMP_OUT)/bootstrap/bpftool > -LIBBPF_SRC := $(abspath ../lib/bpf) > -BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(BPF_PATH) -I$(LIBBPF_SRC)/.. > +BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(LIBBPF_INCLUDE) > > -$(SKEL_TMP_OUT): > +$(SKEL_TMP_OUT) $(LIBBPF_OUTPUT): > $(Q)$(MKDIR) -p $@ > > $(BPFTOOL): | $(SKEL_TMP_OUT) > - CFLAGS= $(MAKE) -C ../bpf/bpftool \ > + $(Q)CFLAGS= $(MAKE) -C ../bpf/bpftool \ > OUTPUT=$(SKEL_TMP_OUT)/ bootstrap > > VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \ > -- > 2.32.0 >
On November 5, 2021 3:38:50 PM GMT-03:00, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: >On Thu, Nov 4, 2021 at 7:02 PM Quentin Monnet <quentin@isovalent.com> wrote: >> >> API headers from libbpf should not be accessed directly from the >> library's source directory. Instead, they should be exported with "make >> install_headers". Let's adjust perf's Makefile to install those headers >> locally when building libbpf. >> >> Signed-off-by: Quentin Monnet <quentin@isovalent.com> >> --- >> Note: Sending to bpf-next because it's directly related to libbpf, and >> to similar patches merged through bpf-next, but maybe Arnaldo prefers to >> take it? > >Arnaldo would know better how to thoroughly test it, so I'd prefer to >route this through perf tree. Any objections, Arnaldo? Sure, I'll review and test it. - Arnaldo > >> --- >> tools/perf/Makefile.perf | 24 +++++++++++++----------- >> 1 file changed, 13 insertions(+), 11 deletions(-) >> >> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf >> index b856afa6eb52..3a81b6c712a9 100644 >> --- a/tools/perf/Makefile.perf >> +++ b/tools/perf/Makefile.perf >> @@ -241,7 +241,7 @@ else # force_fixdep >> >> LIB_DIR = $(srctree)/tools/lib/api/ >> TRACE_EVENT_DIR = $(srctree)/tools/lib/traceevent/ >> -BPF_DIR = $(srctree)/tools/lib/bpf/ >> +LIBBPF_DIR = $(srctree)/tools/lib/bpf/ >> SUBCMD_DIR = $(srctree)/tools/lib/subcmd/ >> LIBPERF_DIR = $(srctree)/tools/lib/perf/ >> DOC_DIR = $(srctree)/tools/perf/Documentation/ >> @@ -293,7 +293,6 @@ strip-libs = $(filter-out -l%,$(1)) >> ifneq ($(OUTPUT),) >> TE_PATH=$(OUTPUT) >> PLUGINS_PATH=$(OUTPUT) >> - BPF_PATH=$(OUTPUT) >> SUBCMD_PATH=$(OUTPUT) >> LIBPERF_PATH=$(OUTPUT) >> ifneq ($(subdir),) >> @@ -305,7 +304,6 @@ else >> TE_PATH=$(TRACE_EVENT_DIR) >> PLUGINS_PATH=$(TRACE_EVENT_DIR)plugins/ >> API_PATH=$(LIB_DIR) >> - BPF_PATH=$(BPF_DIR) >> SUBCMD_PATH=$(SUBCMD_DIR) >> LIBPERF_PATH=$(LIBPERF_DIR) >> endif >> @@ -324,7 +322,10 @@ LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS = $(if $(findstring -static,$(LDFLAGS)),,$(DY >> LIBAPI = $(API_PATH)libapi.a >> export LIBAPI >> >> -LIBBPF = $(BPF_PATH)libbpf.a >> +LIBBPF_OUTPUT = $(OUTPUT)libbpf >> +LIBBPF_DESTDIR = $(LIBBPF_OUTPUT) >> +LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include >> +LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a >> >> LIBSUBCMD = $(SUBCMD_PATH)libsubcmd.a >> >> @@ -829,12 +830,14 @@ $(LIBAPI)-clean: >> $(call QUIET_CLEAN, libapi) >> $(Q)$(MAKE) -C $(LIB_DIR) O=$(OUTPUT) clean >/dev/null >> >> -$(LIBBPF): FORCE >> - $(Q)$(MAKE) -C $(BPF_DIR) O=$(OUTPUT) $(OUTPUT)libbpf.a FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) >> +$(LIBBPF): FORCE | $(LIBBPF_OUTPUT) >> + $(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \ >> + O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \ >> + $@ install_headers >> >> $(LIBBPF)-clean: >> $(call QUIET_CLEAN, libbpf) >> - $(Q)$(MAKE) -C $(BPF_DIR) O=$(OUTPUT) clean >/dev/null >> + $(Q)$(RM) -r -- $(LIBBPF_OUTPUT) >> >> $(LIBPERF): FORCE >> $(Q)$(MAKE) -C $(LIBPERF_DIR) EXTRA_CFLAGS="$(LIBPERF_CFLAGS)" O=$(OUTPUT) $(OUTPUT)libperf.a >> @@ -1036,14 +1039,13 @@ SKELETONS += $(SKEL_OUT)/bperf_cgroup.skel.h >> >> ifdef BUILD_BPF_SKEL >> BPFTOOL := $(SKEL_TMP_OUT)/bootstrap/bpftool >> -LIBBPF_SRC := $(abspath ../lib/bpf) >> -BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(BPF_PATH) -I$(LIBBPF_SRC)/.. >> +BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(LIBBPF_INCLUDE) >> >> -$(SKEL_TMP_OUT): >> +$(SKEL_TMP_OUT) $(LIBBPF_OUTPUT): >> $(Q)$(MKDIR) -p $@ >> >> $(BPFTOOL): | $(SKEL_TMP_OUT) >> - CFLAGS= $(MAKE) -C ../bpf/bpftool \ >> + $(Q)CFLAGS= $(MAKE) -C ../bpf/bpftool \ >> OUTPUT=$(SKEL_TMP_OUT)/ bootstrap >> >> VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \ >> -- >> 2.32.0 >>
Em Fri, Nov 05, 2021 at 11:38:50AM -0700, Andrii Nakryiko escreveu: > On Thu, Nov 4, 2021 at 7:02 PM Quentin Monnet <quentin@isovalent.com> wrote: > > > > API headers from libbpf should not be accessed directly from the > > library's source directory. Instead, they should be exported with "make > > install_headers". Let's adjust perf's Makefile to install those headers > > locally when building libbpf. > > > > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > > --- > > Note: Sending to bpf-next because it's directly related to libbpf, and > > to similar patches merged through bpf-next, but maybe Arnaldo prefers to > > take it? > > Arnaldo would know better how to thoroughly test it, so I'd prefer to > route this through perf tree. Any objections, Arnaldo? Preliminary testing passed for 'BUILD_BPF_SKEL=1' with without LIBBPF_DYNAMIC=1 (using the system's libbpf-devel to build perf), so far so good, so I tentatively applied it, will see with the full set of containers. Thanks! - Arnaldo > > --- > > tools/perf/Makefile.perf | 24 +++++++++++++----------- > > 1 file changed, 13 insertions(+), 11 deletions(-) > > > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf > > index b856afa6eb52..3a81b6c712a9 100644 > > --- a/tools/perf/Makefile.perf > > +++ b/tools/perf/Makefile.perf > > @@ -241,7 +241,7 @@ else # force_fixdep > > > > LIB_DIR = $(srctree)/tools/lib/api/ > > TRACE_EVENT_DIR = $(srctree)/tools/lib/traceevent/ > > -BPF_DIR = $(srctree)/tools/lib/bpf/ > > +LIBBPF_DIR = $(srctree)/tools/lib/bpf/ > > SUBCMD_DIR = $(srctree)/tools/lib/subcmd/ > > LIBPERF_DIR = $(srctree)/tools/lib/perf/ > > DOC_DIR = $(srctree)/tools/perf/Documentation/ > > @@ -293,7 +293,6 @@ strip-libs = $(filter-out -l%,$(1)) > > ifneq ($(OUTPUT),) > > TE_PATH=$(OUTPUT) > > PLUGINS_PATH=$(OUTPUT) > > - BPF_PATH=$(OUTPUT) > > SUBCMD_PATH=$(OUTPUT) > > LIBPERF_PATH=$(OUTPUT) > > ifneq ($(subdir),) > > @@ -305,7 +304,6 @@ else > > TE_PATH=$(TRACE_EVENT_DIR) > > PLUGINS_PATH=$(TRACE_EVENT_DIR)plugins/ > > API_PATH=$(LIB_DIR) > > - BPF_PATH=$(BPF_DIR) > > SUBCMD_PATH=$(SUBCMD_DIR) > > LIBPERF_PATH=$(LIBPERF_DIR) > > endif > > @@ -324,7 +322,10 @@ LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS = $(if $(findstring -static,$(LDFLAGS)),,$(DY > > LIBAPI = $(API_PATH)libapi.a > > export LIBAPI > > > > -LIBBPF = $(BPF_PATH)libbpf.a > > +LIBBPF_OUTPUT = $(OUTPUT)libbpf > > +LIBBPF_DESTDIR = $(LIBBPF_OUTPUT) > > +LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include > > +LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a > > > > LIBSUBCMD = $(SUBCMD_PATH)libsubcmd.a > > > > @@ -829,12 +830,14 @@ $(LIBAPI)-clean: > > $(call QUIET_CLEAN, libapi) > > $(Q)$(MAKE) -C $(LIB_DIR) O=$(OUTPUT) clean >/dev/null > > > > -$(LIBBPF): FORCE > > - $(Q)$(MAKE) -C $(BPF_DIR) O=$(OUTPUT) $(OUTPUT)libbpf.a FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) > > +$(LIBBPF): FORCE | $(LIBBPF_OUTPUT) > > + $(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \ > > + O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \ > > + $@ install_headers > > > > $(LIBBPF)-clean: > > $(call QUIET_CLEAN, libbpf) > > - $(Q)$(MAKE) -C $(BPF_DIR) O=$(OUTPUT) clean >/dev/null > > + $(Q)$(RM) -r -- $(LIBBPF_OUTPUT) > > > > $(LIBPERF): FORCE > > $(Q)$(MAKE) -C $(LIBPERF_DIR) EXTRA_CFLAGS="$(LIBPERF_CFLAGS)" O=$(OUTPUT) $(OUTPUT)libperf.a > > @@ -1036,14 +1039,13 @@ SKELETONS += $(SKEL_OUT)/bperf_cgroup.skel.h > > > > ifdef BUILD_BPF_SKEL > > BPFTOOL := $(SKEL_TMP_OUT)/bootstrap/bpftool > > -LIBBPF_SRC := $(abspath ../lib/bpf) > > -BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(BPF_PATH) -I$(LIBBPF_SRC)/.. > > +BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(LIBBPF_INCLUDE) > > > > -$(SKEL_TMP_OUT): > > +$(SKEL_TMP_OUT) $(LIBBPF_OUTPUT): > > $(Q)$(MKDIR) -p $@ > > > > $(BPFTOOL): | $(SKEL_TMP_OUT) > > - CFLAGS= $(MAKE) -C ../bpf/bpftool \ > > + $(Q)CFLAGS= $(MAKE) -C ../bpf/bpftool \ > > OUTPUT=$(SKEL_TMP_OUT)/ bootstrap > > > > VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \ > > -- > > 2.32.0 > >
Em Sat, Nov 06, 2021 at 04:29:16PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Fri, Nov 05, 2021 at 11:38:50AM -0700, Andrii Nakryiko escreveu: > > On Thu, Nov 4, 2021 at 7:02 PM Quentin Monnet <quentin@isovalent.com> wrote: > > > > > > API headers from libbpf should not be accessed directly from the > > > library's source directory. Instead, they should be exported with "make > > > install_headers". Let's adjust perf's Makefile to install those headers > > > locally when building libbpf. > > > > > > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > > > --- > > > Note: Sending to bpf-next because it's directly related to libbpf, and > > > to similar patches merged through bpf-next, but maybe Arnaldo prefers to > > > take it? > > > > Arnaldo would know better how to thoroughly test it, so I'd prefer to > > route this through perf tree. Any objections, Arnaldo? > > Preliminary testing passed for 'BUILD_BPF_SKEL=1' with without > LIBBPF_DYNAMIC=1 (using the system's libbpf-devel to build perf), so far > so good, so I tentatively applied it, will see with the full set of > containers. Because all the preliminary tests used O= to have that OUTPUT variable set, when we do simply: make -C tools/perf it breaks: ⬢[acme@toolbox perf]$ make -C tools clean > /dev/null 2>&1 ⬢[acme@toolbox perf]$ make JOBS=1 -C tools/perf make: Entering directory '/var/home/acme/git/perf/tools/perf' BUILD: Doing 'make -j1' parallel build HOSTCC fixdep.o HOSTLD fixdep-in.o LINK fixdep <SNIP ABI sync warnings> Auto-detecting system features: ... dwarf: [ on ] ... dwarf_getlocations: [ on ] ... glibc: [ on ] ... libbfd: [ on ] ... libbfd-buildid: [ on ] ... libcap: [ on ] ... libelf: [ on ] ... libnuma: [ on ] ... numa_num_possible_cpus: [ on ] ... libperl: [ on ] ... libpython: [ on ] ... libcrypto: [ on ] ... libunwind: [ on ] ... libdw-dwarf-unwind: [ on ] ... zlib: [ on ] ... lzma: [ on ] ... get_cpuid: [ on ] ... bpf: [ on ] ... libaio: [ on ] ... libzstd: [ on ] ... disassembler-four-args: [ on ] CC fd/array.o LD fd/libapi-in.o CC fs/fs.o CC fs/tracing_path.o CC fs/cgroup.o LD fs/libapi-in.o CC cpu.o CC debug.o CC str_error_r.o LD libapi-in.o AR libapi.a CC exec-cmd.o CC help.o CC pager.o CC parse-options.o CC run-command.o CC sigchain.o CC subcmd-config.o LD libsubcmd-in.o AR libsubcmd.a CC core.o CC cpumap.o CC threadmap.o CC evsel.o CC evlist.o CC mmap.o CC zalloc.o CC xyarray.o CC lib.o LD libperf-in.o AR libperf.a make[2]: *** No rule to make target 'libbpf', needed by 'libbpf/libbpf.a'. Stop. make[1]: *** [Makefile.perf:240: sub-make] Error 2 make: *** [Makefile:70: all] Error 2 make: Leaving directory '/var/home/acme/git/perf/tools/perf' ⬢[acme@toolbox perf]$ Trying to fix... - Arnaldo
Em Sat, Nov 06, 2021 at 05:12:48PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Sat, Nov 06, 2021 at 04:29:16PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Fri, Nov 05, 2021 at 11:38:50AM -0700, Andrii Nakryiko escreveu: > > > On Thu, Nov 4, 2021 at 7:02 PM Quentin Monnet <quentin@isovalent.com> wrote: > > > > > > > > API headers from libbpf should not be accessed directly from the > > > > library's source directory. Instead, they should be exported with "make > > > > install_headers". Let's adjust perf's Makefile to install those headers > > > > locally when building libbpf. > > > > > > > > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > > > > --- > > > > Note: Sending to bpf-next because it's directly related to libbpf, and > > > > to similar patches merged through bpf-next, but maybe Arnaldo prefers to > > > > take it? > > > > > > Arnaldo would know better how to thoroughly test it, so I'd prefer to > > > route this through perf tree. Any objections, Arnaldo? > > > > Preliminary testing passed for 'BUILD_BPF_SKEL=1' with without > > LIBBPF_DYNAMIC=1 (using the system's libbpf-devel to build perf), so far > > so good, so I tentatively applied it, will see with the full set of > > containers. > > Because all the preliminary tests used O= to have that OUTPUT variable > set, when we do simply: > > make -C tools/perf So I'll have to remove it now as my container builds test both O= and in-place builds (make -C tools/perf), I know many people (Jiri for instance) don't use O=. I tried to fix this but run out of time today, visits arriving soon, so I'll try to come back to this tomorrow early morning, to push what I have in to Linus, that is blocked by this now :-\ - Arnaldo > it breaks: > > ⬢[acme@toolbox perf]$ make -C tools clean > /dev/null 2>&1 > ⬢[acme@toolbox perf]$ make JOBS=1 -C tools/perf > make: Entering directory '/var/home/acme/git/perf/tools/perf' > BUILD: Doing 'make -j1' parallel build > HOSTCC fixdep.o > HOSTLD fixdep-in.o > LINK fixdep > <SNIP ABI sync warnings> > > Auto-detecting system features: > ... dwarf: [ on ] > ... dwarf_getlocations: [ on ] > ... glibc: [ on ] > ... libbfd: [ on ] > ... libbfd-buildid: [ on ] > ... libcap: [ on ] > ... libelf: [ on ] > ... libnuma: [ on ] > ... numa_num_possible_cpus: [ on ] > ... libperl: [ on ] > ... libpython: [ on ] > ... libcrypto: [ on ] > ... libunwind: [ on ] > ... libdw-dwarf-unwind: [ on ] > ... zlib: [ on ] > ... lzma: [ on ] > ... get_cpuid: [ on ] > ... bpf: [ on ] > ... libaio: [ on ] > ... libzstd: [ on ] > ... disassembler-four-args: [ on ] > > > CC fd/array.o > LD fd/libapi-in.o > CC fs/fs.o > CC fs/tracing_path.o > CC fs/cgroup.o > LD fs/libapi-in.o > CC cpu.o > CC debug.o > CC str_error_r.o > LD libapi-in.o > AR libapi.a > CC exec-cmd.o > CC help.o > CC pager.o > CC parse-options.o > CC run-command.o > CC sigchain.o > CC subcmd-config.o > LD libsubcmd-in.o > AR libsubcmd.a > CC core.o > CC cpumap.o > CC threadmap.o > CC evsel.o > CC evlist.o > CC mmap.o > CC zalloc.o > CC xyarray.o > CC lib.o > LD libperf-in.o > AR libperf.a > make[2]: *** No rule to make target 'libbpf', needed by 'libbpf/libbpf.a'. Stop. > make[1]: *** [Makefile.perf:240: sub-make] Error 2 > make: *** [Makefile:70: all] Error 2 > make: Leaving directory '/var/home/acme/git/perf/tools/perf' > ⬢[acme@toolbox perf]$ > > Trying to fix... > > - Arnaldo
Em Sat, Nov 06, 2021 at 06:05:07PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Sat, Nov 06, 2021 at 05:12:48PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Sat, Nov 06, 2021 at 04:29:16PM -0300, Arnaldo Carvalho de Melo escreveu: > > > Em Fri, Nov 05, 2021 at 11:38:50AM -0700, Andrii Nakryiko escreveu: > > > > On Thu, Nov 4, 2021 at 7:02 PM Quentin Monnet <quentin@isovalent.com> wrote: > > > > > > > > > > API headers from libbpf should not be accessed directly from the > > > > > library's source directory. Instead, they should be exported with "make > > > > > install_headers". Let's adjust perf's Makefile to install those headers > > > > > locally when building libbpf. > > > > > > > > > > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > > > > > --- > > > > > Note: Sending to bpf-next because it's directly related to libbpf, and > > > > > to similar patches merged through bpf-next, but maybe Arnaldo prefers to > > > > > take it? > > > > > > > > Arnaldo would know better how to thoroughly test it, so I'd prefer to > > > > route this through perf tree. Any objections, Arnaldo? > > > > > > Preliminary testing passed for 'BUILD_BPF_SKEL=1' with without > > > LIBBPF_DYNAMIC=1 (using the system's libbpf-devel to build perf), so far > > > so good, so I tentatively applied it, will see with the full set of > > > containers. > > > > Because all the preliminary tests used O= to have that OUTPUT variable > > set, when we do simply: > > > > make -C tools/perf > > So I'll have to remove it now as my container builds test both O= and > in-place builds (make -C tools/perf), I know many people (Jiri for > instance) don't use O=. > > I tried to fix this but run out of time today, visits arriving soon, so > I'll try to come back to this tomorrow early morning, to push what I > have in to Linus, that is blocked by this now :-\ What I have, with your patch, is at: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tmp.perf/core It has both patches, as its needed for the BUILD_BPF_SKEL=1 mode to build correctly with/without LIBBPF_DYNAMIC=1. - Arnaldo > - Arnaldo > > > it breaks: > > > > ⬢[acme@toolbox perf]$ make -C tools clean > /dev/null 2>&1 > > ⬢[acme@toolbox perf]$ make JOBS=1 -C tools/perf > > make: Entering directory '/var/home/acme/git/perf/tools/perf' > > BUILD: Doing 'make -j1' parallel build > > HOSTCC fixdep.o > > HOSTLD fixdep-in.o > > LINK fixdep > > <SNIP ABI sync warnings> > > > > Auto-detecting system features: > > ... dwarf: [ on ] > > ... dwarf_getlocations: [ on ] > > ... glibc: [ on ] > > ... libbfd: [ on ] > > ... libbfd-buildid: [ on ] > > ... libcap: [ on ] > > ... libelf: [ on ] > > ... libnuma: [ on ] > > ... numa_num_possible_cpus: [ on ] > > ... libperl: [ on ] > > ... libpython: [ on ] > > ... libcrypto: [ on ] > > ... libunwind: [ on ] > > ... libdw-dwarf-unwind: [ on ] > > ... zlib: [ on ] > > ... lzma: [ on ] > > ... get_cpuid: [ on ] > > ... bpf: [ on ] > > ... libaio: [ on ] > > ... libzstd: [ on ] > > ... disassembler-four-args: [ on ] > > > > > > CC fd/array.o > > LD fd/libapi-in.o > > CC fs/fs.o > > CC fs/tracing_path.o > > CC fs/cgroup.o > > LD fs/libapi-in.o > > CC cpu.o > > CC debug.o > > CC str_error_r.o > > LD libapi-in.o > > AR libapi.a > > CC exec-cmd.o > > CC help.o > > CC pager.o > > CC parse-options.o > > CC run-command.o > > CC sigchain.o > > CC subcmd-config.o > > LD libsubcmd-in.o > > AR libsubcmd.a > > CC core.o > > CC cpumap.o > > CC threadmap.o > > CC evsel.o > > CC evlist.o > > CC mmap.o > > CC zalloc.o > > CC xyarray.o > > CC lib.o > > LD libperf-in.o > > AR libperf.a > > make[2]: *** No rule to make target 'libbpf', needed by 'libbpf/libbpf.a'. Stop. > > make[1]: *** [Makefile.perf:240: sub-make] Error 2 > > make: *** [Makefile:70: all] Error 2 > > make: Leaving directory '/var/home/acme/git/perf/tools/perf' > > ⬢[acme@toolbox perf]$ > > > > Trying to fix... > > > > - Arnaldo > > -- > > - Arnaldo
2021-11-06 18:20 UTC-0300 ~ Arnaldo Carvalho de Melo <acme@kernel.org> > Em Sat, Nov 06, 2021 at 06:05:07PM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Sat, Nov 06, 2021 at 05:12:48PM -0300, Arnaldo Carvalho de Melo escreveu: >>> Em Sat, Nov 06, 2021 at 04:29:16PM -0300, Arnaldo Carvalho de Melo escreveu: >>>> Em Fri, Nov 05, 2021 at 11:38:50AM -0700, Andrii Nakryiko escreveu: >>>>> On Thu, Nov 4, 2021 at 7:02 PM Quentin Monnet <quentin@isovalent.com> wrote: >>>>>> >>>>>> API headers from libbpf should not be accessed directly from the >>>>>> library's source directory. Instead, they should be exported with "make >>>>>> install_headers". Let's adjust perf's Makefile to install those headers >>>>>> locally when building libbpf. >>>>>> >>>>>> Signed-off-by: Quentin Monnet <quentin@isovalent.com> >>>>>> --- >>>>>> Note: Sending to bpf-next because it's directly related to libbpf, and >>>>>> to similar patches merged through bpf-next, but maybe Arnaldo prefers to >>>>>> take it? >>>>> >>>>> Arnaldo would know better how to thoroughly test it, so I'd prefer to >>>>> route this through perf tree. Any objections, Arnaldo? >>>> >>>> Preliminary testing passed for 'BUILD_BPF_SKEL=1' with without >>>> LIBBPF_DYNAMIC=1 (using the system's libbpf-devel to build perf), so far >>>> so good, so I tentatively applied it, will see with the full set of >>>> containers. >>> >>> Because all the preliminary tests used O= to have that OUTPUT variable >>> set, when we do simply: >>> >>> make -C tools/perf >> >> So I'll have to remove it now as my container builds test both O= and >> in-place builds (make -C tools/perf), I know many people (Jiri for >> instance) don't use O=. >> >> I tried to fix this but run out of time today, visits arriving soon, so >> I'll try to come back to this tomorrow early morning, to push what I >> have in to Linus, that is blocked by this now :-\ > > What I have, with your patch, is at: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tmp.perf/core > > It has both patches, as its needed for the BUILD_BPF_SKEL=1 mode to > build correctly with/without LIBBPF_DYNAMIC=1. Hi Arnaldo, thanks for the review and testing. Apologies, I missed that the recipe for the $(LIBBPF_OUTPUT) directory was located under the "ifdef BUILD_BPF_SKEL". I'm sending a v2 that will handle this case better. Quentin
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf index b856afa6eb52..3a81b6c712a9 100644 --- a/tools/perf/Makefile.perf +++ b/tools/perf/Makefile.perf @@ -241,7 +241,7 @@ else # force_fixdep LIB_DIR = $(srctree)/tools/lib/api/ TRACE_EVENT_DIR = $(srctree)/tools/lib/traceevent/ -BPF_DIR = $(srctree)/tools/lib/bpf/ +LIBBPF_DIR = $(srctree)/tools/lib/bpf/ SUBCMD_DIR = $(srctree)/tools/lib/subcmd/ LIBPERF_DIR = $(srctree)/tools/lib/perf/ DOC_DIR = $(srctree)/tools/perf/Documentation/ @@ -293,7 +293,6 @@ strip-libs = $(filter-out -l%,$(1)) ifneq ($(OUTPUT),) TE_PATH=$(OUTPUT) PLUGINS_PATH=$(OUTPUT) - BPF_PATH=$(OUTPUT) SUBCMD_PATH=$(OUTPUT) LIBPERF_PATH=$(OUTPUT) ifneq ($(subdir),) @@ -305,7 +304,6 @@ else TE_PATH=$(TRACE_EVENT_DIR) PLUGINS_PATH=$(TRACE_EVENT_DIR)plugins/ API_PATH=$(LIB_DIR) - BPF_PATH=$(BPF_DIR) SUBCMD_PATH=$(SUBCMD_DIR) LIBPERF_PATH=$(LIBPERF_DIR) endif @@ -324,7 +322,10 @@ LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS = $(if $(findstring -static,$(LDFLAGS)),,$(DY LIBAPI = $(API_PATH)libapi.a export LIBAPI -LIBBPF = $(BPF_PATH)libbpf.a +LIBBPF_OUTPUT = $(OUTPUT)libbpf +LIBBPF_DESTDIR = $(LIBBPF_OUTPUT) +LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include +LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a LIBSUBCMD = $(SUBCMD_PATH)libsubcmd.a @@ -829,12 +830,14 @@ $(LIBAPI)-clean: $(call QUIET_CLEAN, libapi) $(Q)$(MAKE) -C $(LIB_DIR) O=$(OUTPUT) clean >/dev/null -$(LIBBPF): FORCE - $(Q)$(MAKE) -C $(BPF_DIR) O=$(OUTPUT) $(OUTPUT)libbpf.a FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) +$(LIBBPF): FORCE | $(LIBBPF_OUTPUT) + $(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \ + O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \ + $@ install_headers $(LIBBPF)-clean: $(call QUIET_CLEAN, libbpf) - $(Q)$(MAKE) -C $(BPF_DIR) O=$(OUTPUT) clean >/dev/null + $(Q)$(RM) -r -- $(LIBBPF_OUTPUT) $(LIBPERF): FORCE $(Q)$(MAKE) -C $(LIBPERF_DIR) EXTRA_CFLAGS="$(LIBPERF_CFLAGS)" O=$(OUTPUT) $(OUTPUT)libperf.a @@ -1036,14 +1039,13 @@ SKELETONS += $(SKEL_OUT)/bperf_cgroup.skel.h ifdef BUILD_BPF_SKEL BPFTOOL := $(SKEL_TMP_OUT)/bootstrap/bpftool -LIBBPF_SRC := $(abspath ../lib/bpf) -BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(BPF_PATH) -I$(LIBBPF_SRC)/.. +BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(LIBBPF_INCLUDE) -$(SKEL_TMP_OUT): +$(SKEL_TMP_OUT) $(LIBBPF_OUTPUT): $(Q)$(MKDIR) -p $@ $(BPFTOOL): | $(SKEL_TMP_OUT) - CFLAGS= $(MAKE) -C ../bpf/bpftool \ + $(Q)CFLAGS= $(MAKE) -C ../bpf/bpftool \ OUTPUT=$(SKEL_TMP_OUT)/ bootstrap VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \
API headers from libbpf should not be accessed directly from the library's source directory. Instead, they should be exported with "make install_headers". Let's adjust perf's Makefile to install those headers locally when building libbpf. Signed-off-by: Quentin Monnet <quentin@isovalent.com> --- Note: Sending to bpf-next because it's directly related to libbpf, and to similar patches merged through bpf-next, but maybe Arnaldo prefers to take it? --- tools/perf/Makefile.perf | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)