[PATCH 0/2] pahole: Make encoding percpu vars into BTF optional.

[PATCH 0/2] pahole: Make encoding percpu vars into BTF optional.

[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

[PATCH 1/2] btf_encoder: Make encoding vars into BTF optional.

[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

Re: [PATCH 1/2] btf_encoder: Make encoding vars into BTF optional.

[Arnaldo Carvalho de Melo]

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

Re: [PATCH 1/2] btf_encoder: Make encoding vars into BTF optional.

[Hao Luo]

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

Re: [PATCH 1/2] btf_encoder: Make encoding vars into BTF optional.

[Arnaldo Carvalho de Melo]

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

[PATCH 2/2] btf_encoder: Introduce option '--btf_encode_force'

[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

Re: [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional.

[Alexei Starovoitov]

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.

Re: [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional.

[Hao Luo]

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.

Re: [PATCH 0/2] pahole: Make encoding percpu vars into BTF optional.

[Arnaldo Carvalho de Melo]

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