| Message ID | 20231018122926.735416-1-alan.maguire@oracle.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | pahole, btf_encoder: support --btf_features | expand |
On Wed, 2023-10-18 at 13:29 +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_func,consistent_func" > 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 v2 [2]: > - added acks from Andrii and Jiri (patches 1-5) > - merged suggestions from Eduard which simplify and clean up code > considerably; these changes fix issues with --btf_features_strict > while providing better diagnostic output when an unknown feature > is encountered (specifying the unknown feature if in verbose > or strict modes). Added Suggested-bys from Eduard for the > relevant patches (Eduard, patches 3,5) > > Changes since RFC [3]: > > - 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/20231013153359.88274-1-alan.maguire@oracle.com/ > [3] https://lore.kernel.org/bpf/20231011091732.93254-1-alan.maguire@oracle.com/ Acked-by: Eduard Zingerman <eddyz87@gmail.com> Hi Alan, thank you for accommodating my changes, I've tested this patch-set and everything seems to work fine. I left one nitpick for patch #3 feel free to ignore it if you don't agree. Thanks, Eduard
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_func,consistent_func" 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 v2 [2]: - added acks from Andrii and Jiri (patches 1-5) - merged suggestions from Eduard which simplify and clean up code considerably; these changes fix issues with --btf_features_strict while providing better diagnostic output when an unknown feature is encountered (specifying the unknown feature if in verbose or strict modes). Added Suggested-bys from Eduard for the relevant patches (Eduard, patches 3,5) Changes since RFC [3]: - 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/20231013153359.88274-1-alan.maguire@oracle.com/ [3] https://lore.kernel.org/bpf/20231011091732.93254-1-alan.maguire@oracle.com/ 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 | 191 ++++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 234 insertions(+), 34 deletions(-)