Re: [RFC PATCH bpf-next 3/3] selftests/bpf: build each test runner instance in its own sub-make

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Parallel build race on shared helper objects (`testing_helpers.o`, `cgroup_helpers.o`) due to overlapping rules between the parent Makefile and `Makefile.runner` sub-makes.
--

--- Patch [3]: [RFC PATCH bpf-next 3/3] selftests/bpf: build each test runner instance in its own sub-make ---
commit e06c43ef78e32188e09532ae20788ac48fd20702
Author: Mykola Lysenko <nickolay.lysenko@gmail.com>
selftests/bpf: build each test runner instance in its own sub-make

This patch refactors the BPF selftests build system by moving the
double-expansion DEFINE_TEST_RUNNER logic into independent sub-makes via a
new Makefile.runner. Shared configurations and prebuilt toolchain elements
are kept in the top-level Makefile.

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index f5c0f81f61db7..a3ab525f7dfab 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile

[ ... ]

> +HELPER_OBJS := $(addprefix $(OUTPUT)/,testing_helpers.o cgroup_helpers.o \
> +	       trace_helpers.o network_helpers.o json_writer.o		\
> +	       cap_helpers.o unpriv_helpers.o usdt_1.o usdt_2.o xsk.o)
> +
> +# $(BPFTOOL) is a prerequisite because its sub-make is what installs
> +# libbpf's internal headers (bpf/hashmap.h, bpf/libbpf_internal.h) into
> +# $(INCLUDE_DIR); without it, helpers including those headers race the
> +# install and can silently pick up the source-tree copies instead.
> +$(HELPER_OBJS): flow_dissector_load.h ip_check_defrag_frags.h		\
> +		$(VERIFY_SIG_HDR) prog_tests/tests.h $(BPFOBJ) $(BPFTOOL)

[ ... ]

> diff --git a/tools/testing/selftests/bpf/Makefile.runner b/tools/testing/selftests/bpf/Makefile.runner
> new file mode 100644
> index 0000000000000..4c58cc51f2514
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/Makefile.runner

[ ... ]

> +ifeq ($(RUNNER),test_progs)
> +EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c		\
> +		 network_helpers.c testing_helpers.c btf_helpers.c	\
> +		 cap_helpers.c unpriv_helpers.c sysctl_helpers.c	\
> +		 netlink_helpers.c jit_disasm_helpers.c io_helpers.c	\
> +		 test_loader.c xsk.c disasm.c disasm_helpers.c		\
> +		 json_writer.c bpftool_helpers.c usdt_1.c usdt_2.c

[ ... ]

> +EXTRA_OBJS := $(patsubst %.c,$(RDIR)/%.o,$(filter %.c,$(EXTRA_SOURCES)))
> +LIB_OBJS   := $(patsubst %.c,$(RDIR)/%.o,$(LIB_SOURCES))

[ ... ]

> +$(EXTRA_OBJS): $(RDIR)/%.o: %.c $(EXTRA_HDRS) $(TESTS_HDR) $(BPFOBJ) | $(RDIR)

[Severity: Medium]
Could this cause a parallel build race on the shared helper objects?

The top-level Makefile builds shared helpers like testing_helpers.o and
cgroup_helpers.o with a limited set of dependencies (shown above). However,
this EXTRA_OBJS rule inside the sub-makes encompasses those same helpers and
declares broader dependencies, such as $(LIBARENA_SKEL) for test_progs and
map_tests/tests.h for test_maps.

Because these broader dependencies are built in parallel by the top-level
make, they can end up with newer timestamps than the pre-built helpers. If
that happens, both the test_progs and test_maps sub-makes might evaluate
the helpers as out-of-date and concurrently recompile them.

Can this lead to intermittent "file truncated" or "file format not recognized"
link errors when building with -j?

> +	$(call msg,EXT-OBJ,$(BINARY),$@)
> +	$(Q)$(CC) $(CFLAGS) -c $< -o $@

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174909.8044-1-nickolay.lysenko@gmail.com?part=3