linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Bring back vmlinux.h generation
@ 2023-06-05 20:27 Ian Rogers
  2023-06-05 20:27 ` [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h Ian Rogers
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Ian Rogers @ 2023-06-05 20:27 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, James Clark, Tiezhu Yang, Yang Jihong,
	linux-perf-users, linux-kernel, bpf

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.

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                      | 33 ++++++++++++++++++-
 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, 51 insertions(+), 3 deletions(-)
 rename tools/perf/util/bpf_skel/{ => vmlinux}/vmlinux.h (90%)

-- 
2.41.0.rc0.172.g3f132b7071-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h
  2023-06-05 20:27 [PATCH v2 0/4] Bring back vmlinux.h generation Ian Rogers
@ 2023-06-05 20:27 ` Ian Rogers
  2023-06-06  2:56   ` Jiri Olsa
  2023-06-06 12:26   ` kernel test robot
  2023-06-05 20:27 ` [PATCH v2 2/4] perf bpf: Move the declaration of struct rq Ian Rogers
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 12+ messages in thread
From: Ian Rogers @ 2023-06-05 20:27 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, James Clark, Tiezhu Yang, Yang Jihong,
	linux-perf-users, linux-kernel, bpf
  Cc: Andrii Nakryiko

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 <irogers@google.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
 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 a794d9eca93d..08d4e7eaa721 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 f48794816d82..f1840af195c0 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -1080,7 +1080,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.rc0.172.g3f132b7071-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v2 2/4] perf bpf: Move the declaration of struct rq
  2023-06-05 20:27 [PATCH v2 0/4] Bring back vmlinux.h generation Ian Rogers
  2023-06-05 20:27 ` [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h Ian Rogers
@ 2023-06-05 20:27 ` Ian Rogers
  2023-06-05 20:27 ` [PATCH v2 3/4] perf test: Add build tests for BUILD_BPF_SKEL Ian Rogers
  2023-06-05 20:27 ` [PATCH v2 4/4] perf build: Filter out BTF sources without a .BTF section Ian Rogers
  3 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2023-06-05 20:27 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, James Clark, Tiezhu Yang, Yang Jihong,
	linux-perf-users, linux-kernel, bpf
  Cc: Andrii Nakryiko

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 <irogers@google.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
 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.rc0.172.g3f132b7071-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v2 3/4] perf test: Add build tests for BUILD_BPF_SKEL
  2023-06-05 20:27 [PATCH v2 0/4] Bring back vmlinux.h generation Ian Rogers
  2023-06-05 20:27 ` [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h Ian Rogers
  2023-06-05 20:27 ` [PATCH v2 2/4] perf bpf: Move the declaration of struct rq Ian Rogers
@ 2023-06-05 20:27 ` Ian Rogers
  2023-06-05 20:27 ` [PATCH v2 4/4] perf build: Filter out BTF sources without a .BTF section Ian Rogers
  3 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2023-06-05 20:27 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, James Clark, Tiezhu Yang, Yang Jihong,
	linux-perf-users, linux-kernel, bpf
  Cc: Andrii Nakryiko

Add tests with and without generating vmlinux.h.

Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/perf/tests/make | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/tests/make b/tools/perf/tests/make
index 8dd3f8090352..775f374d9345 100644
--- a/tools/perf/tests/make
+++ b/tools/perf/tests/make
@@ -69,6 +69,8 @@ make_clean_all      := clean all
 make_python_perf_so := $(python_perf_so)
 make_debug          := DEBUG=1
 make_nondistro      := BUILD_NONDISTRO=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
@@ -136,6 +138,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.rc0.172.g3f132b7071-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v2 4/4] perf build: Filter out BTF sources without a .BTF section
  2023-06-05 20:27 [PATCH v2 0/4] Bring back vmlinux.h generation Ian Rogers
                   ` (2 preceding siblings ...)
  2023-06-05 20:27 ` [PATCH v2 3/4] perf test: Add build tests for BUILD_BPF_SKEL Ian Rogers
@ 2023-06-05 20:27 ` Ian Rogers
  2023-06-05 23:35   ` Namhyung Kim
  3 siblings, 1 reply; 12+ messages in thread
From: Ian Rogers @ 2023-06-05 20:27 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, James Clark, Tiezhu Yang, Yang Jihong,
	linux-perf-users, linux-kernel, bpf

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 <irogers@google.com>
---
 tools/perf/Makefile.perf | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index f1840af195c0..c3bb27a912b0 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -193,6 +193,7 @@ FLEX    ?= flex
 BISON   ?= bison
 STRIP   = strip
 AWK     = awk
+READELF ?= readelf
 
 # include Makefile.config by default and rule out
 # non-config cases
@@ -1080,12 +1081,28 @@ $(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) -t "$$file" | grep .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))
 
 $(SKEL_OUT)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL)
 ifeq ($(VMLINUX_H),)
-- 
2.41.0.rc0.172.g3f132b7071-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] perf build: Filter out BTF sources without a .BTF section
  2023-06-05 20:27 ` [PATCH v2 4/4] perf build: Filter out BTF sources without a .BTF section Ian Rogers
@ 2023-06-05 23:35   ` Namhyung Kim
  2023-06-07  7:11     ` Ian Rogers
  0 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2023-06-05 23:35 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	James Clark, Tiezhu Yang, Yang Jihong, linux-perf-users,
	linux-kernel, bpf

Hi Ian,

On Mon, Jun 5, 2023 at 1:28 PM Ian Rogers <irogers@google.com> wrote:
>
> 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 <irogers@google.com>
> ---
>  tools/perf/Makefile.perf | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index f1840af195c0..c3bb27a912b0 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -193,6 +193,7 @@ FLEX    ?= flex
>  BISON   ?= bison
>  STRIP   = strip
>  AWK     = awk
> +READELF ?= readelf
>
>  # include Makefile.config by default and rule out
>  # non-config cases
> @@ -1080,12 +1081,28 @@ $(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) -t "$$file" | grep .BTF); \

Wouldn't it be `readelf -S` instead?  Also I think grep needs -q to
suppress output.


> +                               then \
> +                                       echo "$$file"; \
> +                               fi; \
> +                       done) \
> +                       $(wildcard $(VMLINUX_BTF_BTF_PATHS))

This changes the order of processing the sysfs file.
But I'm not sure it matters much as both /boot/vmlinux and sysfs
should refer to the running kernel.

Thanks,
Namhyung


> +
> +# Select the first as the source of vmlinux.h.
> +VMLINUX_BTF ?= $(firstword $(VMLINUX_BTF_PATHS))
>
>  $(SKEL_OUT)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL)
>  ifeq ($(VMLINUX_H),)
> --
> 2.41.0.rc0.172.g3f132b7071-goog
>

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h
  2023-06-05 20:27 ` [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h Ian Rogers
@ 2023-06-06  2:56   ` Jiri Olsa
  2023-06-06  4:25     ` Ian Rogers
  2023-06-06 12:26   ` kernel test robot
  1 sibling, 1 reply; 12+ messages in thread
From: Jiri Olsa @ 2023-06-06  2:56 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Namhyung Kim, Adrian Hunter,
	James Clark, Tiezhu Yang, Yang Jihong, linux-perf-users,
	linux-kernel, bpf, Andrii Nakryiko

On Mon, Jun 05, 2023 at 01:27:09PM -0700, Ian Rogers wrote:
> 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 <irogers@google.com>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  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%)

looks good, but I don't understand why you moved the vmlinux.h

jirka

> 
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index a794d9eca93d..08d4e7eaa721 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 f48794816d82..f1840af195c0 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -1080,7 +1080,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.rc0.172.g3f132b7071-goog
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h
  2023-06-06  2:56   ` Jiri Olsa
@ 2023-06-06  4:25     ` Ian Rogers
  2023-06-06 18:06       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Rogers @ 2023-06-06  4:25 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Namhyung Kim, Adrian Hunter,
	James Clark, Tiezhu Yang, Yang Jihong, linux-perf-users,
	linux-kernel, bpf, Andrii Nakryiko

On Mon, Jun 5, 2023 at 7:57 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Jun 05, 2023 at 01:27:09PM -0700, Ian Rogers wrote:
> > 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 <irogers@google.com>
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> >  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%)
>
> looks good, but I don't understand why you moved the vmlinux.h
>
> jirka

Dumb reason, as headers in the same directory take priority, I had to
move the vmlinux.h out of the directory with the C code for skeletons
so that it could be selected via a -I.

Thanks,
Ian

> >
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index a794d9eca93d..08d4e7eaa721 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 f48794816d82..f1840af195c0 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -1080,7 +1080,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.rc0.172.g3f132b7071-goog
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h
  2023-06-05 20:27 ` [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h Ian Rogers
  2023-06-06  2:56   ` Jiri Olsa
@ 2023-06-06 12:26   ` kernel test robot
  1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2023-06-06 12:26 UTC (permalink / raw)
  To: Ian Rogers, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Adrian Hunter, James Clark, Tiezhu Yang,
	Yang Jihong, linux-perf-users, linux-kernel, bpf
  Cc: llvm, oe-kbuild-all, Andrii Nakryiko

Hi Ian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on acme/perf/core]
[also build test WARNING on tip/master tip/auto-latest linus/master v6.4-rc5 next-20230606]
[cannot apply to tip/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ian-Rogers/perf-build-Add-ability-to-build-with-a-generated-vmlinux-h/20230606-043106
base:   https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/core
patch link:    https://lore.kernel.org/r/20230605202712.1690876-2-irogers%40google.com
patch subject: [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h
config: i386-randconfig-i052-20230605 (https://download.01.org/0day-ci/archive/20230606/202306062005.DdBIyoSS-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build):
        mkdir -p ~/bin
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git remote add acme https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
        git fetch acme perf/core
        git checkout acme/perf/core
        b4 shazam https://lore.kernel.org/r/20230605202712.1690876-2-irogers@google.com
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306062005.DdBIyoSS-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> tools/perf/util/bpf_skel/vmlinux/vmlinux.h: warning: ignored by one of the .gitignore files
   tools/testing/selftests/arm64/tags/.gitignore: warning: ignored by one of the .gitignore files
   tools/testing/selftests/arm64/tags/Makefile: warning: ignored by one of the .gitignore files
   tools/testing/selftests/arm64/tags/run_tags_test.sh: warning: ignored by one of the .gitignore files
   tools/testing/selftests/arm64/tags/tags_test.c: warning: ignored by one of the .gitignore files
   tools/testing/selftests/kvm/.gitignore: warning: ignored by one of the .gitignore files
   tools/testing/selftests/kvm/Makefile: warning: ignored by one of the .gitignore files
   tools/testing/selftests/kvm/config: warning: ignored by one of the .gitignore files
   tools/testing/selftests/kvm/settings: warning: ignored by one of the .gitignore files

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h
  2023-06-06  4:25     ` Ian Rogers
@ 2023-06-06 18:06       ` Arnaldo Carvalho de Melo
  2023-06-06 23:18         ` Ian Rogers
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-06-06 18:06 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Namhyung Kim, Adrian Hunter, James Clark,
	Tiezhu Yang, Yang Jihong, linux-perf-users, linux-kernel, bpf,
	Andrii Nakryiko

Em Mon, Jun 05, 2023 at 09:25:54PM -0700, Ian Rogers escreveu:
> On Mon, Jun 5, 2023 at 7:57 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Mon, Jun 05, 2023 at 01:27:09PM -0700, Ian Rogers wrote:
> > > 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 <irogers@google.com>
> > > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> > > ---
> > >  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%)
> >
> > looks good, but I don't understand why you moved the vmlinux.h
> >
> > jirka
> 
> Dumb reason, as headers in the same directory take priority, I had to
> move the vmlinux.h out of the directory with the C code for skeletons
> so that it could be selected via a -I.

Can this be in a separate patch, i.e. moving vmlinux to a separate
directory? I was going to cherry pick the 'struct rq' fix but then it
touches the vmlinux/vmlinux.h file that is in this first patch that has
review comments.

- Arnaldo
 
> Thanks,
> Ian
> 
> > >
> > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > > index a794d9eca93d..08d4e7eaa721 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 f48794816d82..f1840af195c0 100644
> > > --- a/tools/perf/Makefile.perf
> > > +++ b/tools/perf/Makefile.perf
> > > @@ -1080,7 +1080,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.rc0.172.g3f132b7071-goog
> > >

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h
  2023-06-06 18:06       ` Arnaldo Carvalho de Melo
@ 2023-06-06 23:18         ` Ian Rogers
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2023-06-06 23:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Namhyung Kim, Adrian Hunter, James Clark,
	Tiezhu Yang, Yang Jihong, linux-perf-users, linux-kernel, bpf,
	Andrii Nakryiko

On Tue, Jun 6, 2023 at 11:06 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Mon, Jun 05, 2023 at 09:25:54PM -0700, Ian Rogers escreveu:
> > On Mon, Jun 5, 2023 at 7:57 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Mon, Jun 05, 2023 at 01:27:09PM -0700, Ian Rogers wrote:
> > > > 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 <irogers@google.com>
> > > > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> > > > ---
> > > >  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%)
> > >
> > > looks good, but I don't understand why you moved the vmlinux.h
> > >
> > > jirka
> >
> > Dumb reason, as headers in the same directory take priority, I had to
> > move the vmlinux.h out of the directory with the C code for skeletons
> > so that it could be selected via a -I.
>
> Can this be in a separate patch, i.e. moving vmlinux to a separate
> directory? I was going to cherry pick the 'struct rq' fix but then it
> touches the vmlinux/vmlinux.h file that is in this first patch that has
> review comments.

I think the comments were more of a question of why the move?
Hopefully I answered that, the move is necessary to defeat the include
path order preferring files in the same directory. We need the move,
we could vary the directory name.

Thanks,
Ian

> - Arnaldo
>
> > Thanks,
> > Ian
> >
> > > >
> > > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > > > index a794d9eca93d..08d4e7eaa721 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 f48794816d82..f1840af195c0 100644
> > > > --- a/tools/perf/Makefile.perf
> > > > +++ b/tools/perf/Makefile.perf
> > > > @@ -1080,7 +1080,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.rc0.172.g3f132b7071-goog
> > > >
>
> --
>
> - Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] perf build: Filter out BTF sources without a .BTF section
  2023-06-05 23:35   ` Namhyung Kim
@ 2023-06-07  7:11     ` Ian Rogers
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2023-06-07  7:11 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	James Clark, Tiezhu Yang, Yang Jihong, linux-perf-users,
	linux-kernel, bpf

On Mon, Jun 5, 2023 at 4:35 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> On Mon, Jun 5, 2023 at 1:28 PM Ian Rogers <irogers@google.com> wrote:
> >
> > 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 <irogers@google.com>
> > ---
> >  tools/perf/Makefile.perf | 23 ++++++++++++++++++++---
> >  1 file changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index f1840af195c0..c3bb27a912b0 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -193,6 +193,7 @@ FLEX    ?= flex
> >  BISON   ?= bison
> >  STRIP   = strip
> >  AWK     = awk
> > +READELF ?= readelf
> >
> >  # include Makefile.config by default and rule out
> >  # non-config cases
> > @@ -1080,12 +1081,28 @@ $(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) -t "$$file" | grep .BTF); \
>
> Wouldn't it be `readelf -S` instead?  Also I think grep needs -q to
> suppress output.

Makes sense, I can change it in v3.

> > +                               then \
> > +                                       echo "$$file"; \
> > +                               fi; \
> > +                       done) \
> > +                       $(wildcard $(VMLINUX_BTF_BTF_PATHS))
>
> This changes the order of processing the sysfs file.
> But I'm not sure it matters much as both /boot/vmlinux and sysfs
> should refer to the running kernel.

Agreed. readelf fails for /sys/kernel/btf/vmlinux as it isn't an elf
file and I'm not sure it is worth worrying too much about the order
here.

Thanks,
Ian

> Thanks,
> Namhyung
>
>
> > +
> > +# Select the first as the source of vmlinux.h.
> > +VMLINUX_BTF ?= $(firstword $(VMLINUX_BTF_PATHS))
> >
> >  $(SKEL_OUT)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL)
> >  ifeq ($(VMLINUX_H),)
> > --
> > 2.41.0.rc0.172.g3f132b7071-goog
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-06-07  7:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-05 20:27 [PATCH v2 0/4] Bring back vmlinux.h generation Ian Rogers
2023-06-05 20:27 ` [PATCH v2 1/4] perf build: Add ability to build with a generated vmlinux.h Ian Rogers
2023-06-06  2:56   ` Jiri Olsa
2023-06-06  4:25     ` Ian Rogers
2023-06-06 18:06       ` Arnaldo Carvalho de Melo
2023-06-06 23:18         ` Ian Rogers
2023-06-06 12:26   ` kernel test robot
2023-06-05 20:27 ` [PATCH v2 2/4] perf bpf: Move the declaration of struct rq Ian Rogers
2023-06-05 20:27 ` [PATCH v2 3/4] perf test: Add build tests for BUILD_BPF_SKEL Ian Rogers
2023-06-05 20:27 ` [PATCH v2 4/4] perf build: Filter out BTF sources without a .BTF section Ian Rogers
2023-06-05 23:35   ` Namhyung Kim
2023-06-07  7:11     ` Ian Rogers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).