2021-01-26 11:24 UTC+0000 ~ Quentin Monnet <quentin@xxxxxxxxxxxxx> > 2021-01-25 16:32 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> >> On Mon, Jan 25, 2021 at 7:49 AM Quentin Monnet <quentin@xxxxxxxxxxxxx> wrote: >>> >>> Building the kernel with CONFIG_BPF_PRELOAD, and by providing a relative >>> path for the output directory, may fail with the following error: >>> >>> $ make O=build bindeb-pkg >>> ... >>> /.../linux/tools/scripts/Makefile.include:5: *** O=build does not exist. Stop. >>> make[7]: *** [/.../linux/kernel/bpf/preload/Makefile:9: kernel/bpf/preload/libbpf.a] Error 2 >>> make[6]: *** [/.../linux/scripts/Makefile.build:500: kernel/bpf/preload] Error 2 >>> make[5]: *** [/.../linux/scripts/Makefile.build:500: kernel/bpf] Error 2 >>> make[4]: *** [/.../linux/Makefile:1799: kernel] Error 2 >>> make[4]: *** Waiting for unfinished jobs.... >>> >>> In the case above, for the "bindeb-pkg" target, the error is produced by >>> the "dummy" check in Makefile.include, called from libbpf's Makefile. >>> This check changes directory to $(PWD) before checking for the existence >>> of $(O). But at this step we have $(PWD) pointing to "/.../linux/build", >>> and $(O) pointing to "build". So the Makefile.include tries in fact to >>> assert the existence of a directory named "/.../linux/build/build", >>> which does not exist. >>> >>> By contrast, other tools called from the main Linux Makefile get the >>> variable set to $(abspath $(objtree)), where $(objtree) is ".". We can >>> update the Makefile for kernel/bpf/preload to set $(O) to the same >>> value, to permit compiling with a relative path for output. Note that >>> apart from the Makefile.include, the variable $(O) is not used in >>> libbpf's build system. >>> >>> Note that the error does not occur for all make targets and >>> architectures combinations. >>> >>> - On x86, "make O=build vmlinux" appears to work fine. >>> $(PWD) points to "/.../linux/tools", but $(O) points to the absolute >>> path "/.../linux/build" and the test succeeds. >>> - On UML, it has been reported to fail with a message similar to the >>> above (see [0]). >>> - On x86, "make O=build bindeb-pkg" fails, as described above. >>> >>> It is unsure where the different values for $(O) and $(PWD) come from >>> (likely some recursive make with different arguments at some point), and >>> because several targets are broken, it feels safer to fix the $(O) value >>> passed to libbpf rather than to hunt down all changes to the variable. >>> >>> David Gow previously posted a slightly different version of this patch >>> as a RFC [0], two months ago or so. >>> >>> [0] https://lore.kernel.org/bpf/20201119085022.3606135-1-davidgow@xxxxxxxxxx/t/#u >>> >>> Cc: Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> >>> Cc: Brendan Higgins <brendanhiggins@xxxxxxxxxx> >>> Cc: David Gow <davidgow@xxxxxxxxxx> >>> Reported-by: David Gow <davidgow@xxxxxxxxxx> >>> Signed-off-by: Quentin Monnet <quentin@xxxxxxxxxxxxx> >>> --- >> >> I still think it would benefit everyone to figure out where this is >> breaking (given Linux Makefile explicitly tries to handle such >> relative path situation for O=, I believe), but this is trivial >> enough, so: >> >> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > Agreed, I'll try to spend a bit more time on this when I can. But it > would be nice to have the fix in the meantime. Thanks for the review and > ack. +Cc Masahiro Yamada Looking further into this, my understanding is the following. tools/scripts/Makefile.include contains this check: dummy := $(if $(shell cd $(PWD); test -d $(O) || \ echo $(O)),$(error O=$(O) does not exist),) ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) Note the use of $(PWD). As I understand, it is the shell environment variable, as it was set when the initial "make" command was run. This seems to be passed down to recursive calls to make. So if I type $ cd /linux $ make O=build vmlinux Then I get $(PWD) set to "/linux" and $(O) set to "build". The Makefile executes a submake from the output directory: # Invoke a second make in the output directory, passing relevant # variables __sub-make: $(Q)$(MAKE) -C $(abs_objtree) \ -f $(abs_srctree)/Makefile $(MAKECMDGOALS) But the variables are preserved. So far, so good. When I try to build "bindeb-pkg" instead: $ cd /linux $ make O=build bindeb-pkg Then I initially set $(PWD) and $(O) to the same values. They are preserved after the call to the submake, after we have changed to the output directory. But if I understand correctly, the "bindeb-pkg" target writes a new Makefile as build/debian/rules in scripts/package/mkdebian, and calls it _indirectly_ through dpkg-buildpackage, which does _not_ preserve $(PWD) (instead, it is reset to /linux/build, the current directory when calling the script). I end up with $(O) set to "build", and $(PWD) set to "/linux/build". The "dummy" check called for libbpf fails to find "/linux/build/build". Can we avoid using $(PWD) in the first place? I'm not sure how. It was added in commit be40920fbf10 ("tools: Let O= makes handle a relative path with -C option") to accommodate building perf with "-C", so we could not replace $(PWD) with $$PWD (shell value at the time the directive is executed) for example, the values will be different. Can we unset $(O) so that, when we call dpkg-buildpackage, it reflects the output directory relatively to /linux/build/? Or pass a value for $(PWD), so it is preserved? Maybe, I don't know. It could be done in scripts/package/mkdebian for example, by passing "O=''" to the make call. It seems that the packages build well with this change. But then we might need to check and update the other packages too (RPM, snap, perf archives), and identify if something similar might be happening for UML. I'm not sure this is worth the trouble at this point, if all we want is to fix the eBPF preloads? But I'm open to discussion if this is really the path we want to go. Fixing $(O) to pass the dummy check is easier. However, when reading the commits I noticed that my patch is incorrect. It would break on something like "make O=~/build bindeb-pkg", because abspath does not resolve special shell characters like "~". See also commit 028568d84da3 ("kbuild: revert $(realpath ...) to $(shell cd ... && /bin/pwd)"): this is why tools/scripts/Makefile.include still has an "ABSOLUTE_O" variable. So instead of setting "O=$(abspath .)", I'll send a v2 with Andrii's suggestion to use $(LIBBPF_OUT). Quentin
