* [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional. @ 2020-09-18 20:40 Hao Luo 2020-09-18 20:40 ` [PATCH 1/2] btf_encoder: Make encoding " Hao Luo ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Hao Luo @ 2020-09-18 20:40 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Andrii Nakryiko, Alexei Starovoitov, daniel, dwarves, yhs, Hao Luo Previous commit f3d9054ba8ff ("btf_encoder: Teach pahole to store percpu variables in vmlinux BTF.") introduced a feature in btf_encoder that encodes global symbols in BTF. However, this feature is not protected by any flag. In order to avoid surprises after Pahole v1.18 rolls out, make this feature off by default and enable only upon request. On the kernel side, we can add a Kconfig to enable this new capability on an opt-in pattern. Also as a refactor, introduce '--btf_encode_force' to replace the old '--force' and '-j' option that is used to forcefully emit BTF. Hao Luo (2): btf_encoder: Make encoding vars into BTF optional. btf_encoder: Introduce option '--btf_encode_force' btf_encoder.c | 5 ++++- btf_encoder.h | 2 +- pahole.c | 22 +++++++++++++++++----- 3 files changed, 22 insertions(+), 7 deletions(-) -- 2.28.0.681.g6f77f65b4e-goog ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] btf_encoder: Make encoding vars into BTF optional. 2020-09-18 20:40 [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional Hao Luo @ 2020-09-18 20:40 ` Hao Luo 2020-09-18 21:24 ` Arnaldo Carvalho de Melo 2020-09-18 20:40 ` [PATCH 2/2] btf_encoder: Introduce option '--btf_encode_force' Hao Luo 2020-09-19 0:57 ` [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional Alexei Starovoitov 2 siblings, 1 reply; 9+ messages in thread From: Hao Luo @ 2020-09-18 20:40 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Andrii Nakryiko, Alexei Starovoitov, daniel, dwarves, yhs, Hao Luo Introduce an option --btf_encode_symbols to pahole so that pahole will only encode global symbols upon request. On the kernel side, we can add a Kconfig to turn on this feature. This would make the rollout of pahole v1.18 safe against potential bugs in the feature introduced in commit f3d9054ba8ff ("btf_encoder: Teach pahole to store percpu variables in vmlinux BTF."). Signed-off-by: Hao Luo <haoluo@google.com> --- btf_encoder.c | 5 ++++- btf_encoder.h | 2 +- pahole.c | 11 ++++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/btf_encoder.c b/btf_encoder.c index 982f59d..54ffdd1 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -237,7 +237,7 @@ static struct variable *hashaddr__find_variable(const struct hlist_head hashtabl return NULL; } -int cu__encode_btf(struct cu *cu, int verbose, bool force) +int cu__encode_btf(struct cu *cu, int verbose, bool force, bool encode_symbols) { bool add_index_type = false; uint32_t type_id_off; @@ -314,6 +314,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force) } } + /* if there is no request to encode symbols, just finish. */ + if (!encode_symbols) + goto out; if (btfe->percpu_shndx == 0 || !btfe->symtab) goto out; diff --git a/btf_encoder.h b/btf_encoder.h index bca2348..f2e95fe 100644 --- a/btf_encoder.h +++ b/btf_encoder.h @@ -13,6 +13,6 @@ struct cu; int btf_encoder__encode(); -int cu__encode_btf(struct cu *cu, int verbose, bool force); +int cu__encode_btf(struct cu *cu, int verbose, bool force, bool encode_symbols); #endif /* _BTF_ENCODER_H_ */ diff --git a/pahole.c b/pahole.c index e1cde61..079d951 100644 --- a/pahole.c +++ b/pahole.c @@ -26,6 +26,7 @@ static bool btf_encode; static bool ctf_encode; static bool first_obj_only; +static bool btf_encode_symbols; static uint8_t class__include_anonymous; static uint8_t class__include_nested_anonymous; @@ -809,6 +810,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; #define ARGP_header_type 314 #define ARGP_size_bytes 315 #define ARGP_range 316 +#define ARGP_btf_encode_symbols 317 static const struct argp_option pahole__options[] = { { @@ -1087,6 +1089,11 @@ static const struct argp_option pahole__options[] = { .key = 'J', .doc = "Encode as BTF", }, + { + .name = "btf_encode_symbols", + .key = ARGP_btf_encode_symbols, + .doc = "Encode global symbols in BTF. Off by default." + }, { .name = "force", .key = 'j', @@ -1207,6 +1214,8 @@ static error_t pahole__options_parser(int key, char *arg, conf.range = arg; break; case ARGP_header_type: conf.header_type = arg; break; + case ARGP_btf_encode_symbols: + btf_encode_symbols = true; break; default: return ARGP_ERR_UNKNOWN; } @@ -2352,7 +2361,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, goto filter_it; if (btf_encode) { - cu__encode_btf(cu, global_verbose, force); + cu__encode_btf(cu, global_verbose, force, btf_encode_symbols); return LSK__KEEPIT; } -- 2.28.0.681.g6f77f65b4e-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] btf_encoder: Make encoding vars into BTF optional. 2020-09-18 20:40 ` [PATCH 1/2] btf_encoder: Make encoding " Hao Luo @ 2020-09-18 21:24 ` Arnaldo Carvalho de Melo 2020-09-18 21:26 ` Hao Luo 0 siblings, 1 reply; 9+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-09-18 21:24 UTC (permalink / raw) To: Hao Luo Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Alexei Starovoitov, daniel, dwarves, yhs Em Fri, Sep 18, 2020 at 01:40:57PM -0700, Hao Luo escreveu: > Introduce an option --btf_encode_symbols to pahole so that pahole > will only encode global symbols upon request. On the kernel side, > we can add a Kconfig to turn on this feature. This would make the > rollout of pahole v1.18 safe against potential bugs in the feature > introduced in commit f3d9054ba8ff ("btf_encoder: Teach pahole to > store percpu variables in vmlinux BTF."). 'encode_symbols' is too generic, please rename it to encode_global_symbols or something like that. - Arnaldo > Signed-off-by: Hao Luo <haoluo@google.com> > --- > btf_encoder.c | 5 ++++- > btf_encoder.h | 2 +- > pahole.c | 11 ++++++++++- > 3 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/btf_encoder.c b/btf_encoder.c > index 982f59d..54ffdd1 100644 > --- a/btf_encoder.c > +++ b/btf_encoder.c > @@ -237,7 +237,7 @@ static struct variable *hashaddr__find_variable(const struct hlist_head hashtabl > return NULL; > } > > -int cu__encode_btf(struct cu *cu, int verbose, bool force) > +int cu__encode_btf(struct cu *cu, int verbose, bool force, bool encode_symbols) > { > bool add_index_type = false; > uint32_t type_id_off; > @@ -314,6 +314,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force) > } > } > > + /* if there is no request to encode symbols, just finish. */ > + if (!encode_symbols) > + goto out; > > if (btfe->percpu_shndx == 0 || !btfe->symtab) > goto out; > diff --git a/btf_encoder.h b/btf_encoder.h > index bca2348..f2e95fe 100644 > --- a/btf_encoder.h > +++ b/btf_encoder.h > @@ -13,6 +13,6 @@ struct cu; > > int btf_encoder__encode(); > > -int cu__encode_btf(struct cu *cu, int verbose, bool force); > +int cu__encode_btf(struct cu *cu, int verbose, bool force, bool encode_symbols); > > #endif /* _BTF_ENCODER_H_ */ > diff --git a/pahole.c b/pahole.c > index e1cde61..079d951 100644 > --- a/pahole.c > +++ b/pahole.c > @@ -26,6 +26,7 @@ > static bool btf_encode; > static bool ctf_encode; > static bool first_obj_only; > +static bool btf_encode_symbols; > > static uint8_t class__include_anonymous; > static uint8_t class__include_nested_anonymous; > @@ -809,6 +810,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; > #define ARGP_header_type 314 > #define ARGP_size_bytes 315 > #define ARGP_range 316 > +#define ARGP_btf_encode_symbols 317 > > static const struct argp_option pahole__options[] = { > { > @@ -1087,6 +1089,11 @@ static const struct argp_option pahole__options[] = { > .key = 'J', > .doc = "Encode as BTF", > }, > + { > + .name = "btf_encode_symbols", > + .key = ARGP_btf_encode_symbols, > + .doc = "Encode global symbols in BTF. Off by default." > + }, > { > .name = "force", > .key = 'j', > @@ -1207,6 +1214,8 @@ static error_t pahole__options_parser(int key, char *arg, > conf.range = arg; break; > case ARGP_header_type: > conf.header_type = arg; break; > + case ARGP_btf_encode_symbols: > + btf_encode_symbols = true; break; > default: > return ARGP_ERR_UNKNOWN; > } > @@ -2352,7 +2361,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, > goto filter_it; > > if (btf_encode) { > - cu__encode_btf(cu, global_verbose, force); > + cu__encode_btf(cu, global_verbose, force, btf_encode_symbols); > return LSK__KEEPIT; > } > > -- > 2.28.0.681.g6f77f65b4e-goog > -- - Arnaldo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] btf_encoder: Make encoding vars into BTF optional. 2020-09-18 21:24 ` Arnaldo Carvalho de Melo @ 2020-09-18 21:26 ` Hao Luo 2020-09-18 21:40 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 9+ messages in thread From: Hao Luo @ 2020-09-18 21:26 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, dwarves, Yonghong Song How about '--btf_encode_vars'? It is actually encoding BTF_KIND_VAR. Hao On Fri, Sep 18, 2020 at 2:24 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Fri, Sep 18, 2020 at 01:40:57PM -0700, Hao Luo escreveu: > > Introduce an option --btf_encode_symbols to pahole so that pahole > > will only encode global symbols upon request. On the kernel side, > > we can add a Kconfig to turn on this feature. This would make the > > rollout of pahole v1.18 safe against potential bugs in the feature > > introduced in commit f3d9054ba8ff ("btf_encoder: Teach pahole to > > store percpu variables in vmlinux BTF."). > > 'encode_symbols' is too generic, please rename it to > encode_global_symbols or something like that. > > - Arnaldo > > > Signed-off-by: Hao Luo <haoluo@google.com> > > --- > > btf_encoder.c | 5 ++++- > > btf_encoder.h | 2 +- > > pahole.c | 11 ++++++++++- > > 3 files changed, 15 insertions(+), 3 deletions(-) > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > index 982f59d..54ffdd1 100644 > > --- a/btf_encoder.c > > +++ b/btf_encoder.c > > @@ -237,7 +237,7 @@ static struct variable *hashaddr__find_variable(const struct hlist_head hashtabl > > return NULL; > > } > > > > -int cu__encode_btf(struct cu *cu, int verbose, bool force) > > +int cu__encode_btf(struct cu *cu, int verbose, bool force, bool encode_symbols) > > { > > bool add_index_type = false; > > uint32_t type_id_off; > > @@ -314,6 +314,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force) > > } > > } > > > > + /* if there is no request to encode symbols, just finish. */ > > + if (!encode_symbols) > > + goto out; > > > > if (btfe->percpu_shndx == 0 || !btfe->symtab) > > goto out; > > diff --git a/btf_encoder.h b/btf_encoder.h > > index bca2348..f2e95fe 100644 > > --- a/btf_encoder.h > > +++ b/btf_encoder.h > > @@ -13,6 +13,6 @@ struct cu; > > > > int btf_encoder__encode(); > > > > -int cu__encode_btf(struct cu *cu, int verbose, bool force); > > +int cu__encode_btf(struct cu *cu, int verbose, bool force, bool encode_symbols); > > > > #endif /* _BTF_ENCODER_H_ */ > > diff --git a/pahole.c b/pahole.c > > index e1cde61..079d951 100644 > > --- a/pahole.c > > +++ b/pahole.c > > @@ -26,6 +26,7 @@ > > static bool btf_encode; > > static bool ctf_encode; > > static bool first_obj_only; > > +static bool btf_encode_symbols; > > > > static uint8_t class__include_anonymous; > > static uint8_t class__include_nested_anonymous; > > @@ -809,6 +810,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; > > #define ARGP_header_type 314 > > #define ARGP_size_bytes 315 > > #define ARGP_range 316 > > +#define ARGP_btf_encode_symbols 317 > > > > static const struct argp_option pahole__options[] = { > > { > > @@ -1087,6 +1089,11 @@ static const struct argp_option pahole__options[] = { > > .key = 'J', > > .doc = "Encode as BTF", > > }, > > + { > > + .name = "btf_encode_symbols", > > + .key = ARGP_btf_encode_symbols, > > + .doc = "Encode global symbols in BTF. Off by default." > > + }, > > { > > .name = "force", > > .key = 'j', > > @@ -1207,6 +1214,8 @@ static error_t pahole__options_parser(int key, char *arg, > > conf.range = arg; break; > > case ARGP_header_type: > > conf.header_type = arg; break; > > + case ARGP_btf_encode_symbols: > > + btf_encode_symbols = true; break; > > default: > > return ARGP_ERR_UNKNOWN; > > } > > @@ -2352,7 +2361,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, > > goto filter_it; > > > > if (btf_encode) { > > - cu__encode_btf(cu, global_verbose, force); > > + cu__encode_btf(cu, global_verbose, force, btf_encode_symbols); > > return LSK__KEEPIT; > > } > > > > -- > > 2.28.0.681.g6f77f65b4e-goog > > > > -- > > - Arnaldo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] btf_encoder: Make encoding vars into BTF optional. 2020-09-18 21:26 ` Hao Luo @ 2020-09-18 21:40 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 9+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-09-18 21:40 UTC (permalink / raw) To: Hao Luo Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, dwarves, Yonghong Song Em Fri, Sep 18, 2020 at 02:26:14PM -0700, Hao Luo escreveu: > How about '--btf_encode_vars'? It is actually encoding BTF_KIND_VAR. Sounds good as it references both its for BTF and that is for KIND_VAR. - Arnaldo > Hao > > On Fri, Sep 18, 2020 at 2:24 PM Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > > > Em Fri, Sep 18, 2020 at 01:40:57PM -0700, Hao Luo escreveu: > > > Introduce an option --btf_encode_symbols to pahole so that pahole > > > will only encode global symbols upon request. On the kernel side, > > > we can add a Kconfig to turn on this feature. This would make the > > > rollout of pahole v1.18 safe against potential bugs in the feature > > > introduced in commit f3d9054ba8ff ("btf_encoder: Teach pahole to > > > store percpu variables in vmlinux BTF."). > > > > 'encode_symbols' is too generic, please rename it to > > encode_global_symbols or something like that. > > > > - Arnaldo > > > > > Signed-off-by: Hao Luo <haoluo@google.com> > > > --- > > > btf_encoder.c | 5 ++++- > > > btf_encoder.h | 2 +- > > > pahole.c | 11 ++++++++++- > > > 3 files changed, 15 insertions(+), 3 deletions(-) > > > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > > index 982f59d..54ffdd1 100644 > > > --- a/btf_encoder.c > > > +++ b/btf_encoder.c > > > @@ -237,7 +237,7 @@ static struct variable *hashaddr__find_variable(const struct hlist_head hashtabl > > > return NULL; > > > } > > > > > > -int cu__encode_btf(struct cu *cu, int verbose, bool force) > > > +int cu__encode_btf(struct cu *cu, int verbose, bool force, bool encode_symbols) > > > { > > > bool add_index_type = false; > > > uint32_t type_id_off; > > > @@ -314,6 +314,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force) > > > } > > > } > > > > > > + /* if there is no request to encode symbols, just finish. */ > > > + if (!encode_symbols) > > > + goto out; > > > > > > if (btfe->percpu_shndx == 0 || !btfe->symtab) > > > goto out; > > > diff --git a/btf_encoder.h b/btf_encoder.h > > > index bca2348..f2e95fe 100644 > > > --- a/btf_encoder.h > > > +++ b/btf_encoder.h > > > @@ -13,6 +13,6 @@ struct cu; > > > > > > int btf_encoder__encode(); > > > > > > -int cu__encode_btf(struct cu *cu, int verbose, bool force); > > > +int cu__encode_btf(struct cu *cu, int verbose, bool force, bool encode_symbols); > > > > > > #endif /* _BTF_ENCODER_H_ */ > > > diff --git a/pahole.c b/pahole.c > > > index e1cde61..079d951 100644 > > > --- a/pahole.c > > > +++ b/pahole.c > > > @@ -26,6 +26,7 @@ > > > static bool btf_encode; > > > static bool ctf_encode; > > > static bool first_obj_only; > > > +static bool btf_encode_symbols; > > > > > > static uint8_t class__include_anonymous; > > > static uint8_t class__include_nested_anonymous; > > > @@ -809,6 +810,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; > > > #define ARGP_header_type 314 > > > #define ARGP_size_bytes 315 > > > #define ARGP_range 316 > > > +#define ARGP_btf_encode_symbols 317 > > > > > > static const struct argp_option pahole__options[] = { > > > { > > > @@ -1087,6 +1089,11 @@ static const struct argp_option pahole__options[] = { > > > .key = 'J', > > > .doc = "Encode as BTF", > > > }, > > > + { > > > + .name = "btf_encode_symbols", > > > + .key = ARGP_btf_encode_symbols, > > > + .doc = "Encode global symbols in BTF. Off by default." > > > + }, > > > { > > > .name = "force", > > > .key = 'j', > > > @@ -1207,6 +1214,8 @@ static error_t pahole__options_parser(int key, char *arg, > > > conf.range = arg; break; > > > case ARGP_header_type: > > > conf.header_type = arg; break; > > > + case ARGP_btf_encode_symbols: > > > + btf_encode_symbols = true; break; > > > default: > > > return ARGP_ERR_UNKNOWN; > > > } > > > @@ -2352,7 +2361,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, > > > goto filter_it; > > > > > > if (btf_encode) { > > > - cu__encode_btf(cu, global_verbose, force); > > > + cu__encode_btf(cu, global_verbose, force, btf_encode_symbols); > > > return LSK__KEEPIT; > > > } > > > > > > -- > > > 2.28.0.681.g6f77f65b4e-goog > > > > > > > -- > > > > - Arnaldo -- - Arnaldo ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] btf_encoder: Introduce option '--btf_encode_force' 2020-09-18 20:40 [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional Hao Luo 2020-09-18 20:40 ` [PATCH 1/2] btf_encoder: Make encoding " Hao Luo @ 2020-09-18 20:40 ` Hao Luo 2020-09-19 0:57 ` [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional Alexei Starovoitov 2 siblings, 0 replies; 9+ messages in thread From: Hao Luo @ 2020-09-18 20:40 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Andrii Nakryiko, Alexei Starovoitov, daniel, dwarves, yhs, Hao Luo Commit f3d9054ba8ff ("btf_encoder: Teach pahole to store percpu variables in vmlinux BTF.") introduced an option '-j' that makes best effort in emitting VAR entries in BTF. Before no one has been using this flag, replace the one-letter option '-j' with a full flag name '--btf_encode_force' to save '-j' for future uses. Signed-off-by: Hao Luo <haoluo@google.com> --- pahole.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pahole.c b/pahole.c index 079d951..3297729 100644 --- a/pahole.c +++ b/pahole.c @@ -27,6 +27,7 @@ static bool btf_encode; static bool ctf_encode; static bool first_obj_only; static bool btf_encode_symbols; +static bool btf_encode_force; static uint8_t class__include_anonymous; static uint8_t class__include_nested_anonymous; @@ -62,7 +63,6 @@ static int show_reorg_steps; static const char *class_name; static LIST_HEAD(class_names); static char separator = '\t'; -static bool force; static struct conf_fprintf conf = { .emit_stats = 1, @@ -811,6 +811,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; #define ARGP_size_bytes 315 #define ARGP_range 316 #define ARGP_btf_encode_symbols 317 +#define ARGP_btf_encode_force 318 static const struct argp_option pahole__options[] = { { @@ -1095,8 +1096,8 @@ static const struct argp_option pahole__options[] = { .doc = "Encode global symbols in BTF. Off by default." }, { - .name = "force", - .key = 'j', + .name = "btf_encode_force", + .key = ARGP_btf_encode_force, .doc = "Ignore those symbols found invalid when encoding BTF." }, { @@ -1143,7 +1144,6 @@ static error_t pahole__options_parser(int key, char *arg, case 'J': btf_encode = 1; conf_load.get_addr_info = true; no_bitfield_type_recode = true; break; - case 'j': force = true; break; case 'l': conf.show_first_biggest_size_base_type_member = 1; break; case 'M': conf.show_only_data_members = 1; break; case 'm': stats_formatter = nr_methods_formatter; break; @@ -1216,6 +1216,8 @@ static error_t pahole__options_parser(int key, char *arg, conf.header_type = arg; break; case ARGP_btf_encode_symbols: btf_encode_symbols = true; break; + case ARGP_btf_encode_force: + btf_encode_force = true; break; default: return ARGP_ERR_UNKNOWN; } @@ -2361,7 +2363,8 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, goto filter_it; if (btf_encode) { - cu__encode_btf(cu, global_verbose, force, btf_encode_symbols); + cu__encode_btf(cu, global_verbose, btf_encode_force, + btf_encode_symbols); return LSK__KEEPIT; } -- 2.28.0.681.g6f77f65b4e-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional. 2020-09-18 20:40 [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional Hao Luo 2020-09-18 20:40 ` [PATCH 1/2] btf_encoder: Make encoding " Hao Luo 2020-09-18 20:40 ` [PATCH 2/2] btf_encoder: Introduce option '--btf_encode_force' Hao Luo @ 2020-09-19 0:57 ` Alexei Starovoitov 2020-09-19 4:00 ` Hao Luo 2 siblings, 1 reply; 9+ messages in thread From: Alexei Starovoitov @ 2020-09-19 0:57 UTC (permalink / raw) To: Hao Luo Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Daniel Borkmann, dwarves, Yonghong Song On Fri, Sep 18, 2020 at 1:41 PM Hao Luo <haoluo@google.com> wrote: > > Previous commit f3d9054ba8ff ("btf_encoder: Teach pahole to store > percpu variables in vmlinux BTF.") introduced a feature in btf_encoder > that encodes global symbols in BTF. However, this feature is not > protected by any flag. In order to avoid surprises after Pahole v1.18 > rolls out, make this feature off by default and enable only upon > request. On the kernel side, we can add a Kconfig to enable this > new capability on an opt-in pattern. I think I've missed some earlier discussions and motivation for opt-in. Why opt-out is a problem? I would think the kernel build should always include percpu vars. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional. 2020-09-19 0:57 ` [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional Alexei Starovoitov @ 2020-09-19 4:00 ` Hao Luo 2020-09-21 12:27 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 9+ messages in thread From: Hao Luo @ 2020-09-19 4:00 UTC (permalink / raw) To: Alexei Starovoitov Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Daniel Borkmann, dwarves, Yonghong Song Hi Alexei, I was syncing with Arnaldo offline on whether the change in Pahole was enough for supporting percpu vars in BPF. While I think about it, I felt it's better to have a flag in pahole to toggle the feature on and off. My concern is mainly malformed BTF caused by the newly introduced VAR entries. I've tested over some kernel configurations and compilers but not confident enough. I haven't thought about opt-out. Maybe we should use opt-out instead of opt-in? Hao On Fri, Sep 18, 2020 at 5:58 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Sep 18, 2020 at 1:41 PM Hao Luo <haoluo@google.com> wrote: > > > > Previous commit f3d9054ba8ff ("btf_encoder: Teach pahole to store > > percpu variables in vmlinux BTF.") introduced a feature in btf_encoder > > that encodes global symbols in BTF. However, this feature is not > > protected by any flag. In order to avoid surprises after Pahole v1.18 > > rolls out, make this feature off by default and enable only upon > > request. On the kernel side, we can add a Kconfig to enable this > > new capability on an opt-in pattern. > > I think I've missed some earlier discussions and motivation for opt-in. > Why opt-out is a problem? > I would think the kernel build should always include percpu vars. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional. 2020-09-19 4:00 ` Hao Luo @ 2020-09-21 12:27 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 9+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-09-21 12:27 UTC (permalink / raw) To: Hao Luo Cc: Alexei Starovoitov, Arnaldo Carvalho de Melo, Andrii Nakryiko, Daniel Borkmann, dwarves, Yonghong Song Em Fri, Sep 18, 2020 at 09:00:10PM -0700, Hao Luo escreveu: > Hi Alexei, > > I was syncing with Arnaldo offline on whether the change in Pahole was > enough for supporting percpu vars in BPF. While I think about it, I > felt it's better to have a flag in pahole to toggle the feature on and > off. My concern is mainly malformed BTF caused by the newly introduced > VAR entries. I've tested over some kernel configurations and compilers > but not confident enough. > > I haven't thought about opt-out. Maybe we should use opt-out instead of opt-in? Yeah, having an opt-out should be useful as a debugging technique. - Arnaldo > Hao > > On Fri, Sep 18, 2020 at 5:58 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Fri, Sep 18, 2020 at 1:41 PM Hao Luo <haoluo@google.com> wrote: > > > > > > Previous commit f3d9054ba8ff ("btf_encoder: Teach pahole to store > > > percpu variables in vmlinux BTF.") introduced a feature in btf_encoder > > > that encodes global symbols in BTF. However, this feature is not > > > protected by any flag. In order to avoid surprises after Pahole v1.18 > > > rolls out, make this feature off by default and enable only upon > > > request. On the kernel side, we can add a Kconfig to enable this > > > new capability on an opt-in pattern. > > > > I think I've missed some earlier discussions and motivation for opt-in. > > Why opt-out is a problem? > > I would think the kernel build should always include percpu vars. -- - Arnaldo ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-09-21 12:27 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-18 20:40 [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional Hao Luo 2020-09-18 20:40 ` [PATCH 1/2] btf_encoder: Make encoding " Hao Luo 2020-09-18 21:24 ` Arnaldo Carvalho de Melo 2020-09-18 21:26 ` Hao Luo 2020-09-18 21:40 ` Arnaldo Carvalho de Melo 2020-09-18 20:40 ` [PATCH 2/2] btf_encoder: Introduce option '--btf_encode_force' Hao Luo 2020-09-19 0:57 ` [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional Alexei Starovoitov 2020-09-19 4:00 ` Hao Luo 2020-09-21 12:27 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).