Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jiri Olsa <olsajiri@gmail.com>
Date:   Tue, 6 Jun 2023 04:56:38 +0200
To:     Ian Rogers <irogers@google.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Adrian Hunter <adrian.hunter@intel.com>,
        James Clark <james.clark@arm.com>,
        Tiezhu Yang <yangtiezhu@loongson.cn>,
        Yang Jihong <yangjihong1@huawei.com>,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
        bpf@vger.kernel.org, Andrii Nakryiko <andrii@kernel.org>
Subject: Re: [PATCH v2 1/4] perf build: Add ability to build with a generated
 vmlinux.h
Message-ID: <ZH6gZgcwAbDrEiqX@krava>
References: <20230605202712.1690876-1-irogers@google.com>
 <20230605202712.1690876-2-irogers@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230605202712.1690876-2-irogers@google.com>
Precedence: bulk

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
>