Re: [PATCH v2 08/18] perf build: Move BPF skeleton generation out of Makefile.perf

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The `bpf-skel-clean` target does not clean intermediate `.bpf.o` files generated for benchmark skeletons.
- [Low] The `bpf_skel_deps` variable is defined but never utilized.
--

commit 82e063c8cb877e030e9f5ae7b0984031f34bbb1e
Author: Ian Rogers <irogers@google.com>

perf build: Move BPF skeleton generation out of Makefile.perf

This refactors the BPF skeleton generation to better achieve directory
isolation. It moves the generation infrastructure out of Makefile.perf into a
shared bpf_skel.mak, and adjusts local prerequisites.

> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 24581941e912e..d4fc10f36781f 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf

[ ... ]

>  bpf-skel-clean:
> -	$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS) $(SKEL_OUT)/vmlinux.h
> +	$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKEL_OUT)/*.skel.h $(SKEL_OUT)/vmlinux.h $(OUTPUT)bench/bpf_skel/*.skel.h

Does the bpf-skel-clean target leave behind intermediate .bpf.o files
for the benchmark skeletons?

Since bench_uprobe.bpf.c compilation was moved out of util/bpf_skel/.tmp/
and directly into bench/bpf_skel/.tmp/, it seems the updated clean target
only removes $(SKEL_TMP_OUT) (which resolves to util/bpf_skel/.tmp) and
the .skel.h files. It appears to miss the new .tmp directory in the bench/
tree where Kbuild outputs the intermediate .bpf.o artifacts, which could
leave files on disk after a clean.

I noticed this was addressed in a later commit ("perf build: Encapsulate
vmlinux.h and bpftool in bpf_skel.mak").

[ ... ]

> diff --git a/tools/perf/bpf_skel.mak b/tools/perf/bpf_skel.mak
> new file mode 100644
> index 0000000000000..bcb704d5af32f
> --- /dev/null
> +++ b/tools/perf/bpf_skel.mak

[ ... ]

> +ifeq ($(CONFIG_PERF_BPF_SKEL),y)
> +BPFTOOL := $(SKEL_TOOL_TMP_OUT)/bootstrap/bpftool
> +VMLINUX_H := $(SKEL_TOOL_OUT)/vmlinux.h
> +bpf_skel_deps := $(BPFTOOL) $(VMLINUX_H)

Is bpf_skel_deps used anywhere after being defined?

It looks like the pattern rules duplicate $(BPFTOOL) and $(VMLINUX_H) as
explicit prerequisites instead, leaving bpf_skel_deps unused.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512174638.120445-1-irogers@google.com?part=8