Re: [PATCH bpf-next v4] selftests/bpf: use auto-dependencies for test objects

Ihor Solodrai
2024-09-15T04:47:47Z

On Saturday, September 14th, 2024 at 3:54 AM, Björn Töpel <bjorn@kernel.org> wrote:

[...]

> >
> > Could you please check on your side if this helps? Maybe there are
> > other issues.
>
>
> I don't even get that far in the "install" target. When I revert the
> patch, I get to the issue you describe above (trying to install
> non-existing file).
>
> Here's an excerpt from a failed "install":
>
> | BINARY test_progs
> | BINARY test_progs-no_alu32
> | BINARY test_progs-cpuv4
> | make[1]: Leaving directory '/home/bjorn/src/linux/linux/tools/testing/selftests/bpf'
>
> At this point the "all" target is complete; All good, and the "install"
> is started.
>
> | mkdir -p /home/bjorn/output/foo/kselftest/kselftest
> | install -m 744 kselftest/module.sh /home/bjorn/output/foo/kselftest/kselftest/
> | install -m 744 kselftest/runner.sh /home/bjorn/output/foo/kselftest/kselftest/
> | install -m 744 kselftest/prefix.pl /home/bjorn/output/foo/kselftest/kselftest/
> | install -m 744 kselftest/ktap_helpers.sh /home/bjorn/output/foo/kselftest/kselftest/
> | install -m 744 kselftest/ksft.py /home/bjorn/output/foo/kselftest/kselftest/
> | install -m 744 run_kselftest.sh /home/bjorn/output/foo/kselftest/
> | rm -f /home/bjorn/output/foo/kselftest/kselftest-list.txt
>
> This is from the top-level "tools/testing/selftests/Makefile", and we
> enter the BPF directory for "install".
>
> | make[1]: Entering directory '/home/bjorn/src/linux/linux/tools/testing/selftests/bpf'
> |
> | Auto-detecting system features:
> | ... llvm: [ OFF ]
> |
> | LINK-BPF [test_progs] test_static_linked.bpf.o
> | LINK-BPF [test_progs] linked_funcs.bpf.o
> | LINK-BPF [test_progs] linked_vars.bpf.o
> | LINK-BPF [test_progs] linked_maps.bpf.o
> | LINK-BPF [test_progs] test_subskeleton.bpf.o
> | LINK-BPF [test_progs] test_subskeleton_lib.bpf.o
> | ...
> | EXT-COPY [test_maps]
> | make[1]: *** No rule to make target 'atomics.lskel.h', needed by '/home/bjorn/output/foo/kselftest/bpf/atomics.test.o'. Stop.
> | make[1]: *** Waiting for unfinished jobs....
> | make[1]: Leaving directory '/home/bjorn/src/linux/linux/tools/testing/selftests/bpf'
> | make: *** [Makefile:259: install] Error 2
> | make: Leaving directory '/home/bjorn/src/linux/linux/tools/testing/selftests'
>
> ...and for some reason the auto-dependencies decides to re-build, and
> fails.
>
> So, unfortunately it's something else related to your patch.

Björn,

I think I figured out the cause of the issue, but some details and a
proper solution are unclear yet.

In generated %.test.d makefiles some dependencies (in particular
%.skel.h) are referred to by filename as opposed to full path. For
example:

$ cat /output/foo/kselftest/bpf/cpuv4/atomics.test.d
/output/foo/kselftest/bpf/cpuv4/atomics.test.o: \
/opt/linux/tools/testing/selftests/bpf/prog_tests/atomics.c \
[...]
/opt/linux/tools/testing/selftests/bpf/trace_helpers.h \
/opt/linux/tools/testing/selftests/bpf/testing_helpers.h atomics.lskel.h \ # <-- THIS
/output/foo/kselftest/bpf/tools/include/bpf/skel_internal.h \
/output/foo/kselftest/bpf/tools/include/bpf/bpf.h

This is of course a problem, because make needs to know how to build a
target with this exact name. And in the patch it was (partially)
solved by this piece:

+# When the compiler generates a %.d file, only skel basenames (not
+# full paths) are specified as prerequisites for corresponding %.o
+# file. This target makes %.skel.h basename dependent on full paths,
+# linking generated %.d dependency with actual %.skel.h files.
+$(notdir %.skel.h): $(TRUNNER_OUTPUT)/%.skel.h
+ @true

This links %.skel.h to /output/foo/kselftest/bpf/no_alu32/%.skel.h and alike.

Your build is breaking because there is no such rule for
%.lskel.h and %.subskel.h, which are trivial to add:

--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -628,6 +628,12 @@ $(TRUNNER_BPF_SKELS_LINKED): $(TRUNNER_OUTPUT)/%: $$$$(%-deps) $(BPFTOOL) | $(TR
$(notdir %.skel.h): $(TRUNNER_OUTPUT)/%.skel.h
@true

+$(notdir %.lskel.h): $(TRUNNER_OUTPUT)/%.lskel.h
+ @true
+
+$(notdir %.subskel.h): $(TRUNNER_OUTPUT)/%.subskel.h
+ @true
+
endif

With this change a command below completed for me in the environment
you shared:

$ make -j \$((\$(nproc)-1)) O=/output/foo TARGETS=bpf SKIP_TARGETS="" KSFT_INSTALL_PATH=/output/foo/kselftest -C tools/testing/selftests install

What is a mystery to me is why this was not an issue for simple `make
-C tool/testing/selftests/bpf`. And also why in the environment I
tried yesterday I didn't get the failure you're seeing.

Do you happen to have a Dockerfile of ghcr.io/linux-riscv/pw-builder ?
If possible, I'd like to look at it and compare with my local
environment.

The other issue that I'll have to think about is the unnecessary
re-builds that you've noticed. I suspect this happens due to the same
reason: a generated dependency on X.skel.h can't find a file in
current directory (because it was put to /output/foo), and so rebuild
is triggered. I'll have to come up with a workaround.

Björn, please try a change suggested above and let me know if it helps.

I will investigate these problems more, and there will definitely be a
follow up patch.

Thank you for reporting.

>
> A reproducer:
> | $ docker run --rm -it --volume ${PWD}:/build/my-linux ghcr.io/linux-riscv/pw-builder bash -l
> | # cd /build/my-linux
> | # cat > ./build.sh <<EOF
>
> | #!/bin/bash
> | set -euo pipefail
> |
> | rm -rf /output/foo
> | mkdir -p /output/foo
> |
> | export PATH=\$(echo \$PATH | tr : "\n"| grep -v ^/opt | tr "\n" :)
> |
> | make -j \$((\$(nproc)-1)) O=/output/foo defconfig
> | make -j \$((\$(nproc)-1)) O=/output/foo kselftest-merge
> | make -j \$((\$(nproc)-1)) O=/output/foo
> | make -j \$((\$(nproc)-1)) O=/output/foo headers
> | make -j \$((\$(nproc)-1)) O=/output/foo TARGETS=bpf SKIP_TARGETS="" KSFT_INSTALL_PATH=/output/foo/kselftest -C tools/testing/selftests install
> | EOF
> |
> | # chmod +x ./build.sh
> | # ./build.sh

And thank you for a reproducer, very helpful.