Re: [PATCH v2 dwarves 0/5] pahole, btf_encoder: support --btf_features

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Oct 13, 2023 at 04:33:54PM +0100, Alan Maguire wrote:
> Currently, the kernel uses pahole version checking as the way to
> determine which BTF encoding features to request from pahole.  This
> means that such features have to be tied to a specific version and
> as new features are added, additional clauses in scripts/pahole-flags.sh
> have to be added; for example
> 
> if [ "${pahole_ver}" -ge "125" ]; then
>         extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_inconsistent_proto --btf_gen_optimized"
> fi
> 
> To better future-proof this process, this series introduces a
> single "btf_features" parameter that uses a comma-separated list
> of encoding options.  This is helpful because
> 
> - the semantics are simpler for the user; the list comprises the set of
>   BTF features asked for, rather than having to specify a combination of
>   --skip_encoding_btf_feature and --btf_gen_feature options; and
> - any version of pahole that supports --btf_features can accept the
>   option list; unknown options are silently ignored.  As a result, there
>   would be no need to add additional version clauses beyond
> 
> if [ "${pahole_ver}" -ge "126" ]; then
>         extra_pahole_opt="-j --lang_exclude=rust
> --btf_features=encode_force,var,float,decl_tag,type_tag,enum64,optimized,consistent"
> fi
> 
>   Newly-supported features would simply be appended to the btf_features
>   list, and these would have impact on BTF encoding only if the features
>   were supported by pahole.  This means pahole will not require a version
>   bump when new BTF features are added, and should ease the burden of
>   coordinating such changes between bpf-next and dwarves.
> 
> Patches 1 and 2 are preparatory work, while patch 3 adds the
> --btf_features support.  Patch 4 provides a means of querying
> the supported feature set since --btf_features will not error
> out when it encounters unrecognized features (this ensures
> an older pahole without a requested feature will not dump warnings
> in the build log for kernel/module BTF generation).  Patch 5
> adds --btf_features_strict, which is identical to --btf_features
> aside from the fact it will fail if an unrecognized feature is used.
> 
> See [1] for more background on this topic.
> 
> Changes since RFC [2]:
> 
> - ensure features are disabled unless requested; use "default" field in
>   "struct btf_features" to specify the conf_load default value which
>   corresponds to the feature being disabled.  For
>   conf_load->btf_gen_floats for example, the default value is false,
>   while for conf_load->skip_encoding_btf_type_tags the default is
>   true; in both cases the intent is to _not_ encode the associated
>   feature by default.  However if the user specifies "float" or
>   "type_tag" in --btf_features, the default conf_load value is negated,
>   resulting in a BTF encoding that contains floats and type tags
>   (Eduard, patch 3)
> - clarify feature default/setting behaviour and how it only applies
>   when --btf_features is used (Eduard, patch 3)
> - ensure we do not run off the end of the feature_list[] array
>   (Eduard, patch 3)
> - rather than having each struct btf_feature record the offset in the
>   conf_load structure of the boolean (requiring us to later do pointer
>   math to update it), record the pointers to the boolean conf_load
>   values associated with each feature (Jiri, patch 3)
> - allow for multiple specifications of --btf_features, enabling the
>   union of all features specified (Andrii, patch 3)
> - rename function-related optimized/consistent to optimized_func and
>   consistent_func in recognition of the fact they are function-specific
>   (Andrii, patch 3)
> - add a strict version of --btf_features, --btf_features_strict that
>   will error out if an unrecognized feature is used (Andrii, patch 5)
> 
> [1] https://lore.kernel.org/bpf/CAEf4Bzaz1UqqxuZ7Q+KQee-HLyY1nwhAurBE2n9YTWchqoYLbg@mail.gmail.com/
> [2] https://lore.kernel.org/bpf/20231011091732.93254-1-alan.maguire@oracle.com/
> 

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> Alan Maguire (5):
>   btf_encoder, pahole: move btf encoding options into conf_load
>   dwarves: move ARRAY_SIZE() to dwarves.h
>   pahole: add --btf_features support
>   pahole: add --supported_btf_features
>   pahole: add --btf_features_strict to reject unknown BTF features
> 
>  btf_encoder.c      |   8 +--
>  btf_encoder.h      |   2 +-
>  dwarves.c          |  16 -----
>  dwarves.h          |  19 ++++++
>  man-pages/pahole.1 |  32 +++++++++
>  pahole.c           | 167 +++++++++++++++++++++++++++++++++++++++++----
>  6 files changed, 210 insertions(+), 34 deletions(-)
> 
> -- 
> 2.31.1
>