On Mon, Jun 5, 2023 at 7:57 PM Jiri Olsa <olsajiri@xxxxxxxxx> 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@xxxxxxxxxx>
> > Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > ---
> > 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
> >
