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

Thanks for the detailed review and sorry for the delayed reply! Had to
interrupt this adventure for a bit.

On Thu, Aug 6, 2026 at 1:02 AM Eduard Zingerman <[email protected]> wrote:
>
> On Tue, 2026-08-04 at 10:01 -0700, Mykola Lysenko wrote:
>
> ...
>
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 58c567a22..af3709d51 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
>
> ...
>
> > @@ -262,6 +249,35 @@ $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> >  $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS) $(UNPRIV_HELPERS)
> >  $(OUTPUT)/xsk.o: $(BPFOBJ)
> >
> > +# All helper objects a runner instance links, pre-built here so runner
> > +# sub-makes sharing $(OUTPUT) never compile them concurrently.
> > +HELPER_OBJS := $(TESTING_HELPERS) $(CGROUP_HELPERS) $(UNPRIV_HELPERS)        \
> > +            $(TRACE_HELPERS) $(JSON_WRITER) $(CAP_HELPERS)           \
> > +            $(NETWORK_HELPERS) $(OUTPUT)/usdt_1.o $(OUTPUT)/usdt_2.o \
> > +            $(OUTPUT)/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.
> > +#
> > +# Invariant: this list must be a superset of every prerequisite the
> > +# runner's EXTRA_OBJS rule attaches to these same objects (EXTRA_HDRS,
> > +# both tests.h flavors, $(BPFOBJ)). It guarantees the helpers built
> > +# here are never older than a runner-side prerequisite, so the
> > +# unflavored test_progs and test_maps sub-makes, which share $(OUTPUT),
> > +# treat them as up to date instead of both recompiling the same object
> > +# file concurrently.
> > +$(HELPER_OBJS): flow_dissector_load.h ip_check_defrag_frags.h                \
> > +             $(VERIFY_SIG_HDR) $(LIBARENA_SKEL) $(LIBARENA_ASAN_SKEL)\
> > +             prog_tests/tests.h map_tests/tests.h $(BPFOBJ) $(BPFTOOL)
> > +
>
> Tbh, I don't understand what this comment is attempting to convey and
> maintaining the invariant would be taxing going forward. LLM is also
> trying to convince me that several dependencies are missing from the
> list:
>
>   test_progs.o btf_helpers.o sysctl_helpers.o netlink_helpers.o
>   jit_disasm_helpers.o io_helpers.o test_loader.o disasm.o
>   disasm_helpers.o bpftool_helpers.o find_bit.o
>
> Which would form a race when compiling test_progs and test_maps that
> both share the same $(OUTPUT) directory. This seem plausible.

For the record, that race does not exist: the test_maps sub-make
compiles only test_maps.o and testing_helpers.o (and testing_helpers.o
is in HELPER_OBJS, pre-built), while each of the eleven objects above
has exactly one builder - the unflavored test_progs sub-make.
find_bit.c never becomes $(OUTPUT)/find_bit.o outside it either:
xskxceiver compiles it directly from source on the link line.
HELPER_OBJS was exactly the multi-writer set - objects the top
Makefile's own binaries (test_verifier etc.) share with the runners.

That said, I agree the arrangement was too subtle to live: it took
the paragraph you quoted to explain, and the invariant is the kind of
thing that silently rots.

>
> How about a change as in the attached patch? (squashed with this patch).
> It sidesteps a previously discussed constraint to keep refactoring to
> the minimum, but HELPER_OBJS is just too ugly to pass.

Applied your patch to v6, thank you - it is clearly better: one
builder for every user space object by construction, no invariant,
and the flavored instances stop compiling identical copies of the
shared objects.

>
> > +$(OUTPUT)/flow_dissector_load: flow_dissector_load.h
> > +
> > +# Tool sub-builds: bpftool, libbpf (host and, when cross-compiling,
> > +# target variants) and resolve_btfids. Deliberate asymmetry: the host
> > +# libbpf builds without sanitizers and with the host toolchain.
>
> Nit: is this comment necessary?

Dropped in v6.

>
> >  $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)    \
> >                   $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
> >       $(Q)$(MAKE) $(submake_extras)  -C $(BPFTOOLDIR)                        \
>
> ...
>
> > @@ -672,81 +413,125 @@ $(LIBARENA_ASAN_SKEL): $(INCLUDE_DIR)/vmlinux.h $(BPFOBJ) $(LIBARENA_BPF_DEPS)
>
> ...
>
> > +# Everything a runner instance references but does not know how to build.
> > +RUNNER_PREREQS := $(INCLUDE_DIR)/vmlinux.h $(BPFOBJ) $(BPFTOOL)              \
> > +               $(TRUNNER_BPFTOOL) $(RESOLVE_BTFIDS)                  \
> > +               $(OUTPUT)/veristat                                    \
> > +               $(VERIFY_SIG_HDR) $(PRIVATE_KEY) $(VERIFICATION_CERT) \
> > +               $(LIBARENA_SKEL) $(LIBARENA_ASAN_SKEL)                \
> > +               prog_tests/tests.h map_tests/tests.h                  \
> > +               $(HELPER_OBJS)                                        \
> > +               $(OUTPUT)/urandom_read $(OUTPUT)/liburandom_read.so   \
> > +               $(OUTPUT)/xdp_synproxy $(OUTPUT)/sign-file            \
> > +               $(OUTPUT)/uprobe_multi $(TEST_KMOD_TARGETS)
> > +
> > +FORCE:
>
> Nit: please drop a line here, saying that the main makefile does not
>      know if $(OUTPUT)/test_progs is stale, hence FORCE to force
>      delegation to Makefile.runner.

Done in v6, essentially in your words.

>
> > +
> > +# The default flavor's skeletons are also inputs of bench, test_maps,
> > +# xskxceiver, xdp_* and test_cpp; "default-skels" builds just those
> > +# without the test_progs test objects and binary.
> > +.PHONY: default-skels
> > +default-skels: $(RUNNER_PREREQS)
> > +     +$(Q)$(RUNNER_MAKE) RUNNER=test_progs FLAVOR= TESTS_DIR=prog_tests \
> > +             BPF_CC='$(CLANG)' BPF_CC_MSG=CLNG-BPF BPF_SYS_INCLUDES='$(CLANG_SYS_INCLUDES)'  \
> > +             BPF_CC_FLAGS='-O2 $(BPF_TARGET_ENDIAN) -mcpu=v3'                \
> > +             BPF_DEFINES=-DENABLE_ATOMICS_TESTS skels
> > +
> > +$(OUTPUT)/test_progs: $(RUNNER_PREREQS) default-skels FORCE
> > +     +$(Q)$(RUNNER_MAKE) RUNNER=test_progs FLAVOR= TESTS_DIR=prog_tests \
> > +             BPF_CC='$(CLANG)' BPF_CC_MSG=CLNG-BPF BPF_SYS_INCLUDES='$(CLANG_SYS_INCLUDES)'  \
> > +             BPF_CC_FLAGS='-O2 $(BPF_TARGET_ENDIAN) -mcpu=v3'                \
> > +             BPF_DEFINES=-DENABLE_ATOMICS_TESTS
>
> Is there a way to reduce copy-paste between default-skels and
> test_progs? Why does test_progs depend on default-skels?

The dependency existed for serialization: under -j, bench and friends
reached the default flavor's skeletons through the default-skels
sub-make while the test_progs recipe launched its own sub-make that
also builds them - two make instances writing the same files
concurrently; the dependency made default-skels complete first.

Both the duplication and the serialization trick are gone in v6
though: following your second mail, the skeleton rules moved into
Makefile.skel and the main Makefile builds the default flavor's
artifacts itself - default-skels no longer exists. Details in the
reply there.

>
> > +
> > +$(OUTPUT)/test_progs-no_alu32: $(RUNNER_PREREQS) FORCE
> > +     +$(Q)$(RUNNER_MAKE) RUNNER=test_progs FLAVOR=no_alu32 TESTS_DIR=prog_tests \
> > +             BPF_CC='$(CLANG)' BPF_CC_MSG=CLNG-BPF BPF_SYS_INCLUDES='$(CLANG_SYS_INCLUDES)'  \
> > +             BPF_CC_FLAGS='-O2 $(BPF_TARGET_ENDIAN) -mcpu=v2'
> > +
> >  ifneq ($(CLANG_CPUV4),)
> > -TRUNNER_BPF_BUILD_RULE := CLANG_CPUV4_BPF_BUILD_RULE
> > -TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) -DENABLE_ATOMICS_TESTS
> > -$(eval $(call DEFINE_TEST_RUNNER,test_progs,cpuv4))
> > +$(OUTPUT)/test_progs-cpuv4: $(RUNNER_PREREQS) FORCE
> > +     +$(Q)$(RUNNER_MAKE) RUNNER=test_progs FLAVOR=cpuv4 TESTS_DIR=prog_tests \
> > +             BPF_CC='$(CLANG)' BPF_CC_MSG=CLNG-BPF BPF_SYS_INCLUDES='$(CLANG_SYS_INCLUDES)'  \
> > +             BPF_CC_FLAGS='-O2 $(BPF_TARGET_ENDIAN) -mcpu=v4'                \
> > +             BPF_DEFINES=-DENABLE_ATOMICS_TESTS
> >  endif
> >
> > -# Define test_progs BPF-GCC-flavored test runner.
> >  ifneq ($(BPF_GCC),)
> > -TRUNNER_BPF_BUILD_RULE := GCC_BPF_BUILD_RULE
> > -TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(call get_sys_includes,gcc,)
> > -$(eval $(call DEFINE_TEST_RUNNER,test_progs,bpf_gcc))
> > +# Sys includes come from the *host* gcc, not $(BPF_GCC) - bpf-gcc's own
> > +# search path lacks the host's asm-generic/ headers.
> > +GCC_SYS_INCLUDES := $(call get_sys_includes,gcc,)
> > +
> > +$(OUTPUT)/test_progs-bpf_gcc: $(RUNNER_PREREQS) FORCE
> > +     +$(Q)$(RUNNER_MAKE) RUNNER=test_progs FLAVOR=bpf_gcc TESTS_DIR=prog_tests \
> > +             BPF_CC='$(BPF_GCC)' BPF_CC_MSG=GCC-BPF                          \
> > +             BPF_SYS_INCLUDES='$(GCC_SYS_INCLUDES)'                          \
> > +             BPF_CC_FLAGS='-DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2'
> >  endif
>
> Did you test the bpf_gcc flavor?
> I have a recipe to build the toolchain somewhere, if you need it.

Two data points. The BPF CI GCC-BPF job builds the flavor and is
green on this series. Locally I ran it with the bpf-gcc Ubuntu
packages (gcc 14 snapshot, 2024): the machinery works - 671 of 982
programs compile, the ones that old gcc cannot handle skip cleanly
under BPF_STRICT_BUILD=0, skeletons generate or skip with the
objects - until resolve_btfids aborts on btf_data.bpf.o carrying no
.BTF section, which that gcc does not emit; the BTFIDS recipe step is
not covered by the permissive suffix in the current Makefile either,
so the behavior matches stock. I would take your toolchain recipe -
with a current gcc-bpf I expect the flavor to complete, and I'd like
it in my pre-send checklist.

>
> >
> > -# Define test_maps test runner.
> > -TRUNNER_TESTS_DIR := map_tests
> > -TRUNNER_BPF_PROGS_DIR := progs
> > -TRUNNER_EXTRA_SOURCES := test_maps.c
> > -TRUNNER_LIB_SOURCES :=
> > -TRUNNER_EXTRA_FILES :=
> > -TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built)
> > -TRUNNER_BPF_CFLAGS :=
> > -$(eval $(call DEFINE_TEST_RUNNER,test_maps))
> > -
> > -# Define test_verifier test runner.
> > -# It is much simpler than test_maps/test_progs and sufficiently different from
> > -# them (e.g., test.h is using completely pattern), that it's worth just
> > -# explicitly defining all the rules explicitly.
> > +# test_maps compiles map_tests/*.c against the default flavor's skeletons
> > +$(OUTPUT)/test_maps: $(RUNNER_PREREQS) map_tests/tests.h default-skels FORCE
> > +     +$(Q)$(RUNNER_MAKE) RUNNER=test_maps FLAVOR= TESTS_DIR=map_tests
> > +
> > +# Default-flavor BPF objects and skeletons consumed by targets in this
> > +# Makefile (and by lib.mk's install rule) are produced by the runner
> > +# sub-make above; the empty recipe plus the order-only prerequisite
> > +# makes them available here without duplicating the runner's rules.
> > +DEFAULT_BPF_OBJS := $(patsubst progs/%.c,$(OUTPUT)/%.bpf.o,$(wildcard progs/*.c))
> > +TEST_GEN_FILES += $(DEFAULT_BPF_OBJS)
>
> --- 8< --------------------------------------------------------------------------
>
> > +BENCH_SKELS := $(addprefix $(OUTPUT)/,                                       \
> > +            test_overhead.skel.h trigger_bench.skel.h                \
> > +            ringbuf_bench.skel.h perfbuf_bench.skel.h                \
> > +            bloom_filter_bench.skel.h bpf_loop_bench.skel.h          \
> > +            strncmp_bench.skel.h bpf_hashmap_full_update_bench.skel.h\
> > +            local_storage_bench.skel.h                               \
> > +            local_storage_rcu_tasks_trace_bench.skel.h               \
> > +            bench_local_storage_create.skel.h                        \
> > +            bpf_hashmap_lookup.skel.h htab_mem_bench.skel.h          \
> > +            crypto_bench.skel.h bench_sockmap_prog.skel.h            \
> > +            lpm_trie_bench.skel.h lpm_trie_map.skel.h                \
> > +            bpf_nop_bench.skel.h xdp_lb_bench.skel.h)
> > +TOP_SKELS := $(addprefix $(OUTPUT)/,xsk_xdp_progs.skel.h             \
> > +          xdp_hw_metadata.skel.h xdp_features.skel.h                 \
> > +          test_core_extern.skel.h) $(BENCH_SKELS)
> > +
> > +# Empty-recipe delegation: "default-skels" (re)builds the file via the
> > +# runner sub-make, after which make re-stats the target, so dependents
> > +# see fresh timestamps within the same invocation. Dependents still
> > +# rebuild on mtime only - the phony prerequisite does not cascade.
> > +$(DEFAULT_BPF_OBJS) $(TOP_SKELS): default-skels ;
>
> -------------------------------------------------------------------------- >8 ---
>
> Note, the above two lists can be replaced with the following two rules:
>
>   $(OUTPUT)/%.skel.h: default-skels ;
>   $(DEFAULT_BPF_OBJS): default-skels ;

I verified this works (a no-op build stays a no-op, cascades and
re-stat behave, nothing becomes an intermediate file for make to
delete), but it ended up moot: with Makefile.skel from your second
mail the delegation rules are gone entirely - the main Makefile owns
the default flavor's skeleton rules and the hand lists disappear with
them. Details in the reply there.

>
> > +
> > +# Standalone test binaries
> > +
> > +# test_verifier is much simpler than test_maps/test_progs and
> > +# sufficiently different from them (e.g., its tests.h uses a completely
> > +# different pattern), that it's worth just defining its rules explicitly.
> >  verifier/tests.h: verifier/*.c
> >       $(Q)( cd verifier/; \
> >                 echo '/* Generated header, do not edit */'; \
>
> ...