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 <[email protected]> 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 <[email protected]>
> > > > > ---
> > > > > 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
--
- Arnaldo
2021-11-06 18:20 UTC-0300 ~ Arnaldo Carvalho de Melo <[email protected]>
> 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 <[email protected]> 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 <[email protected]>
>>>>>> ---
>>>>>> 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
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.
v2:
- Fix $(LIBBPF_OUTPUT) when $(OUTPUT) is null.
- Make sure the recipe for $(LIBBPF_OUTPUT) is not under a "ifdef".
Signed-off-by: Quentin Monnet <[email protected]>
---
tools/perf/Makefile.perf | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index b856afa6eb52..e01ada5c9876 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,14 @@ LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS = $(if $(findstring -static,$(LDFLAGS)),,$(DY
LIBAPI = $(API_PATH)libapi.a
export LIBAPI
-LIBBPF = $(BPF_PATH)libbpf.a
+ifneq ($(OUTPUT),)
+ LIBBPF_OUTPUT = $(abspath $(OUTPUT))/libbpf
+else
+ LIBBPF_OUTPUT = $(CURDIR)/libbpf
+endif
+LIBBPF_DESTDIR = $(LIBBPF_OUTPUT)
+LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include
+LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a
LIBSUBCMD = $(SUBCMD_PATH)libsubcmd.a
@@ -829,12 +834,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
@@ -1034,16 +1041,15 @@ SKELETONS := $(SKEL_OUT)/bpf_prog_profiler.skel.h
SKELETONS += $(SKEL_OUT)/bperf_leader.skel.h $(SKEL_OUT)/bperf_follower.skel.h
SKELETONS += $(SKEL_OUT)/bperf_cgroup.skel.h
+$(SKEL_TMP_OUT) $(LIBBPF_OUTPUT):
+ $(Q)$(MKDIR) -p $@
+
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)/..
-
-$(SKEL_TMP_OUT):
- $(Q)$(MKDIR) -p $@
+BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(LIBBPF_INCLUDE)
$(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 Sun, Nov 07, 2021 at 12:24:45AM +0000, Quentin Monnet escreveu:
> 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.
>
> v2:
> - Fix $(LIBBPF_OUTPUT) when $(OUTPUT) is null.
> - Make sure the recipe for $(LIBBPF_OUTPUT) is not under a "ifdef".
Thanks for the prompt reply, now the cases where it was failing are
passing!
Best regards,
- Arnaldo
> Signed-off-by: Quentin Monnet <[email protected]>
> ---
> tools/perf/Makefile.perf | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index b856afa6eb52..e01ada5c9876 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,14 @@ LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS = $(if $(findstring -static,$(LDFLAGS)),,$(DY
> LIBAPI = $(API_PATH)libapi.a
> export LIBAPI
>
> -LIBBPF = $(BPF_PATH)libbpf.a
> +ifneq ($(OUTPUT),)
> + LIBBPF_OUTPUT = $(abspath $(OUTPUT))/libbpf
> +else
> + LIBBPF_OUTPUT = $(CURDIR)/libbpf
> +endif
> +LIBBPF_DESTDIR = $(LIBBPF_OUTPUT)
> +LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include
> +LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a
>
> LIBSUBCMD = $(SUBCMD_PATH)libsubcmd.a
>
> @@ -829,12 +834,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
> @@ -1034,16 +1041,15 @@ SKELETONS := $(SKEL_OUT)/bpf_prog_profiler.skel.h
> SKELETONS += $(SKEL_OUT)/bperf_leader.skel.h $(SKEL_OUT)/bperf_follower.skel.h
> SKELETONS += $(SKEL_OUT)/bperf_cgroup.skel.h
>
> +$(SKEL_TMP_OUT) $(LIBBPF_OUTPUT):
> + $(Q)$(MKDIR) -p $@
> +
> 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)/..
> -
> -$(SKEL_TMP_OUT):
> - $(Q)$(MKDIR) -p $@
> +BPF_INCLUDE := -I$(SKEL_TMP_OUT)/.. -I$(LIBBPF_INCLUDE)
>
> $(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
--
- Arnaldo