Thread (11 messages) 11 messages, 4 authors, 2021-02-04

Re: [PATCH] bpf: Hoise pahole version checks into Kconfig

From: Nathan Chancellor <hidden>
Date: 2021-01-11 19:34:46
Also in: bpf, linux-kbuild, lkml

On Tue, Jan 12, 2021 at 04:19:01AM +0900, Masahiro Yamada wrote:
On Tue, Jan 12, 2021 at 3:06 AM Nathan Chancellor
[off-list ref] wrote:
quoted
After commit da5fb18225b4 ("bpf: Support pre-2.25-binutils objcopy for
vmlinux BTF"), having CONFIG_DEBUG_INFO_BTF enabled but lacking a valid
copy of pahole results in a kernel that will fully compile but fail to
link. The user then has to either install pahole or disable
CONFIG_DEBUG_INFO_BTF and rebuild the kernel but only after their build
has failed, which could have been a significant amount of time depending
on the hardware.

Avoid a poor user experience and require pahole to be installed with an
appropriate version to select and use CONFIG_DEBUG_INFO_BTF, which is
standard for options that require a specific tools version.

Suggested-by: Sedat Dilek <redacted>
Signed-off-by: Nathan Chancellor <redacted>


I am not sure if this is the right direction.


I used to believe moving any tool test to the Kconfig
was the right thing to do.

For example, I tried to move the libelf test to Kconfig,
and make STACK_VALIDATION depend on it.

https://patchwork.kernel.org/project/linux-kbuild/patch/1531186516-15764-1-git-send-email-yamada.masahiro@socionext.com/

It was rejected.


In my understanding, it is good to test target toolchains
in Kconfig (e.g. cc-option, ld-option, etc).

As for host tools, in contrast, it is better to _intentionally_
break the build in order to let users know that something needed is missing.
Then, they will install necessary tools or libraries.
It is just a one-time setup, in most cases,
just running 'apt install' or 'dnf install'.



Recently, a similar thing happened to GCC_PLUGINS
https://patchwork.kernel.org/project/linux-kbuild/patch/20201203125700.161354-1-masahiroy@kernel.org/#23855673




Following this pattern, if a new pahole is not installed,
it might be better to break the build instead of hiding
the CONFIG option.

In my case, it is just a matter of 'apt install pahole'.
On some distributions, the bundled pahole is not new enough,
and people may end up with building pahole from the source code.
This is fair enough. However, I think that parts of this patch could
still be salvaged into something that fits this by making it so that if
pahole is not installed (CONFIG_PAHOLE_VERSION=0) or too old, the build
errors at the beginning, rather at the end. I am not sure where the best
place to put that check would be though.

Cheers,
Nathan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help