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

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

[Alan Maguire]

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)
+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

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

[Andrii Nakryiko]

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?
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)?

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
>

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

[Alan Maguire]

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
>>

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

[Andrii Nakryiko]

On Thu, May 9, 2024 at 1:20 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> 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.
>

Ok, let's see if Masahiro would like this improvement or not. For now
this patch gets us into a nicer place where there are legacy parts and
a better --btf_features setup completely separate, so I applied the
patch as is to bpf-next. If we decide to do test-eq, we can improve
this further separately. Thanks!


> 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
> >>

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

[Masahiro Yamada]

On Fri, May 10, 2024 at 7:01 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, May 9, 2024 at 1:20 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > 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.
> >
>
> Ok, let's see if Masahiro would like this improvement or not. For now
> this patch gets us into a nicer place where there are legacy parts and
> a better --btf_features setup completely separate, so I applied the
> patch as is to bpf-next. If we decide to do test-eq, we can improve
> this further separately. Thanks!


That is a noise change.
You did not need to modify the line in the first place.


The previous

  pahole-flags-$(call test-ge, $(pahole-ver), 125)

works as-is.




--
Best Regards
Masahiro Yamada

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

[Andrii Nakryiko]

On Thu, May 9, 2024 at 11:30 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, May 10, 2024 at 7:01 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, May 9, 2024 at 1:20 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> > >
> > > 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.
> > >
> >
> > Ok, let's see if Masahiro would like this improvement or not. For now
> > this patch gets us into a nicer place where there are legacy parts and
> > a better --btf_features setup completely separate, so I applied the
> > patch as is to bpf-next. If we decide to do test-eq, we can improve
> > this further separately. Thanks!
>
>
> That is a noise change.
> You did not need to modify the line in the first place.
>

Not sure which specific line you are referring to. But the idea here
is that starting from pahole 1.26 we want to stop to set those
"legacy" arguments (like --skip_encoding_btf_vars, --btf_gen_floats)
and *only* use more usable and forward-compatible --btf_features.

>
> The previous
>
>   pahole-flags-$(call test-ge, $(pahole-ver), 125)
>
> works as-is.
>
>
>
>
> --
> Best Regards
> Masahiro Yamada

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

[Masahiro Yamada]

[-- Attachment #1: Type: text/plain, Size: 4788 bytes --]

On Sat, May 11, 2024 at 6:45 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, May 9, 2024 at 11:30 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Fri, May 10, 2024 at 7:01 AM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Thu, May 9, 2024 at 1:20 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> > > >
> > > > 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.
> > > >
> > >
> > > Ok, let's see if Masahiro would like this improvement or not. For now
> > > this patch gets us into a nicer place where there are legacy parts and
> > > a better --btf_features setup completely separate, so I applied the
> > > patch as is to bpf-next. If we decide to do test-eq, we can improve
> > > this further separately. Thanks!
> >
> >
> > That is a noise change.
> > You did not need to modify the line in the first place.
> >
>
> Not sure which specific line you are referring to. But the idea here
> is that starting from pahole 1.26 we want to stop to set those
> "legacy" arguments (like --skip_encoding_btf_vars, --btf_gen_floats)
> and *only* use more usable and forward-compatible --btf_features.
>
> >
> > The previous
> >
> >   pahole-flags-$(call test-ge, $(pahole-ver), 125)
> >
> > works as-is.


You did not not need to change

  pahole-flags-$(call test-ge, $(pahole-ver), 125) += ...


to


  ifeq ($(pahole-ver), 125)
  pahole-flags-y += ...
  endif



Please note it exists in

  ifeq ($(call test-le, $(pahole-ver), 125),y)
     ...
  else





if (pahole_ver <= 125) {
      do_something();
      if (pahole_ver >= 125)
             do_other();
}


  and


if (pahole_ver <= 125) {
      do_something();
      if (pahole_ver == 125)
            do_other();
}


are equivalent, don't they?



The former is more intuitive because pahole 1.25+ supports
--skip_encoding_btf_inconsistent_proto --btf_gen_optimized



I attached a simpler and more correct patch.










-- 
Best Regards
Masahiro Yamada

[-- Attachment #2: diff.patch --]
[-- Type: text/x-patch, Size: 1141 bytes --]

diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf
index 82377e470aed..bca8a8f26ea4 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,15 @@ 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
-
 pahole-flags-$(call test-ge, $(pahole-ver), 125)	+= --skip_encoding_btf_inconsistent_proto --btf_gen_optimized
 
+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
+
+endif
+
+pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE)		+= --lang_exclude=rust
+
 export PAHOLE_FLAGS := $(pahole-flags-y)

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

[Andrii Nakryiko]

On Sat, May 11, 2024 at 3:01 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Sat, May 11, 2024 at 6:45 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, May 9, 2024 at 11:30 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Fri, May 10, 2024 at 7:01 AM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Thu, May 9, 2024 at 1:20 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> > > > >
> > > > > 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.
> > > > >
> > > >
> > > > Ok, let's see if Masahiro would like this improvement or not. For now
> > > > this patch gets us into a nicer place where there are legacy parts and
> > > > a better --btf_features setup completely separate, so I applied the
> > > > patch as is to bpf-next. If we decide to do test-eq, we can improve
> > > > this further separately. Thanks!
> > >
> > >
> > > That is a noise change.
> > > You did not need to modify the line in the first place.
> > >
> >
> > Not sure which specific line you are referring to. But the idea here
> > is that starting from pahole 1.26 we want to stop to set those
> > "legacy" arguments (like --skip_encoding_btf_vars, --btf_gen_floats)
> > and *only* use more usable and forward-compatible --btf_features.
> >
> > >
> > > The previous
> > >
> > >   pahole-flags-$(call test-ge, $(pahole-ver), 125)
> > >
> > > works as-is.
>
>
> You did not not need to change
>
>   pahole-flags-$(call test-ge, $(pahole-ver), 125) += ...
>
>
> to
>
>
>   ifeq ($(pahole-ver), 125)
>   pahole-flags-y += ...
>   endif
>
>
>
> Please note it exists in
>
>   ifeq ($(call test-le, $(pahole-ver), 125),y)
>      ...
>   else
>
>
>
>
>
> if (pahole_ver <= 125) {
>       do_something();
>       if (pahole_ver >= 125)
>              do_other();
> }
>
>
>   and
>
>
> if (pahole_ver <= 125) {
>       do_something();
>       if (pahole_ver == 125)
>             do_other();
> }
>
>
> are equivalent, don't they?
>
>
>
> The former is more intuitive because pahole 1.25+ supports
> --skip_encoding_btf_inconsistent_proto --btf_gen_optimized

The point here is to not specify these "legacy" arguments starting
from pahole v1.26, and the patch makes it more obvious, IMO.

But I don't mind adding (test-ge,125) check back, Alan, please send a
follow up patch.

>
>
>
> I attached a simpler and more correct patch.

It's not more correct, because this patch didn't break anything. It's
just as correct.

>
>
>
>
>
>
>
>
>
>
> --
> Best Regards
> Masahiro Yamada

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

[Alan Maguire]

On 14/05/2024 16:53, Andrii Nakryiko wrote:
> On Sat, May 11, 2024 at 3:01 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>
>> On Sat, May 11, 2024 at 6:45 AM Andrii Nakryiko
>> <andrii.nakryiko@gmail.com> wrote:
>>>
>>> On Thu, May 9, 2024 at 11:30 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>>>
>>>> On Fri, May 10, 2024 at 7:01 AM Andrii Nakryiko
>>>> <andrii.nakryiko@gmail.com> wrote:
>>>>>
>>>>> On Thu, May 9, 2024 at 1:20 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>
>>>>> Ok, let's see if Masahiro would like this improvement or not. For now
>>>>> this patch gets us into a nicer place where there are legacy parts and
>>>>> a better --btf_features setup completely separate, so I applied the
>>>>> patch as is to bpf-next. If we decide to do test-eq, we can improve
>>>>> this further separately. Thanks!
>>>>
>>>>
>>>> That is a noise change.
>>>> You did not need to modify the line in the first place.
>>>>
>>>
>>> Not sure which specific line you are referring to. But the idea here
>>> is that starting from pahole 1.26 we want to stop to set those
>>> "legacy" arguments (like --skip_encoding_btf_vars, --btf_gen_floats)
>>> and *only* use more usable and forward-compatible --btf_features.
>>>
>>>>
>>>> The previous
>>>>
>>>>   pahole-flags-$(call test-ge, $(pahole-ver), 125)
>>>>
>>>> works as-is.
>>
>>
>> You did not not need to change
>>
>>   pahole-flags-$(call test-ge, $(pahole-ver), 125) += ...
>>
>>
>> to
>>
>>
>>   ifeq ($(pahole-ver), 125)
>>   pahole-flags-y += ...
>>   endif
>>
>>
>>
>> Please note it exists in
>>
>>   ifeq ($(call test-le, $(pahole-ver), 125),y)
>>      ...
>>   else
>>
>>
>>
>>
>>
>> if (pahole_ver <= 125) {
>>       do_something();
>>       if (pahole_ver >= 125)
>>              do_other();
>> }
>>
>>
>>   and
>>
>>
>> if (pahole_ver <= 125) {
>>       do_something();
>>       if (pahole_ver == 125)
>>             do_other();
>> }
>>
>>
>> are equivalent, don't they?
>>
>>
>>
>> The former is more intuitive because pahole 1.25+ supports
>> --skip_encoding_btf_inconsistent_proto --btf_gen_optimized
> 
> The point here is to not specify these "legacy" arguments starting
> from pahole v1.26, and the patch makes it more obvious, IMO.
> 
> But I don't mind adding (test-ge,125) check back, Alan, please send a
> follow up patch.
>

Done, see [1]. Thanks!

Alan

[1]
https://lore.kernel.org/bpf/20240514162716.2448265-1-alan.maguire@oracle.com/

>>
>>
>>
>> I attached a simpler and more correct patch.
> 
> It's not more correct, because this patch didn't break anything. It's
> just as correct.
> 
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> --
>> Best Regards
>> Masahiro Yamada
> 

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Tue,  7 May 2024 14:55:14 +0100 you 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.
> 
> [...]

Here is the summary with links:
  - [v2,bpf-next] kbuild,bpf: switch to using --btf_features for pahole v1.26 and later
    https://git.kernel.org/bpf/bpf-next/c/fcd1ed89a043

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html