On Mon, Apr 29, 2024 at 4:16 AM Alan Maguire <alan.maguire@xxxxxxxxxx> 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@xxxxxxxxxx> wrote: > >>> > >>> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> > >>> > >>> 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@xxxxxxxxxx/ > > > - 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@xxxxxxxxxx> > >>> Cc: Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> > >>> Cc: Daniel Xu <dxu@xxxxxxxxx> > >>> Cc: Eduard Zingerman <eddyz87@xxxxxxxxx> > >>> Cc: Jiri Olsa <jolsa@xxxxxxxxxx> > >>> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> > >>> --- > >>> man-pages/pahole.1 | 6 ++++++ > >>> pahole.c | 6 ++++++ > >>> tests/reproducible_build.sh | 2 +- > >>> 3 files changed, 13 insertions(+), 1 deletion(-) > >>> > >> > >> [...]
