| Message ID | 6d69d6dce917475ffe9c1bd7bc53358904f60915.1714430735.git.dxu@dxuuu.xyz (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | pahole: Inject kfunc decl tags into BTF | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Mon, Apr 29, 2024 at 04:45:59PM -0600, Daniel Xu wrote: > Add a feature flag to guard tagging of kfuncs. The next commit will > implement the actual tagging. > > Reviewed-by: Alan Maguire <alan.maguire@oracle.com> > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> Also 'decl_tag_kfuncs' is not enabled when using --btf_features=default, right? as: BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false), And that false is .default_enabled=false. So I added: diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 index be04f1c617291e21..f0605935a9f1b4dc 100644 --- a/man-pages/pahole.1 +++ b/man-pages/pahole.1 @@ -308,7 +308,6 @@ Encode BTF using the specified feature list, or specify 'default' for all standa in some CUs and not others, or when the same function name has inconsistent BTF descriptions in different CUs. - decl_tag_kfuncs Inject a BTF_KIND_DECL_TAG for each discovered kfunc. .fi Supported non-standard features (not enabled for 'default') @@ -317,6 +316,7 @@ Supported non-standard features (not enabled for 'default') reproducible_build Ensure generated BTF is consistent every time; without this parallel BTF encoding can result in inconsistent BTF ids. + decl_tag_kfuncs Inject a BTF_KIND_DECL_TAG for each discovered kfunc. .fi So for example, specifying \-\-btf_encode=var,enum64 will result in a BTF encoding that (as well as encoding basic BTF information) will contain variables and enum64 values. ---- Please ack. Alan, please check if your Reviewed-by stands with the above change. Thanks, - Arnaldo > --- > btf_encoder.c | 2 ++ > dwarves.h | 1 + > man-pages/pahole.1 | 1 + > pahole.c | 1 + > 4 files changed, 5 insertions(+) > > diff --git a/btf_encoder.c b/btf_encoder.c > index 5ffaf5d..f0ef20a 100644 > --- a/btf_encoder.c > +++ b/btf_encoder.c > @@ -76,6 +76,7 @@ struct btf_encoder { > verbose, > force, > gen_floats, > + tag_kfuncs, > is_rel; > uint32_t array_index_id; > struct { > @@ -1661,6 +1662,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam > encoder->force = conf_load->btf_encode_force; > encoder->gen_floats = conf_load->btf_gen_floats; > encoder->skip_encoding_vars = conf_load->skip_encoding_btf_vars; > + encoder->tag_kfuncs = conf_load->btf_decl_tag_kfuncs; > encoder->verbose = verbose; > encoder->has_index_type = false; > encoder->need_index_type = false; > diff --git a/dwarves.h b/dwarves.h > index dd35a4e..7d566b6 100644 > --- a/dwarves.h > +++ b/dwarves.h > @@ -94,6 +94,7 @@ struct conf_load { > bool btf_gen_floats; > bool btf_encode_force; > bool reproducible_build; > + bool btf_decl_tag_kfuncs; > uint8_t hashtable_bits; > uint8_t max_hashtable_bits; > uint16_t kabi_prefix_len; > diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 > index e3c58e0..4769b58 100644 > --- a/man-pages/pahole.1 > +++ b/man-pages/pahole.1 > @@ -308,6 +308,7 @@ Encode BTF using the specified feature list, or specify 'default' for all standa > in some CUs and not others, or when the same > function name has inconsistent BTF descriptions > in different CUs. > + decl_tag_kfuncs Inject a BTF_KIND_DECL_TAG for each discovered kfunc. > .fi > > Supported non-standard features (not enabled for 'default') > diff --git a/pahole.c b/pahole.c > index 750b847..954498d 100644 > --- a/pahole.c > +++ b/pahole.c > @@ -1289,6 +1289,7 @@ struct btf_feature { > BTF_DEFAULT_FEATURE(enum64, skip_encoding_btf_enum64, true), > BTF_DEFAULT_FEATURE(optimized_func, btf_gen_optimized, false), > BTF_DEFAULT_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false), > + BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false), > BTF_NON_DEFAULT_FEATURE(reproducible_build, reproducible_build, false), > }; > > -- > 2.44.0
Hi Arnaldo, On Tue, Apr 30, 2024 at 03:48:06PM GMT, Arnaldo Carvalho de Melo wrote: > On Mon, Apr 29, 2024 at 04:45:59PM -0600, Daniel Xu wrote: > > Add a feature flag to guard tagging of kfuncs. The next commit will > > implement the actual tagging. > > > > Reviewed-by: Alan Maguire <alan.maguire@oracle.com> > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > > Also 'decl_tag_kfuncs' is not enabled when using --btf_features=default, > right? as: > > BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false), > > And that false is .default_enabled=false. I think that `false` is for `initial_value`, isn't it? The macro sets the `default_enabled` field. Building with this seems to tag the kfuncs for me: diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf index 82377e470aed..7128dc25ba29 100644 --- a/scripts/Makefile.btf +++ b/scripts/Makefile.btf @@ -16,4 +16,6 @@ pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE) += --lang_exclude=rust pahole-flags-$(call test-ge, $(pahole-ver), 125) += --skip_encoding_btf_inconsistent_proto --btf_gen_optimized +pahole-flags-$(call test-ge, $(pahole-ver), 126) = -j --lang_exclude=rust --btf_features=default + export PAHOLE_FLAGS := $(pahole-flags-y) Thanks, Daniel
On 01/05/2024 00:00, Daniel Xu wrote: > Hi Arnaldo, > > On Tue, Apr 30, 2024 at 03:48:06PM GMT, Arnaldo Carvalho de Melo wrote: >> On Mon, Apr 29, 2024 at 04:45:59PM -0600, Daniel Xu wrote: >>> Add a feature flag to guard tagging of kfuncs. The next commit will >>> implement the actual tagging. >>> >>> Reviewed-by: Alan Maguire <alan.maguire@oracle.com> >>> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> >> >> Also 'decl_tag_kfuncs' is not enabled when using --btf_features=default, >> right? as: >> >> BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false), >> >> And that false is .default_enabled=false. > > I think that `false` is for `initial_value`, isn't it? The macro sets > the `default_enabled` field. > yep it's the initial unset value. Specifying an option in --btf_features flips that value, so for initial-off values they are switched on, while initial-on values are switched off. I _think_ the intent here is to tag kfuncs by default, so we can add tag_kfuncs to the set of options specified in pahole-flags for v1.26. We won't be using "default" there as we want to call out the flags explicitly. Alan > Building with this seems to tag the kfuncs for me: > > diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf > index 82377e470aed..7128dc25ba29 100644 > --- a/scripts/Makefile.btf > +++ b/scripts/Makefile.btf > @@ -16,4 +16,6 @@ pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE) += --lang_exclude=rust > > pahole-flags-$(call test-ge, $(pahole-ver), 125) += --skip_encoding_btf_inconsistent_proto --btf_gen_optimized > > +pahole-flags-$(call test-ge, $(pahole-ver), 126) = -j --lang_exclude=rust --btf_features=default > + > export PAHOLE_FLAGS := $(pahole-flags-y) > > Thanks, > Daniel
On Thu, May 02, 2024 at 12:49:26PM +0100, Alan Maguire wrote: > On 01/05/2024 00:00, Daniel Xu wrote: > > On Tue, Apr 30, 2024 at 03:48:06PM GMT, Arnaldo Carvalho de Melo wrote: > >> On Mon, Apr 29, 2024 at 04:45:59PM -0600, Daniel Xu wrote: > >> Also 'decl_tag_kfuncs' is not enabled when using --btf_features=default, > >> right? as: > >> BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false), > >> And that false is .default_enabled=false. > > I think that `false` is for `initial_value`, isn't it? The macro sets > > the `default_enabled` field. > yep it's the initial unset value. Specifying an option in --btf_features > flips that value, so for initial-off values they are switched on, while So --btf_features=something may mean "don't use that feature"? That is confusing, perhaps the '-something' come in handy? > initial-on values are switched off. I _think_ the intent here is to tag > kfuncs by default, so we can add tag_kfuncs to the set of options Probably, if they are present in the file being BTF encoded. > specified in pahole-flags for v1.26. We won't be using "default" there > as we want to call out the flags explicitly. Sure. - Arnaldo
On 02/05/2024 14:35, Arnaldo Carvalho de Melo wrote: > On Thu, May 02, 2024 at 12:49:26PM +0100, Alan Maguire wrote: >> On 01/05/2024 00:00, Daniel Xu wrote: >>> On Tue, Apr 30, 2024 at 03:48:06PM GMT, Arnaldo Carvalho de Melo wrote: >>>> On Mon, Apr 29, 2024 at 04:45:59PM -0600, Daniel Xu wrote: >>>> Also 'decl_tag_kfuncs' is not enabled when using --btf_features=default, >>>> right? as: > >>>> BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false), > >>>> And that false is .default_enabled=false. > >>> I think that `false` is for `initial_value`, isn't it? The macro sets >>> the `default_enabled` field. > >> yep it's the initial unset value. Specifying an option in --btf_features >> flips that value, so for initial-off values they are switched on, while > > So --btf_features=something may mean "don't use that feature"? That is > confusing, perhaps the '-something' come in handy? > No, in fact --btf_features tries to move away from the model of having a mix of enable and skip features. The reason we do things this way is we inherited a situation where some features that would begin as off if not specified (encode FLOAT) and some that would start off as on unless a skip option was specified (encode VAR). Prior to --btf_features, we accordingly had --enable-feature and --skip-feature flags for these. However with --btf_features, all features are positive; that is, if specified, we enable var, float etc; there are no "skip" features. Under the hood however, we preserve the prior "enable or skip" semantics; that's why some default values have initial values of false (an "enable" feature under the hood) and some have an initial value of true (a "skip" value under the hood. But none of that is exposed to the --btf_features user; if a feature is wanted, they just add it to the list. >> initial-on values are switched off. I _think_ the intent here is to tag >> kfuncs by default, so we can add tag_kfuncs to the set of options > > Probably, if they are present in the file being BTF encoded. > >> specified in pahole-flags for v1.26. We won't be using "default" there >> as we want to call out the flags explicitly. > > Sure. > > - Arnaldo >
diff --git a/btf_encoder.c b/btf_encoder.c index 5ffaf5d..f0ef20a 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -76,6 +76,7 @@ struct btf_encoder { verbose, force, gen_floats, + tag_kfuncs, is_rel; uint32_t array_index_id; struct { @@ -1661,6 +1662,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam encoder->force = conf_load->btf_encode_force; encoder->gen_floats = conf_load->btf_gen_floats; encoder->skip_encoding_vars = conf_load->skip_encoding_btf_vars; + encoder->tag_kfuncs = conf_load->btf_decl_tag_kfuncs; encoder->verbose = verbose; encoder->has_index_type = false; encoder->need_index_type = false; diff --git a/dwarves.h b/dwarves.h index dd35a4e..7d566b6 100644 --- a/dwarves.h +++ b/dwarves.h @@ -94,6 +94,7 @@ struct conf_load { bool btf_gen_floats; bool btf_encode_force; bool reproducible_build; + bool btf_decl_tag_kfuncs; uint8_t hashtable_bits; uint8_t max_hashtable_bits; uint16_t kabi_prefix_len; diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 index e3c58e0..4769b58 100644 --- a/man-pages/pahole.1 +++ b/man-pages/pahole.1 @@ -308,6 +308,7 @@ Encode BTF using the specified feature list, or specify 'default' for all standa in some CUs and not others, or when the same function name has inconsistent BTF descriptions in different CUs. + decl_tag_kfuncs Inject a BTF_KIND_DECL_TAG for each discovered kfunc. .fi Supported non-standard features (not enabled for 'default') diff --git a/pahole.c b/pahole.c index 750b847..954498d 100644 --- a/pahole.c +++ b/pahole.c @@ -1289,6 +1289,7 @@ struct btf_feature { BTF_DEFAULT_FEATURE(enum64, skip_encoding_btf_enum64, true), BTF_DEFAULT_FEATURE(optimized_func, btf_gen_optimized, false), BTF_DEFAULT_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false), + BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false), BTF_NON_DEFAULT_FEATURE(reproducible_build, reproducible_build, false), };