| Message ID | 20231011091732.93254-1-alan.maguire@oracle.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | pahole, btf_encoder: support --btf_features | expand |
On Wed, Oct 11, 2023 at 2:17 AM Alan Maguire <alan.maguire@oracle.com> 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, we can have a catch-all > single "btf_features" parameter that uses a comma-separated list > of encoding options. What makes this better is that any version > of pahole that supports btf_features can accept the option list; > unknown options are silently ignored. However 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" I've seen a nice convention used in a lot of CLIs that allow multiple values for a given parameter: --btf_feature=encode_force --btf=var --btf=float and so on It simplifies parsing code, and is also a bit more "modular" in scripts. But it's just a minor nit, feel free to ignore. > 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. Yep, this is great. > > 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 an unrecognized features (this ensures > an older pahole without a requested feature will not dump warnings > in the build log for kernel/module BTF generation). List of features is great. I think we can also have something like --btf_features_strict to make pahole error out if an unsupported feature is specified. > > See [1] for more background on this topic. > > [1] https://lore.kernel.org/bpf/CAEf4Bzaz1UqqxuZ7Q+KQee-HLyY1nwhAurBE2n9YTWchqoYLbg@mail.gmail.com/ > > Alan Maguire (4): > btf_encoder, pahole: move btf encoding options into conf_load > dwarves: move ARRAY_SIZE() to dwarves.h > pahole: add --btf_features=feature1[,feature2...] support > pahole: add --supported_btf_features to display feature support > > btf_encoder.c | 8 +-- > btf_encoder.h | 2 +- > dwarves.c | 16 ------ > dwarves.h | 19 +++++++ > man-pages/pahole.1 | 24 +++++++++ > pahole.c | 127 ++++++++++++++++++++++++++++++++++++++++----- > 6 files changed, 162 insertions(+), 34 deletions(-) > > -- > 2.31.1 >
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, we can have a catch-all single "btf_features" parameter that uses a comma-separated list of encoding options. What makes this better is that any version of pahole that supports btf_features can accept the option list; unknown options are silently ignored. However 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 an unrecognized features (this ensures an older pahole without a requested feature will not dump warnings in the build log for kernel/module BTF generation). See [1] for more background on this topic. [1] https://lore.kernel.org/bpf/CAEf4Bzaz1UqqxuZ7Q+KQee-HLyY1nwhAurBE2n9YTWchqoYLbg@mail.gmail.com/ Alan Maguire (4): btf_encoder, pahole: move btf encoding options into conf_load dwarves: move ARRAY_SIZE() to dwarves.h pahole: add --btf_features=feature1[,feature2...] support pahole: add --supported_btf_features to display feature support btf_encoder.c | 8 +-- btf_encoder.h | 2 +- dwarves.c | 16 ------ dwarves.h | 19 +++++++ man-pages/pahole.1 | 24 +++++++++ pahole.c | 127 ++++++++++++++++++++++++++++++++++++++++----- 6 files changed, 162 insertions(+), 34 deletions(-)