Re: [PATCH bpf-next 2/2] selftests/bpf: do not update vmlinux.h unnecessarily

[Alan Maguire <alan.maguire@oracle.com>]

On 30/08/2024 21:34, Andrii Nakryiko wrote:
> On Wed, Aug 28, 2024 at 3:02 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>>
>> On Wed, 2024-08-28 at 17:46 +0000, Ihor Solodrai wrote:
>>> %.bpf.o objects depend on vmlinux.h, which makes them transitively
>>> dependent on unnecessary libbpf headers. However vmlinux.h doesn't
>>> actually change as often.
>>>
>>> When generating vmlinux.h, compare it to a previous version and update
>>> it only if there are changes.
>>>
>>> Example of build time improvement (after first clean build):
>>>   $ touch ../../../lib/bpf/bpf.h
>>>   $ time make -j8
>>> Before: real  1m37.592s
>>> After:  real  0m27.310s
>>>
>>> Notice that %.bpf.o gen step is skipped if vmlinux.h hasn't changed.
>>>
>>> Link: https://lore.kernel.org/bpf/CAEf4BzY1z5cC7BKye8=A8aTVxpsCzD=p1jdTfKC7i0XVuYoHUQ@mail.gmail.com
>>>
>>> Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>
>>> ---
>>
>> Unfortunately, I think that this is a half-measure.
>> E.g. the following command forces tests rebuild for me:
>>
>>   touch ../../../../kernel/bpf/verifier.c; \
>>   make -j22 -C ../../../../; \
>>   time make test_progs
>>
>> To workaround this we need to enable reproducible_build option:
>>
>>     diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf
>>     index b75f09f3f424..8cd648f3e32b 100644
>>     --- a/scripts/Makefile.btf
>>     +++ b/scripts/Makefile.btf
>>     @@ -19,7 +19,7 @@ pahole-flags-$(call test-ge, $(pahole-ver), 125)      += --skip_encoding_btf_inconsis
>>      else
>>
>>      # Switch to using --btf_features for v1.26 and later.
>>     -pahole-flags-$(call test-ge, $(pahole-ver), 126)  = -j --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs
>>     +pahole-flags-$(call test-ge, $(pahole-ver), 126)  = -j --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs,reproducible_build
>>
>>      ifneq ($(KBUILD_EXTMOD),)
>>      module-pahole-flags-$(call test-ge, $(pahole-ver), 126) += --btf_features=distilled_base
>>
>> Question to the mailing list: do we want this?
> 
> Alan, can you please give us a summary of what are the consequences of
> the reproducible_build pahole option? In terms of performance and
> otherwise.
>

Sure. The original context was that the folks trying to do reproducible
builds were being impacted by the fact that BTF generation was
non-deterministic when done in parallel; i.e. same kernel would give
different BTF ids when rebuilding vmlinux BTF; the reason was largely as
I understand it that when pahole partitioned CUs between multiple
threads, that partitioning could vary. If it varied, when BTF was merged
across threads we could end up with differing id assignments. Since BTF
then was baked into the vmlinux binary, unstable BTF ids meant
non-identical vmlinux.

The first approach to solve this was to remove parallel BTF generation
to support reproducibility. Arnaldo however added support that retained
parallelism while supporting determinism through using the DWARF CU
order. He did some great analysis on the overheads for vmlinux
generation too [1]; summary is that the overhead in runtime is approx
33% versus parallel non-reproducible encoding. Those numbers might not
100% translate to the vmlinux build during kernel since it was a
detached pahole generation and the options might differ slightly, but
they give a sense of things. I don't _think_ there should be additional
memory overheads during pahole generation (we really can't afford any
more memory usage), since it's really more about making order of CU
processing consistent.

Would be good to get Arnaldo's perspective too if we're considering
switching this on by default, as he knows this stuff much better than I do.

[1]
https://lore.kernel.org/dwarves/20240412211604.789632-12-acme@kernel.org/