From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Alan Maguire <alan.maguire@oracle.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
dwarves@vger.kernel.org, Jiri Olsa <jolsa@kernel.org>,
Clark Williams <williams@redhat.com>,
Kate Carcia <kcarcia@redhat.com>,
bpf@vger.kernel.org, Arnaldo Carvalho de Melo <acme@redhat.com>,
Daniel Xu <dxu@dxuuu.xyz>, Eduard Zingerman <eddyz87@gmail.com>
Subject: Re: [PATCH 2/2] pahole: Allow asking for extra features using the '+' prefix in --btf_features
Date: Mon, 29 Apr 2024 09:46:01 -0700 [thread overview]
Message-ID: <CAEf4Bzb4Yw7GyyhdMu7fbPQSHE5PumRWB4nRaoi=BYax57hSTg@mail.gmail.com> (raw)
In-Reply-To: <d9e3a7ab-9799-42b0-9c6f-1809a0527867@oracle.com>
On Mon, Apr 29, 2024 at 4:16 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 26/04/2024 21:47, Arnaldo Carvalho de Melo wrote:
> > On Fri, Apr 26, 2024 at 01:26:40PM -0700, Andrii Nakryiko wrote:
> >> On Fri, Apr 19, 2024 at 1:58 PM Arnaldo Carvalho de Melo
> >> <acme@kernel.org> wrote:
> >>>
> >>> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> >>>
> >>> Instead of the somewhat confusing:
> >>>
> >>> --btf_features=all,reproducible_build
> >>>
> >>> That means "'all' the standard BTF features plus the 'reproducible_build'
> >>> extra BTF feature", use + directly, making it more compact:
> >>>
> >>> --btf_features=+reproducible_build
> >>>
> >>
> >> for older paholes that don't yet know about + syntax, but support
> >> --btf_features, will this effectively disable all features or how will
> >> it work?
> >>
> >> I'm thinking from the perspective of using +reproducible_build
> >> unconditionally in kernel's build scripts, will it regress something
> >> for current pahole versions?
> >
> > Well, I think it will end up being discarded just like "all" or
> > "default", no? I.e. those were keywords not grokked by older pahole
> > versions, so ignored as we're not using --btf_features_strict, right?
> >
> > Alan?
> >
>
> Yep, it would just be ignored, so wouldn't have the desired behaviour
> of enabling defaults + reproducible build option.
>
> > But then we're not yet using --btf_features in scripts/Makefile.btf,
> > right?
> >
> > But as Daniel pointed out and Alan (I think) agreed, for things like
> > scripts we probably end up using the most verbose thing as:
> >
> > --btf_features=default,reproducible_build
> >
> > to mean a set (the default set of BTF options) + an optional/extra
> > feature (reproducibe_build), that for people not used to the + syntax
> > may be more descriptive (I really think that both are confusing for
> > beginners knowing nothing about BTF and its evolution, etc).
> >
> > Alan, also we released 1.26 with "all" meaning what we now call
> > "default", so we need to keep both meaning the same thing, right?
> >
>
> I might be missing something here, but I think we should always call out
> explicitly the set of features we want in the kernel Makefile.btf
> (something like [1]). The reason for this is that the concept of what is
> "default" may evolve over time; for example it's going to include
> Daniel's kfunc definitions for soon. That's a good thing, but it could
> conceivably cause problems down the line. Consider a newer pahole - with
> a newer set of defaults - running on an older kernel. In that case, we
> could end up encoding BTF features we don't want. By contrast, if we
> always call out the full set of BTF features we want via
> --btf_features=feature1,feature2 etc we'll always get the expected set.
> Plus for folks consulting the code, it's much clearer which BTF features
> are in use when they look at the Makefiles for a particular kernel.
> So my sense of the value of "default" is as a shortcut for testing the
> latest and greatest set of BTF feature encoding, but not for use in the
> kernel tree Makefiles. Thanks!
Yep, I agree, the whole point was to not regress older kernel builds
with newer pahole, so we need to explicitly list used features.
>
> Alan
>
> [1]
> https://lore.kernel.org/bpf/20240424154806.3417662-7-alan.maguire@oracle.com/
>
> > - Arnaldo
> >
> >>> In the future we may want the '-' counterpart as a way to _remove_ some
> >>> of the standard set of BTF features.
> >>>
> >>> Cc: Alan Maguire <alan.maguire@oracle.com>
> >>> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> >>> Cc: Daniel Xu <dxu@dxuuu.xyz>
> >>> Cc: Eduard Zingerman <eddyz87@gmail.com>
> >>> Cc: Jiri Olsa <jolsa@kernel.org>
> >>> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >>> ---
> >>> man-pages/pahole.1 | 6 ++++++
> >>> pahole.c | 6 ++++++
> >>> tests/reproducible_build.sh | 2 +-
> >>> 3 files changed, 13 insertions(+), 1 deletion(-)
> >>>
> >>
> >> [...]
next prev parent reply other threads:[~2024-04-29 16:46 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-19 20:57 [PATCHES 0/2] Introduce --btf_features=+extra_features syntax Arnaldo Carvalho de Melo
2024-04-19 20:57 ` [PATCH 1/2] pahole: Factor out routine to process "--btf_features=all" Arnaldo Carvalho de Melo
2024-04-19 20:57 ` [PATCH 2/2] pahole: Allow asking for extra features using the '+' prefix in --btf_features Arnaldo Carvalho de Melo
2024-04-26 20:26 ` Andrii Nakryiko
2024-04-26 20:47 ` Arnaldo Carvalho de Melo
2024-04-29 11:16 ` Alan Maguire
2024-04-29 16:46 ` Andrii Nakryiko [this message]
2024-04-29 18:48 ` Arnaldo Carvalho de Melo
2024-04-23 2:29 ` [PATCHES 0/2] Introduce --btf_features=+extra_features syntax Daniel Xu
2024-04-23 9:02 ` Alan Maguire
2024-04-23 14:22 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAEf4Bzb4Yw7GyyhdMu7fbPQSHE5PumRWB4nRaoi=BYax57hSTg@mail.gmail.com' \
--to=andrii.nakryiko@gmail.com \
--cc=acme@kernel.org \
--cc=acme@redhat.com \
--cc=alan.maguire@oracle.com \
--cc=bpf@vger.kernel.org \
--cc=dwarves@vger.kernel.org \
--cc=dxu@dxuuu.xyz \
--cc=eddyz87@gmail.com \
--cc=jolsa@kernel.org \
--cc=kcarcia@redhat.com \
--cc=williams@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).