Re: [PATCH v2 bpf-next] kbuild,bpf: switch to using --btf_features for pahole v1.26 and later

* Re: [PATCH v2 bpf-next] kbuild,bpf: switch to using --btf_features for pahole v1.26 and later
       [not found] ` <CAEf4BzbWANm+Bf63hcFAB3Tn51tOeBLhyabV3NNz8tjaMnThjg@mail.gmail.com>
@ 2024-05-09  8:18   ` Alan Maguire
  2024-05-09 22:01     ` Andrii Nakryiko
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Maguire @ 2024-05-09  8:18 UTC (permalink / raw)
  To: Andrii Nakryiko, Masahiro Yamada
  Cc: andrii, jolsa, acme, eddyz87, ast, daniel, martin.lau, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo, bpf,
	linux-kbuild

On 07/05/2024 17:48, Andrii Nakryiko wrote:
> On Tue, May 7, 2024 at 6:55 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> The btf_features list can be used for pahole v1.26 and later -
>> it is useful because if a feature is not yet implemented it will
>> not exit with a failure message.  This will allow us to add feature
>> requests to the pahole options without having to check pahole versions
>> in future; if the version of pahole supports the feature it will be
>> added.
>>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> Tested-by: Eduard Zingerman <eddyz87@gmail.com>
>> ---
>>  scripts/Makefile.btf | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf
>> index 82377e470aed..2d6e5ed9081e 100644
>> --- a/scripts/Makefile.btf
>> +++ b/scripts/Makefile.btf
>> @@ -3,6 +3,8 @@
>>  pahole-ver := $(CONFIG_PAHOLE_VERSION)
>>  pahole-flags-y :=
>>
>> +ifeq ($(call test-le, $(pahole-ver), 125),y)
>> +
>>  # pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars
>>  ifeq ($(call test-le, $(pahole-ver), 121),y)
>>  pahole-flags-$(call test-ge, $(pahole-ver), 118)       += --skip_encoding_btf_vars
>> @@ -12,8 +14,17 @@ pahole-flags-$(call test-ge, $(pahole-ver), 121)     += --btf_gen_floats
>>
>>  pahole-flags-$(call test-ge, $(pahole-ver), 122)       += -j
>>
>> -pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE)         += --lang_exclude=rust
>> +ifeq ($(pahole-ver), 125)
> 
> it's a bit of a scope creep, but isn't it strange that we don't have
> test-eq and have to work-around that with more verbose constructs?

Looking at the history, I _think_ the concern that motivated the numeric
comparison constructs was the shell process fork required for numeric
comparisons. In the equality case, ifeq would work for both strings and
numeric values. Adding a test-eq (in a similar form to test-ge) would
require a fallback to shell expansion for older Make without intcmp, and
that would be slower than using ifeq, if less verbose.

> Let's do a good service to the community and add test-eq (and maybe
> test-ne while at it, don't know, up to Masahiro)?
>

Sure, I'm happy to do this if kbuild folks agree. I've cc'ed them; I
neglected to do this in the original patch, apologies about that.

Thanks!

Alan

> Overall the change looks OK to me, so if people are opposed to adding
> test-eq, I'm fine with it as well:
> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
>> +pahole-flags-y += --skip_encoding_btf_inconsistent_proto --btf_gen_optimized
>> +endif
>> +
>> +else
>>
>> -pahole-flags-$(call test-ge, $(pahole-ver), 125)       += --skip_encoding_btf_inconsistent_proto --btf_gen_optimized
>> +# 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
>> +
>> +endif
>> +
>> +pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE)         += --lang_exclude=rust
>>
>>  export PAHOLE_FLAGS := $(pahole-flags-y)
>> --
>> 2.39.3
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread