Commit 760ebc45746b ("perf lock contention: Add empty 'struct rq' to
satisfy libbpf 'runqueue' type verification") inadvertently created a
declaration of 'struct rq' that conflicted with a generated
vmlinux.h's:
```
util/bpf_skel/lock_contention.bpf.c:419:8: error: redefinition of 'rq'
struct rq {};
^
/tmp/perf/util/bpf_skel/.tmp/../vmlinux.h:45630:8: note: previous definition is here
struct rq {
^
1 error generated.
```
Fix the issue by moving the declaration to vmlinux.h. So this can't
happen again, bring back build support for generating vmlinux.h then
add build tests.
v4. Rebase and add Namhyung and Jiri's acked-by.
v3. Address Namhyung's comments on filtering ELF files with readelf.
v2. Rebase on perf-tools-next. Add Andrii's acked-by. Add patch to
filter out kernels that lack a .BTF section and cause the build to
break.
Ian Rogers (4):
perf build: Add ability to build with a generated vmlinux.h
perf bpf: Move the declaration of struct rq
perf test: Add build tests for BUILD_BPF_SKEL
perf build: Filter out BTF sources without a .BTF section
tools/perf/Makefile.config | 4 ++
tools/perf/Makefile.perf | 39 ++++++++++++++++++-
tools/perf/tests/make | 4 ++
tools/perf/util/bpf_skel/.gitignore | 1 +
.../perf/util/bpf_skel/lock_contention.bpf.c | 2 -
.../util/bpf_skel/{ => vmlinux}/vmlinux.h | 10 +++++
6 files changed, 57 insertions(+), 3 deletions(-)
rename tools/perf/util/bpf_skel/{ => vmlinux}/vmlinux.h (90%)
--
2.41.0.162.gfafddb0af9-goog
Commit a887466562b4 ("perf bpf skels: Stop using vmlinux.h generated
from BTF, use subset of used structs + CO-RE") made it so that
vmlinux.h was uncondtionally included from
tools/perf/util/vmlinux.h. This change reverts part of that change (so
that vmlinux.h is once again generated) and makes it so that the
vmlinux.h used at build time is selected from the VMLINUX_H
variable. By default the VMLINUX_H variable is set to the vmlinux.h
added in change a887466562b4, but if GEN_VMLINUX_H=1 is passed on the
build command line then the previous generation behavior kicks in.
The build with GEN_VMLINUX_H=1 currently fails with:
```
util/bpf_skel/lock_contention.bpf.c:419:8: error: redefinition of 'rq'
struct rq {};
^
/tmp/perf/util/bpf_skel/.tmp/../vmlinux.h:45630:8: note: previous definition is here
struct rq {
^
1 error generated.
```
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Andrii Nakryiko <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
---
tools/perf/Makefile.config | 4 ++++
tools/perf/Makefile.perf | 16 +++++++++++++++-
tools/perf/util/bpf_skel/.gitignore | 1 +
tools/perf/util/bpf_skel/{ => vmlinux}/vmlinux.h | 0
4 files changed, 20 insertions(+), 1 deletion(-)
rename tools/perf/util/bpf_skel/{ => vmlinux}/vmlinux.h (100%)
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 9c5aa14a44cf..78411252b72a 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -680,6 +680,10 @@ ifdef BUILD_BPF_SKEL
CFLAGS += -DHAVE_BPF_SKEL
endif
+ifndef GEN_VMLINUX_H
+ VMLINUX_H=$(src-perf)/util/bpf_skel/vmlinux/vmlinux.h
+endif
+
dwarf-post-unwind := 1
dwarf-post-unwind-text := BUG
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index b1e62a621f92..80d772cc5fcb 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -1084,7 +1084,21 @@ $(BPFTOOL): | $(SKEL_TMP_OUT)
$(Q)CFLAGS= $(MAKE) -C ../bpf/bpftool \
OUTPUT=$(SKEL_TMP_OUT)/ bootstrap
-$(SKEL_TMP_OUT)/%.bpf.o: util/bpf_skel/%.bpf.c $(LIBBPF) | $(SKEL_TMP_OUT)
+VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \
+ $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \
+ ../../vmlinux \
+ /sys/kernel/btf/vmlinux \
+ /boot/vmlinux-$(shell uname -r)
+VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
+
+$(SKEL_OUT)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL)
+ifeq ($(VMLINUX_H),)
+ $(QUIET_GEN)$(BPFTOOL) btf dump file $< format c > $@
+else
+ $(Q)cp "$(VMLINUX_H)" $@
+endif
+
+$(SKEL_TMP_OUT)/%.bpf.o: util/bpf_skel/%.bpf.c $(LIBBPF) $(SKEL_OUT)/vmlinux.h | $(SKEL_TMP_OUT)
$(QUIET_CLANG)$(CLANG) -g -O2 -target bpf -Wall -Werror $(BPF_INCLUDE) $(TOOLS_UAPI_INCLUDE) \
-c $(filter util/bpf_skel/%.bpf.c,$^) -o $@
diff --git a/tools/perf/util/bpf_skel/.gitignore b/tools/perf/util/bpf_skel/.gitignore
index 7a1c832825de..cd01455e1b53 100644
--- a/tools/perf/util/bpf_skel/.gitignore
+++ b/tools/perf/util/bpf_skel/.gitignore
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
.tmp
*.skel.h
+vmlinux.h
diff --git a/tools/perf/util/bpf_skel/vmlinux.h b/tools/perf/util/bpf_skel/vmlinux/vmlinux.h
similarity index 100%
rename from tools/perf/util/bpf_skel/vmlinux.h
rename to tools/perf/util/bpf_skel/vmlinux/vmlinux.h
--
2.41.0.162.gfafddb0af9-goog
Add tests with and without generating vmlinux.h.
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Andrii Nakryiko <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/make | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/perf/tests/make b/tools/perf/tests/make
index 885cd321d67b..58cf96d762d0 100644
--- a/tools/perf/tests/make
+++ b/tools/perf/tests/make
@@ -70,6 +70,8 @@ make_python_perf_so := $(python_perf_so)
make_debug := DEBUG=1
make_nondistro := BUILD_NONDISTRO=1
make_extra_tests := EXTRA_TESTS=1
+make_bpf_skel := BUILD_BPF_SKEL=1
+make_gen_vmlinux_h := BUILD_BPF_SKEL=1 GEN_VMLINUX_H=1
make_no_libperl := NO_LIBPERL=1
make_no_libpython := NO_LIBPYTHON=1
make_no_scripts := NO_LIBPYTHON=1 NO_LIBPERL=1
@@ -137,6 +139,8 @@ endif
run += make_python_perf_so
run += make_debug
run += make_nondistro
+run += make_build_bpf_skel
+run += make_gen_vmlinux_h
run += make_no_libperl
run += make_no_libpython
run += make_no_scripts
--
2.41.0.162.gfafddb0af9-goog
struct rq is defined in vmlinux.h when the vmlinux.h is generated,
this causes a redefinition failure if it is declared in
lock_contention.bpf.c. Move the definition to vmlinux.h for
consistency with the generated version.
Fixes: 760ebc45746b ("perf lock contention: Add empty 'struct rq' to satisfy libbpf 'runqueue' type verification")
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Andrii Nakryiko <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
---
tools/perf/util/bpf_skel/lock_contention.bpf.c | 2 --
tools/perf/util/bpf_skel/vmlinux/vmlinux.h | 10 ++++++++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
index 1d48226ae75d..8d3cfbb3cc65 100644
--- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
+++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
@@ -416,8 +416,6 @@ int contention_end(u64 *ctx)
return 0;
}
-struct rq {};
-
extern struct rq runqueues __ksym;
struct rq___old {
diff --git a/tools/perf/util/bpf_skel/vmlinux/vmlinux.h b/tools/perf/util/bpf_skel/vmlinux/vmlinux.h
index c7ed51b0c1ef..ab84a6e1da5e 100644
--- a/tools/perf/util/bpf_skel/vmlinux/vmlinux.h
+++ b/tools/perf/util/bpf_skel/vmlinux/vmlinux.h
@@ -171,4 +171,14 @@ struct bpf_perf_event_data_kern {
struct perf_sample_data *data;
struct perf_event *event;
} __attribute__((preserve_access_index));
+
+/*
+ * If 'struct rq' isn't defined for lock_contention.bpf.c, for the sake of
+ * rq___old and rq___new, then the type for the 'runqueue' variable ends up
+ * being a forward declaration (BTF_KIND_FWD) while the kernel has it defined
+ * (BTF_KIND_STRUCT). The definition appears in vmlinux.h rather than
+ * lock_contention.bpf.c for consistency with a generated vmlinux.h.
+ */
+struct rq {};
+
#endif // __VMLINUX_H
--
2.41.0.162.gfafddb0af9-goog
If generating vmlinux.h, make the code to generate it more tolerant by
filtering out paths to kernels that lack a .BTF section.
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
---
tools/perf/Makefile.perf | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 80d772cc5fcb..8f442419a895 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -197,6 +197,7 @@ FLEX ?= flex
BISON ?= bison
STRIP = strip
AWK = awk
+READELF ?= readelf
# include Makefile.config by default and rule out
# non-config cases
@@ -1084,12 +1085,34 @@ $(BPFTOOL): | $(SKEL_TMP_OUT)
$(Q)CFLAGS= $(MAKE) -C ../bpf/bpftool \
OUTPUT=$(SKEL_TMP_OUT)/ bootstrap
-VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \
+# Paths to search for a kernel to generate vmlinux.h from.
+VMLINUX_BTF_ELF_PATHS ?= $(if $(O),$(O)/vmlinux) \
$(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \
../../vmlinux \
- /sys/kernel/btf/vmlinux \
/boot/vmlinux-$(shell uname -r)
-VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
+
+# Paths to BTF information.
+VMLINUX_BTF_BTF_PATHS ?= /sys/kernel/btf/vmlinux
+
+# Filter out kernels that don't exist or without a BTF section.
+VMLINUX_BTF_ELF_ABSPATHS ?= $(abspath $(wildcard $(VMLINUX_BTF_ELF_PATHS)))
+VMLINUX_BTF_PATHS ?= $(shell for file in $(VMLINUX_BTF_ELF_ABSPATHS); \
+ do \
+ if [ -f $$file ] && ($(READELF) -S "$$file" | grep -q .BTF); \
+ then \
+ echo "$$file"; \
+ fi; \
+ done) \
+ $(wildcard $(VMLINUX_BTF_BTF_PATHS))
+
+# Select the first as the source of vmlinux.h.
+VMLINUX_BTF ?= $(firstword $(VMLINUX_BTF_PATHS))
+
+ifeq ($(VMLINUX_H),)
+ ifeq ($(VMLINUX_BTF),)
+ $(error Missing bpftool input for generating vmlinux.h)
+ endif
+endif
$(SKEL_OUT)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL)
ifeq ($(VMLINUX_H),)
--
2.41.0.162.gfafddb0af9-goog
On Thu, Jun 22, 2023 at 9:14 PM Ian Rogers <[email protected]> wrote:
>
> Commit 760ebc45746b ("perf lock contention: Add empty 'struct rq' to
> satisfy libbpf 'runqueue' type verification") inadvertently created a
> declaration of 'struct rq' that conflicted with a generated
> vmlinux.h's:
>
> ```
> util/bpf_skel/lock_contention.bpf.c:419:8: error: redefinition of 'rq'
> struct rq {};
> ^
> /tmp/perf/util/bpf_skel/.tmp/../vmlinux.h:45630:8: note: previous definition is here
> struct rq {
> ^
> 1 error generated.
> ```
>
> Fix the issue by moving the declaration to vmlinux.h. So this can't
> happen again, bring back build support for generating vmlinux.h then
> add build tests.
>
> v4. Rebase and add Namhyung and Jiri's acked-by.
> v3. Address Namhyung's comments on filtering ELF files with readelf.
> v2. Rebase on perf-tools-next. Add Andrii's acked-by. Add patch to
> filter out kernels that lack a .BTF section and cause the build to
> break.
>
> Ian Rogers (4):
> perf build: Add ability to build with a generated vmlinux.h
> perf bpf: Move the declaration of struct rq
> perf test: Add build tests for BUILD_BPF_SKEL
> perf build: Filter out BTF sources without a .BTF section
Thanks, I'll take them with the following change.
Namhyung
---
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index b1e62a621f92..90f0eaf179fd 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -132,6 +132,8 @@ include ../scripts/utilities.mak
# Define EXTRA_TESTS to enable building extra tests useful mainly to perf
# developers, such as:
# x86 instruction decoder - new instructions test
+#
+# Define GEN_VMLINUX_H to generate vmlinux.h from the BTF.
# As per kernel Makefile, avoid funny character set dependencies
unexport LC_ALL
On Fri, Jun 23, 2023 at 5:41 PM Namhyung Kim <[email protected]> wrote:
>
> On Thu, Jun 22, 2023 at 9:14 PM Ian Rogers <[email protected]> wrote:
> >
> > Commit 760ebc45746b ("perf lock contention: Add empty 'struct rq' to
> > satisfy libbpf 'runqueue' type verification") inadvertently created a
> > declaration of 'struct rq' that conflicted with a generated
> > vmlinux.h's:
> >
> > ```
> > util/bpf_skel/lock_contention.bpf.c:419:8: error: redefinition of 'rq'
> > struct rq {};
> > ^
> > /tmp/perf/util/bpf_skel/.tmp/../vmlinux.h:45630:8: note: previous definition is here
> > struct rq {
> > ^
> > 1 error generated.
> > ```
> >
> > Fix the issue by moving the declaration to vmlinux.h. So this can't
> > happen again, bring back build support for generating vmlinux.h then
> > add build tests.
> >
> > v4. Rebase and add Namhyung and Jiri's acked-by.
> > v3. Address Namhyung's comments on filtering ELF files with readelf.
> > v2. Rebase on perf-tools-next. Add Andrii's acked-by. Add patch to
> > filter out kernels that lack a .BTF section and cause the build to
> > break.
> >
> > Ian Rogers (4):
> > perf build: Add ability to build with a generated vmlinux.h
> > perf bpf: Move the declaration of struct rq
> > perf test: Add build tests for BUILD_BPF_SKEL
> > perf build: Filter out BTF sources without a .BTF section
>
> Thanks, I'll take them with the following change.
Applied to perf-tools-next with it, thanks!