On Wed, 2024-07-17 at 00:36 +0000, Ihor Solodrai wrote: > On Tuesday, July 16th, 2024 at 4:21 PM, Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > [...] > > > The catch-all clause in the current makefile looks as follows: > > > > $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o: \ > > $(TRUNNER_BPF_PROGS_DIR)/%.c \ > > $(TRUNNER_BPF_PROGS_DIR)/*.h \ > > ... > > > > This makes all .bpf.o files dependent on all BPF .c files. > > .bpf.o files rebuild is the main time consumer, at-least for me. > > I might be nit-picking, but just so it's clear this target makes each > individual %.bpf.o dependent on corresponding progs/%.c, and also on > all the headers (because of *.h). On current master touch progs/verifier_and.c leads to complete rebuild of all bpf.o and test.o files. And here is one of the targets: $ make test_progs -p | grep tools/testing/selftests/bpf/sockmap_listen.test.o: | tr ' ' '\n' | head /home/eddy/work/bpf-next/tools/testing/selftests/bpf/sockmap_listen.test.o: prog_tests/sockmap_listen.c flow_dissector_load.h ip_check_defrag_frags.h /home/eddy/work/bpf-next/tools/testing/selftests/bpf/access_map_in_map.bpf.o /home/eddy/work/bpf-next/tools/testing/selftests/bpf/arena_atomics.bpf.o /home/eddy/work/bpf-next/tools/testing/selftests/bpf/arena_htab_asm.bpf.o /home/eddy/work/bpf-next/tools/testing/selftests/bpf/arena_htab.bpf.o /home/eddy/work/bpf-next/tools/testing/selftests/bpf/arena_list.bpf.o /home/eddy/work/bpf-next/tools/testing/selftests/bpf/async_stack_depth.bpf.o 17:57:16 bpf$ make test_progs -p | grep tools/testing/selftests/bpf/sockmap_listen.test.o: | tr ' ' '\n' | grep bpf\.o | wc -l 760 > I don't think we can easily remove/replace the $(TRUNNER_BPF_OBJS) > target, as it also defines a compilation recipe for .bpf.o files. I don't think removing $(TRUNNER_BPF_OBJS) was suggested in the thread. > > I think that simply replacing this catch all by something like: > > > > $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_BPF_OBJS) > > > > Using a catch-all dependency (each %.test.o depends on *all* .bpf.o) > is the current state of the Makefile, without the auto-dependencies > patch: > > $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ > $(TRUNNER_TESTS_DIR)/%.c \ > $(TRUNNER_EXTRA_HDRS) \ > $(TRUNNER_BPF_OBJS) \ # this line > $(TRUNNER_BPF_SKELS) \ > ... Yes, sure, but this is bad, because it forces rebuild of all .test.o files. Having auto-dependencies allows to get rid of this. > In the v3 of the patch this dependency simply moved, such that each > %.test.d file depends on *all* %.bpf.o, so essentially nothing changed > there. > > So what we've been discussing so far is whether it's worth spending > effort on removing/replacing this dependency, and how. > > Eduard's point about simplification of test cases seems reasonable. > However it looks to me that whatever way of handling direct .bpf.o > dependencies we might agree on, it would lead to an independent patch > series. > > And settling on catch-all solution (even "for now") means settling on > v3 of the patch. I don't like .test.d dependency on all .bpf.o files (the v3 change) as it does not encode the dependency we have: test_progs depends on core_reloc.bpf.o at runtime.
