* [PATCH] btf: Add --btf_prefix flag @ 2021-05-17 12:06 程书意 2021-05-17 14:42 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 17+ messages in thread From: 程书意 @ 2021-05-17 12:06 UTC (permalink / raw) To: dwarves; +Cc: wenan.mao To solve problems similar to _RH_KABI_REPLACE, _RH_KABI_REPLACE makes many structures have different names, resulting in a particularly large vmlinux btf. For example, running ./pahole -J vmlinux-3.10.0-1160.el7.x86_64 without --btf_prefix flag, the running time is: real 8m28.912s user 8m27.271s sys 0m1.471s And the size of the generated btf segment is 30678240 bytes. After adding the patch, running ./pahole --btf_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. The running time of the command is: real 0m19.634s user 0m18.457s sys 0m1.169s The size of the generated btf segment is 3117719 bytes. Thanks. Signed-off-by: chengshuyi <chengshuyi@linux.alibaba.com> --- pahole.c | 10 ++++++++++ pahole_strings.h | 2 ++ strings.c | 9 +++++++-- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pahole.c b/pahole.c index dc40ccf..0b4f4ca 100644 --- a/pahole.c +++ b/pahole.c @@ -24,6 +24,7 @@ #include "btf_encoder.h" #include "libbtf.h" #include "lib/bpf/src/libbpf.h" +#include "pahole_strings.h" static bool btf_encode; static bool ctf_encode; @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; #define ARGP_btf_gen_floats 322 #define ARGP_btf_gen_all 323 #define ARGP_with_flexible_array 324 +#define ARGP_btf_prefix 325 static const struct argp_option pahole__options[] = { { @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { .doc = "Path to the base BTF file", }, { + .name = "btf_prefix", + .key = ARGP_btf_prefix, + .arg = "STRING", + .doc = "Strings with the same prefix are considered the same.", + }, + { .name = "btf_encode", .key = 'J', .doc = "Encode as BTF", @@ -1297,6 +1305,8 @@ static error_t pahole__options_parser(int key, char *arg, btf_encode_force = true; break; case ARGP_btf_base: base_btf_file = arg; break; + case ARGP_btf_prefix: + btf_prefix = arg; break; case ARGP_numeric_version: print_numeric_version = true; break; case ARGP_btf_gen_floats: diff --git a/pahole_strings.h b/pahole_strings.h index 522fbf2..bf3dc7c 100644 --- a/pahole_strings.h +++ b/pahole_strings.h @@ -14,6 +14,8 @@ struct strings { struct btf *btf; }; +extern const char *btf_prefix; + struct strings *strings__new(void); void strings__delete(struct strings *strings); diff --git a/strings.c b/strings.c index d37f49d..911ce25 100644 --- a/strings.c +++ b/strings.c @@ -16,6 +16,9 @@ #include "dutil.h" #include "lib/bpf/src/libbpf.h" +#include "libbtf.h" + +const char *btf_prefix; struct strings *strings__new(void) { @@ -47,8 +50,10 @@ strings_t strings__add(struct strings *strs, const char *str) if (str == NULL) return 0; - - index = btf__add_str(strs->btf, str); + if(btf_prefix && strncmp(str,btf_prefix,strlen(btf_prefix))==0) + index = btf__add_str(strs->btf, btf_prefix); + else + index = btf__add_str(strs->btf, str); if (index < 0) return 0; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] btf: Add --btf_prefix flag 2021-05-17 12:06 [PATCH] btf: Add --btf_prefix flag 程书意 @ 2021-05-17 14:42 ` Arnaldo Carvalho de Melo 2021-05-18 9:02 ` chengshuyi 0 siblings, 1 reply; 17+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-05-17 14:42 UTC (permalink / raw) To: 程书意; +Cc: dwarves, wenan.mao, Jiri Olsa, Andrii Nakryiko Em Mon, May 17, 2021 at 08:06:30PM +0800, 程书意 escreveu: > To solve problems similar to _RH_KABI_REPLACE, _RH_KABI_REPLACE makes many Can you explain what is _RH_KABI_REPLACE, why it is needed so that people unfamiliar with it can make sense of your patch? Also why "btf_prefix" when this is related to this other feature? Can you find a better name? You also forgot to update the man page at man-pages/pahole.1. - Arnaldo > structures have different names, resulting in a > particularly large vmlinux btf. For example, running ./pahole -J > vmlinux-3.10.0-1160.el7.x86_64 without --btf_prefix flag, > the running time is: > real 8m28.912s > user 8m27.271s > sys 0m1.471s > And the size of the generated btf segment is 30678240 bytes. > > After adding the patch, running ./pahole > --btf_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. The > running > time of the command is: > real 0m19.634s > user 0m18.457s > sys 0m1.169s > The size of the generated btf segment is 3117719 bytes. > > Thanks. > > Signed-off-by: chengshuyi <chengshuyi@linux.alibaba.com> > --- > pahole.c | 10 ++++++++++ > pahole_strings.h | 2 ++ > strings.c | 9 +++++++-- > 3 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/pahole.c b/pahole.c > index dc40ccf..0b4f4ca 100644 > --- a/pahole.c > +++ b/pahole.c > @@ -24,6 +24,7 @@ > #include "btf_encoder.h" > #include "libbtf.h" > #include "lib/bpf/src/libbpf.h" > +#include "pahole_strings.h" > > static bool btf_encode; > static bool ctf_encode; > @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; > #define ARGP_btf_gen_floats 322 > #define ARGP_btf_gen_all 323 > #define ARGP_with_flexible_array 324 > +#define ARGP_btf_prefix 325 > > static const struct argp_option pahole__options[] = { > { > @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { > .doc = "Path to the base BTF file", > }, > { > + .name = "btf_prefix", > + .key = ARGP_btf_prefix, > + .arg = "STRING", > + .doc = "Strings with the same prefix are considered the same.", > + }, > + { > .name = "btf_encode", > .key = 'J', > .doc = "Encode as BTF", > @@ -1297,6 +1305,8 @@ static error_t pahole__options_parser(int key, char > *arg, > btf_encode_force = true; break; > case ARGP_btf_base: > base_btf_file = arg; break; > + case ARGP_btf_prefix: > + btf_prefix = arg; break; > case ARGP_numeric_version: > print_numeric_version = true; break; > case ARGP_btf_gen_floats: > diff --git a/pahole_strings.h b/pahole_strings.h > index 522fbf2..bf3dc7c 100644 > --- a/pahole_strings.h > +++ b/pahole_strings.h > @@ -14,6 +14,8 @@ struct strings { > struct btf *btf; > }; > > +extern const char *btf_prefix; > + > struct strings *strings__new(void); > > void strings__delete(struct strings *strings); > diff --git a/strings.c b/strings.c > index d37f49d..911ce25 100644 > --- a/strings.c > +++ b/strings.c > @@ -16,6 +16,9 @@ > > #include "dutil.h" > #include "lib/bpf/src/libbpf.h" > +#include "libbtf.h" Why do you need to add this new include? I guess you meant including pahole_strings.h to get the forward declaration for 'btf_prefix'? > + > +const char *btf_prefix; > > struct strings *strings__new(void) > { > @@ -47,8 +50,10 @@ strings_t strings__add(struct strings *strs, const char > *str) > > if (str == NULL) > return 0; > - > - index = btf__add_str(strs->btf, str); > + if(btf_prefix && strncmp(str,btf_prefix,strlen(btf_prefix))==0) Please also follow the existing coding style, i.e. use a space after 'if' and also after the commas. - Arnaldo > + index = btf__add_str(strs->btf, btf_prefix); > + else > + index = btf__add_str(strs->btf, str); > if (index < 0) > return 0; > > -- > 1.8.3.1 > > -- - Arnaldo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] btf: Add --btf_prefix flag 2021-05-17 14:42 ` Arnaldo Carvalho de Melo @ 2021-05-18 9:02 ` chengshuyi 2021-05-18 12:49 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 17+ messages in thread From: chengshuyi @ 2021-05-18 9:02 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: dwarves, wenan.mao, Jiri Olsa, Andrii Nakryiko On 2021/5/17 10:42PM, Arnaldo Carvalho de Melo wrote: > Em Mon, May 17, 2021 at 08:06:30PM +0800, 程书意 escreveu: >> To solve problems similar to _RH_KABI_REPLACE, _RH_KABI_REPLACE makes many > Can you explain what is _RH_KABI_REPLACE, why it is needed so that > people unfamiliar with it can make sense of your patch? The _RH_KABI_REPLACE(_orig, _new) macros perserve size alignment and kabi agreement between _orig and _new.Below is the definition of this macro: # define _RH_KABI_REPLACE(_orig, _new) \ union { \ _new; \ struct { \ _orig; \ } __UNIQUE_ID(rh_kabi_hide); \ __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ } __UNIQUE_ID uses the __COUNTER__ macro, and the __COUNTER__ macro is automatically incremented by 1 every time it is precompiled. Therefore, in different compilation units, the same structure has different names.Here is a concrete example: struct acpi_dev_node { union { struct acpi_device *companion; struct { void *handle; } __UNIQUE_ID_rh_kabi_hide29; union { }; }; }; struct acpi_dev_node { union { struct acpi_device *companion; struct { void *handle; } __UNIQUE_ID_rh_kabi_hide31; union { }; }; }; Finally, it will cause the btf algorithm to de-duplication efficiency is not high, and time-consuming. > > Also why "btf_prefix" when this is related to this other feature? Can > you find a better name? "btf_prefix" means that if two strings have the same prefix, treat them as the same string.From the above example,if btf_prefix is __UNIQUE_ID_rh_kabi_hide, then __UNIQUE_ID_rh_kabi_hide29 and __UNIQUE_ID_rh_kabi_hide31 are equal.Maybe "btf_kabi_prefix_string" is better > You also forgot to update the man page at man-pages/pahole.1. > > - Arnaldo Yes i forgot. >> structures have different names, resulting in a >> particularly large vmlinux btf. For example, running ./pahole -J >> vmlinux-3.10.0-1160.el7.x86_64 without --btf_prefix flag, >> the running time is: >> real 8m28.912s >> user 8m27.271s >> sys 0m1.471s >> And the size of the generated btf segment is 30678240 bytes. >> >> After adding the patch, running ./pahole >> --btf_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. The >> running >> time of the command is: >> real 0m19.634s >> user 0m18.457s >> sys 0m1.169s >> The size of the generated btf segment is 3117719 bytes. >> >> Thanks. >> >> Signed-off-by: chengshuyi<chengshuyi@linux.alibaba.com> >> --- >> pahole.c | 10 ++++++++++ >> pahole_strings.h | 2 ++ >> strings.c | 9 +++++++-- >> 3 files changed, 19 insertions(+), 2 deletions(-) >> >> diff --git a/pahole.c b/pahole.c >> index dc40ccf..0b4f4ca 100644 >> --- a/pahole.c >> +++ b/pahole.c >> @@ -24,6 +24,7 @@ >> #include "btf_encoder.h" >> #include "libbtf.h" >> #include "lib/bpf/src/libbpf.h" >> +#include "pahole_strings.h" >> >> static bool btf_encode; >> static bool ctf_encode; >> @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; >> #define ARGP_btf_gen_floats 322 >> #define ARGP_btf_gen_all 323 >> #define ARGP_with_flexible_array 324 >> +#define ARGP_btf_prefix 325 >> >> static const struct argp_option pahole__options[] = { >> { >> @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { >> .doc = "Path to the base BTF file", >> }, >> { >> + .name = "btf_prefix", >> + .key = ARGP_btf_prefix, >> + .arg = "STRING", >> + .doc = "Strings with the same prefix are considered the same.", >> + }, >> + { >> .name = "btf_encode", >> .key = 'J', >> .doc = "Encode as BTF", @@ -1297,6 +1305,8 @@ static error_t pahole__options_parser(int >> key, char *arg, btf_encode_force = true; break; >> case ARGP_btf_base: base_btf_file = arg; break; >> + case ARGP_btf_prefix: + btf_prefix = arg; break; >> case ARGP_numeric_version: print_numeric_version = >> true; break; case ARGP_btf_gen_floats: diff --git >> a/pahole_strings.h b/pahole_strings.h index 522fbf2..bf3dc7c 100644 >> --- a/pahole_strings.h +++ b/pahole_strings.h @@ -14,6 +14,8 @@ >> struct strings { struct btf *btf; }; +extern const char >> *btf_prefix; + struct strings *strings__new(void); void >> strings__delete(struct strings *strings); diff --git a/strings.c >> b/strings.c index d37f49d..911ce25 100644 --- a/strings.c +++ >> b/strings.c @@ -16,6 +16,9 @@ #include "dutil.h" >> #include "lib/bpf/src/libbpf.h" >> +#include "libbtf.h" > Why do you need to add this new include? I guess you meant including > pahole_strings.h to get the forward declaration for 'btf_prefix'? It is of no use, I forgot to delete it. >> + >> +const char *btf_prefix; >> >> struct strings *strings__new(void) >> { >> @@ -47,8 +50,10 @@ strings_t strings__add(struct strings *strs, const char >> *str) >> >> if (str == NULL) >> return 0; >> - >> - index = btf__add_str(strs->btf, str); >> + if(btf_prefix && strncmp(str,btf_prefix,strlen(btf_prefix))==0) > Please also follow the existing coding style, i.e. use a space after > 'if' and also after the commas. > > - Arnaldo > Okay, I know. If you want to receive this patch, I will send a complete patch later. >> + index = btf__add_str(strs->btf, btf_prefix); >> + else >> + index = btf__add_str(strs->btf, str); >> if (index < 0) >> return 0; >> >> -- >> 1.8.3.1 >> >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] btf: Add --btf_prefix flag 2021-05-18 9:02 ` chengshuyi @ 2021-05-18 12:49 ` Arnaldo Carvalho de Melo 2021-05-18 18:30 ` Andrii Nakryiko 0 siblings, 1 reply; 17+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-05-18 12:49 UTC (permalink / raw) To: chengshuyi; +Cc: dwarves, wenan.mao, Jiri Olsa, Andrii Nakryiko Em Tue, May 18, 2021 at 05:02:26PM +0800, chengshuyi escreveu: > > On 2021/5/17 10:42PM, Arnaldo Carvalho de Melo wrote: > > Em Mon, May 17, 2021 at 08:06:30PM +0800, 程书意 escreveu: > > > To solve problems similar to _RH_KABI_REPLACE, _RH_KABI_REPLACE makes many > > Can you explain what is _RH_KABI_REPLACE, why it is needed so that > > people unfamiliar with it can make sense of your patch? > > The _RH_KABI_REPLACE(_orig, _new) macros perserve size alignment and kabi > agreement between _orig and _new.Below is the definition of this macro: > > # define _RH_KABI_REPLACE(_orig, _new) \ > union { \ > _new; \ > struct { \ > _orig; \ > } __UNIQUE_ID(rh_kabi_hide); \ > __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ > } > > > __UNIQUE_ID uses the __COUNTER__ macro, and the __COUNTER__ macro is > automatically incremented by 1 every time it is precompiled. Therefore, in > different compilation units, the same structure has different names.Here is > a concrete example: > > struct acpi_dev_node { > union { > struct acpi_device *companion; > struct { > void *handle; > } __UNIQUE_ID_rh_kabi_hide29; > union { }; > }; > }; > struct acpi_dev_node { > union { > struct acpi_device *companion; > struct { > void *handle; > } __UNIQUE_ID_rh_kabi_hide31; > union { }; > }; > }; > > Finally, it will cause the btf algorithm to de-duplication efficiency is not > high, and time-consuming. > > > > > Also why "btf_prefix" when this is related to this other feature? Can > > you find a better name? > > "btf_prefix" means that if two strings have the same prefix, treat them as I understood what is its purpose, but pahole is not just about DWARF and BTF, it has an admittedly unmaintained CTF encoder that predates BTF and was even used as an starting point for the BTF encoder, so using BTF isn't appropriate, perhaps --common_prefix? Or even --kabi_prefix to make it even more explicit? - Arnaldo > the same string.From the above example,if btf_prefix is > __UNIQUE_ID_rh_kabi_hide, then __UNIQUE_ID_rh_kabi_hide29 and > __UNIQUE_ID_rh_kabi_hide31 are equal.Maybe "btf_kabi_prefix_string" is > better > > > You also forgot to update the man page at man-pages/pahole.1. > > > > - Arnaldo > > Yes i forgot. > > > > structures have different names, resulting in a > > > particularly large vmlinux btf. For example, running ./pahole -J > > > vmlinux-3.10.0-1160.el7.x86_64 without --btf_prefix flag, > > > the running time is: > > > real 8m28.912s > > > user 8m27.271s > > > sys 0m1.471s > > > And the size of the generated btf segment is 30678240 bytes. > > > > > > After adding the patch, running ./pahole > > > --btf_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. The > > > running > > > time of the command is: > > > real 0m19.634s > > > user 0m18.457s > > > sys 0m1.169s > > > The size of the generated btf segment is 3117719 bytes. > > > > > > Thanks. > > > > > > Signed-off-by: chengshuyi<chengshuyi@linux.alibaba.com> > > > --- > > > pahole.c | 10 ++++++++++ > > > pahole_strings.h | 2 ++ > > > strings.c | 9 +++++++-- > > > 3 files changed, 19 insertions(+), 2 deletions(-) > > > > > > diff --git a/pahole.c b/pahole.c > > > index dc40ccf..0b4f4ca 100644 > > > --- a/pahole.c > > > +++ b/pahole.c > > > @@ -24,6 +24,7 @@ > > > #include "btf_encoder.h" > > > #include "libbtf.h" > > > #include "lib/bpf/src/libbpf.h" > > > +#include "pahole_strings.h" > > > > > > static bool btf_encode; > > > static bool ctf_encode; > > > @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; > > > #define ARGP_btf_gen_floats 322 > > > #define ARGP_btf_gen_all 323 > > > #define ARGP_with_flexible_array 324 > > > +#define ARGP_btf_prefix 325 > > > > > > static const struct argp_option pahole__options[] = { > > > { > > > @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { > > > .doc = "Path to the base BTF file", > > > }, > > > { > > > + .name = "btf_prefix", > > > + .key = ARGP_btf_prefix, > > > + .arg = "STRING", > > > + .doc = "Strings with the same prefix are considered the same.", > > > + }, > > > + { > > > .name = "btf_encode", > > > .key = 'J', > > > .doc = "Encode as BTF", @@ -1297,6 +1305,8 @@ static > > > error_t pahole__options_parser(int key, char *arg, > > > btf_encode_force = true; break; case ARGP_btf_base: > > > base_btf_file = arg; break; + case > > > ARGP_btf_prefix: + btf_prefix = arg; break; case > > > ARGP_numeric_version: print_numeric_version = true; > > > break; case ARGP_btf_gen_floats: diff --git a/pahole_strings.h > > > b/pahole_strings.h index 522fbf2..bf3dc7c 100644 --- > > > a/pahole_strings.h +++ b/pahole_strings.h @@ -14,6 +14,8 @@ struct > > > strings { struct btf *btf; }; +extern const char *btf_prefix; > > > + struct strings *strings__new(void); void strings__delete(struct > > > strings *strings); diff --git a/strings.c b/strings.c index > > > d37f49d..911ce25 100644 --- a/strings.c +++ b/strings.c @@ -16,6 > > > +16,9 @@ #include "dutil.h" > > > #include "lib/bpf/src/libbpf.h" > > > +#include "libbtf.h" > > Why do you need to add this new include? I guess you meant including > > pahole_strings.h to get the forward declaration for 'btf_prefix'? > > It is of no use, I forgot to delete it. > > > > + > > > +const char *btf_prefix; > > > > > > struct strings *strings__new(void) > > > { > > > @@ -47,8 +50,10 @@ strings_t strings__add(struct strings *strs, const char > > > *str) > > > > > > if (str == NULL) > > > return 0; > > > - > > > - index = btf__add_str(strs->btf, str); > > > + if(btf_prefix && strncmp(str,btf_prefix,strlen(btf_prefix))==0) > > Please also follow the existing coding style, i.e. use a space after > > 'if' and also after the commas. > > > > - Arnaldo > > > > Okay, I know. If you want to receive this patch, I will send a complete > patch later. > > > > + index = btf__add_str(strs->btf, btf_prefix); > > > + else > > > + index = btf__add_str(strs->btf, str); > > > if (index < 0) > > > return 0; > > > > > > -- > > > 1.8.3.1 > > > > > > -- - Arnaldo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] btf: Add --btf_prefix flag 2021-05-18 12:49 ` Arnaldo Carvalho de Melo @ 2021-05-18 18:30 ` Andrii Nakryiko 2021-05-19 2:44 ` [PATCH v2] pahole: Add --kabi_prefix flag Shuyi Cheng 0 siblings, 1 reply; 17+ messages in thread From: Andrii Nakryiko @ 2021-05-18 18:30 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: chengshuyi, dwarves, wenan.mao, Jiri Olsa On Tue, May 18, 2021 at 5:49 AM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > Em Tue, May 18, 2021 at 05:02:26PM +0800, chengshuyi escreveu: > > > > On 2021/5/17 10:42PM, Arnaldo Carvalho de Melo wrote: > > > Em Mon, May 17, 2021 at 08:06:30PM +0800, 程书意 escreveu: > > > > To solve problems similar to _RH_KABI_REPLACE, _RH_KABI_REPLACE makes many > > > Can you explain what is _RH_KABI_REPLACE, why it is needed so that > > > people unfamiliar with it can make sense of your patch? > > > > The _RH_KABI_REPLACE(_orig, _new) macros perserve size alignment and kabi > > agreement between _orig and _new.Below is the definition of this macro: > > > > # define _RH_KABI_REPLACE(_orig, _new) \ > > union { \ > > _new; \ > > struct { \ > > _orig; \ > > } __UNIQUE_ID(rh_kabi_hide); \ > > __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ > > } > > > > > > __UNIQUE_ID uses the __COUNTER__ macro, and the __COUNTER__ macro is > > automatically incremented by 1 every time it is precompiled. Therefore, in > > different compilation units, the same structure has different names.Here is > > a concrete example: > > > > struct acpi_dev_node { > > union { > > struct acpi_device *companion; > > struct { > > void *handle; > > } __UNIQUE_ID_rh_kabi_hide29; > > union { }; > > }; > > }; > > struct acpi_dev_node { > > union { > > struct acpi_device *companion; > > struct { > > void *handle; > > } __UNIQUE_ID_rh_kabi_hide31; > > union { }; > > }; > > }; > > > > Finally, it will cause the btf algorithm to de-duplication efficiency is not > > high, and time-consuming. > > > > > > > > Also why "btf_prefix" when this is related to this other feature? Can > > > you find a better name? > > > > "btf_prefix" means that if two strings have the same prefix, treat them as > > I understood what is its purpose, but pahole is not just about DWARF > and BTF, it has an admittedly unmaintained CTF encoder that predates BTF > and was even used as an starting point for the BTF encoder, so using BTF > isn't appropriate, perhaps --common_prefix? Or even --kabi_prefix to > make it even more explicit? > +1 for --kabi_prefix > - Arnaldo > > > the same string.From the above example,if btf_prefix is > > __UNIQUE_ID_rh_kabi_hide, then __UNIQUE_ID_rh_kabi_hide29 and > > __UNIQUE_ID_rh_kabi_hide31 are equal.Maybe "btf_kabi_prefix_string" is > > better > > > > > You also forgot to update the man page at man-pages/pahole.1. > > > > > > - Arnaldo > > > > Yes i forgot. > > > > > > structures have different names, resulting in a > > > > particularly large vmlinux btf. For example, running ./pahole -J > > > > vmlinux-3.10.0-1160.el7.x86_64 without --btf_prefix flag, > > > > the running time is: > > > > real 8m28.912s > > > > user 8m27.271s > > > > sys 0m1.471s > > > > And the size of the generated btf segment is 30678240 bytes. > > > > > > > > After adding the patch, running ./pahole > > > > --btf_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. The > > > > running > > > > time of the command is: > > > > real 0m19.634s > > > > user 0m18.457s > > > > sys 0m1.169s > > > > The size of the generated btf segment is 3117719 bytes. > > > > > > > > Thanks. > > > > > > > > Signed-off-by: chengshuyi<chengshuyi@linux.alibaba.com> > > > > --- > > > > pahole.c | 10 ++++++++++ > > > > pahole_strings.h | 2 ++ > > > > strings.c | 9 +++++++-- > > > > 3 files changed, 19 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/pahole.c b/pahole.c > > > > index dc40ccf..0b4f4ca 100644 > > > > --- a/pahole.c > > > > +++ b/pahole.c > > > > @@ -24,6 +24,7 @@ > > > > #include "btf_encoder.h" > > > > #include "libbtf.h" > > > > #include "lib/bpf/src/libbpf.h" > > > > +#include "pahole_strings.h" > > > > > > > > static bool btf_encode; > > > > static bool ctf_encode; > > > > @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; > > > > #define ARGP_btf_gen_floats 322 > > > > #define ARGP_btf_gen_all 323 > > > > #define ARGP_with_flexible_array 324 > > > > +#define ARGP_btf_prefix 325 > > > > > > > > static const struct argp_option pahole__options[] = { > > > > { > > > > @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { > > > > .doc = "Path to the base BTF file", > > > > }, > > > > { > > > > + .name = "btf_prefix", > > > > + .key = ARGP_btf_prefix, > > > > + .arg = "STRING", > > > > + .doc = "Strings with the same prefix are considered the same.", > > > > + }, > > > > + { > > > > .name = "btf_encode", > > > > .key = 'J', > > > > .doc = "Encode as BTF", @@ -1297,6 +1305,8 @@ static > > > > error_t pahole__options_parser(int key, char *arg, > > > > btf_encode_force = true; break; case ARGP_btf_base: > > > > base_btf_file = arg; break; + case > > > > ARGP_btf_prefix: + btf_prefix = arg; break; case > > > > ARGP_numeric_version: print_numeric_version = true; > > > > break; case ARGP_btf_gen_floats: diff --git a/pahole_strings.h > > > > b/pahole_strings.h index 522fbf2..bf3dc7c 100644 --- > > > > a/pahole_strings.h +++ b/pahole_strings.h @@ -14,6 +14,8 @@ struct > > > > strings { struct btf *btf; }; +extern const char *btf_prefix; > > > > + struct strings *strings__new(void); void strings__delete(struct > > > > strings *strings); diff --git a/strings.c b/strings.c index > > > > d37f49d..911ce25 100644 --- a/strings.c +++ b/strings.c @@ -16,6 > > > > +16,9 @@ #include "dutil.h" > > > > #include "lib/bpf/src/libbpf.h" > > > > +#include "libbtf.h" > > > Why do you need to add this new include? I guess you meant including > > > pahole_strings.h to get the forward declaration for 'btf_prefix'? > > > > It is of no use, I forgot to delete it. > > > > > > + > > > > +const char *btf_prefix; > > > > > > > > struct strings *strings__new(void) > > > > { > > > > @@ -47,8 +50,10 @@ strings_t strings__add(struct strings *strs, const char > > > > *str) > > > > > > > > if (str == NULL) > > > > return 0; > > > > - > > > > - index = btf__add_str(strs->btf, str); > > > > + if(btf_prefix && strncmp(str,btf_prefix,strlen(btf_prefix))==0) > > > Please also follow the existing coding style, i.e. use a space after > > > 'if' and also after the commas. > > > > > > - Arnaldo > > > > > > > Okay, I know. If you want to receive this patch, I will send a complete > > patch later. > > > > > > + index = btf__add_str(strs->btf, btf_prefix); > > > > + else > > > > + index = btf__add_str(strs->btf, str); > > > > if (index < 0) > > > > return 0; > > > > > > > > -- > > > > 1.8.3.1 > > > > > > > > > > -- > > - Arnaldo ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] pahole: Add --kabi_prefix flag 2021-05-18 18:30 ` Andrii Nakryiko @ 2021-05-19 2:44 ` Shuyi Cheng 2021-05-19 20:07 ` Jiri Olsa 2021-05-20 15:25 ` Jiri Olsa 0 siblings, 2 replies; 17+ messages in thread From: Shuyi Cheng @ 2021-05-19 2:44 UTC (permalink / raw) To: Andrii Nakryiko, Arnaldo Carvalho de Melo; +Cc: dwarves, wenan.mao, Jiri Olsa To solve problems similar to _RH_KABI_REPLACE. The _RH_KABI_REPLACE(_orig, _new) macros perserve size alignment and kabi agreement between _orig and _new.Below is the definition of this macro: # define _RH_KABI_REPLACE(_orig, _new) \ union { \ _new; \ struct { \ _orig; \ } __UNIQUE_ID(rh_kabi_hide); \ __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ } __UNIQUE_ID uses the __COUNTER__ macro, and the __COUNTER__ macro is automatically incremented by 1 every time it is precompiled. Therefore, in different compilation units, the same structure has different names.Here is a concrete example: struct acpi_dev_node { union { struct acpi_device *companion; struct { void *handle; } __UNIQUE_ID_rh_kabi_hide29; union { }; }; }; struct acpi_dev_node { union { struct acpi_device *companion; struct { void *handle; } __UNIQUE_ID_rh_kabi_hide31; union { }; }; }; Finally, it will cause the btf algorithm to de-duplication efficiency is not high, and time-consuming. For example, running ./pahole -J vmlinux-3.10.0-1160.el7.x86_64 without --kabi_prefix flag, the running time is: real 8m28.912s user 8m27.271s sys 0m1.471s And the size of the generated btf segment is 30678240 bytes. After adding the patch, running ./pahole --kabi_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. The running time of the command is: real 0m19.634s user 0m18.457s sys 0m1.169s And the size of the generated btf segment is 3117719 bytes. --- v1:https://lore.kernel.org/dwarves/CAEf4Bzazh4RNCY3rGRRXfO2wJ7DSiMx8w+61B_hjhu9FrOffpQ@mail.gmail.com v1->v2: --Change btf_prefix to --kabi_prefix. --Add man page. --Add space after if and comma. man-pages/pahole.1 | 4 ++++ pahole.c | 10 ++++++++++ pahole_strings.h | 2 ++ strings.c | 9 ++++++++- 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 index a10738f..2659fe6 100644 --- a/man-pages/pahole.1 +++ b/man-pages/pahole.1 @@ -340,6 +340,10 @@ Show a traditional string version, i.e.: "v1.18". Show a numeric only version, suitable for use in Makefiles and scripts where one wants to know what if the installed version has some feature, i.e.: 118 instead of "v1.18". +.TP +.B \-\-kabi_prefix=STRING +When the prefix of the string is STRING, treat the string as STRING. + .SH NOTES To enable the generation of debugging information in the Linux kernel build diff --git a/pahole.c b/pahole.c index dc40ccf..6a700d9 100644 --- a/pahole.c +++ b/pahole.c @@ -24,6 +24,7 @@ #include "btf_encoder.h" #include "libbtf.h" #include "lib/bpf/src/libbpf.h" +#include "pahole_strings.h" static bool btf_encode; static bool ctf_encode; @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; #define ARGP_btf_gen_floats 322 #define ARGP_btf_gen_all 323 #define ARGP_with_flexible_array 324 +#define ARGP_kabi_prefix 325 static const struct argp_option pahole__options[] = { { @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { .doc = "Path to the base BTF file", }, { + .name = "kabi_prefix", + .key = ARGP_kabi_prefix, + .arg = "STRING", + .doc = "When the prefix of the string is STRING, treat the string as STRING.", + }, + { .name = "btf_encode", .key = 'J', .doc = "Encode as BTF", @@ -1297,6 +1305,8 @@ static error_t pahole__options_parser(int key, char *arg, btf_encode_force = true; break; case ARGP_btf_base: base_btf_file = arg; break; + case ARGP_kabi_prefix: + kabi_prefix = arg; break; case ARGP_numeric_version: print_numeric_version = true; break; case ARGP_btf_gen_floats: diff --git a/pahole_strings.h b/pahole_strings.h index 522fbf2..a836ba8 100644 --- a/pahole_strings.h +++ b/pahole_strings.h @@ -14,6 +14,8 @@ struct strings { struct btf *btf; }; +extern const char *kabi_prefix; + struct strings *strings__new(void); void strings__delete(struct strings *strings); diff --git a/strings.c b/strings.c index d37f49d..d1a54ec 100644 --- a/strings.c +++ b/strings.c @@ -17,6 +17,8 @@ #include "dutil.h" #include "lib/bpf/src/libbpf.h" +const char *kabi_prefix; + struct strings *strings__new(void) { struct strings *strs = malloc(sizeof(*strs)); @@ -48,7 +50,12 @@ strings_t strings__add(struct strings *strs, const char *str) if (str == NULL) return 0; - index = btf__add_str(strs->btf, str); + if (kabi_prefix && + strncmp(str, kabi_prefix, strlen(kabi_prefix)) == 0) + index = btf__add_str(strs->btf, kabi_prefix); + else + index = btf__add_str(strs->btf, str); + if (index < 0) return 0; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2] pahole: Add --kabi_prefix flag 2021-05-19 2:44 ` [PATCH v2] pahole: Add --kabi_prefix flag Shuyi Cheng @ 2021-05-19 20:07 ` Jiri Olsa [not found] ` <5D76A4F3-6F5A-4061-A274-34FFE5CBA338@gmail.com> 2021-05-20 10:27 ` Shuyi Cheng 2021-05-20 15:25 ` Jiri Olsa 1 sibling, 2 replies; 17+ messages in thread From: Jiri Olsa @ 2021-05-19 20:07 UTC (permalink / raw) To: Shuyi Cheng Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, dwarves, wenan.mao, Jiri Olsa On Wed, May 19, 2021 at 10:44:44AM +0800, Shuyi Cheng wrote: > To solve problems similar to _RH_KABI_REPLACE. The _RH_KABI_REPLACE(_orig, > _new) macros perserve size alignment and kabi agreement between _orig and > _new.Below is the definition of this macro: > > # define _RH_KABI_REPLACE(_orig, _new) \ > union { \ > _new; \ > struct { \ > _orig; \ > } __UNIQUE_ID(rh_kabi_hide); \ > __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ > } hi, that macro sounds familiar ;-) I think this should be already solved directly in the header file by this one: https://gitlab.com/cki-project/kernel-ark/-/commit/331be9c5a436057ee852075c102d9d90a9046a30 jirka > > > __UNIQUE_ID uses the __COUNTER__ macro, and the __COUNTER__ macro is > automatically incremented by 1 every time it is precompiled. Therefore, in > different compilation units, the same structure has different names.Here is > a concrete example: > > struct acpi_dev_node { > union { > struct acpi_device *companion; > struct { > void *handle; > } __UNIQUE_ID_rh_kabi_hide29; > union { }; > }; > }; > struct acpi_dev_node { > union { > struct acpi_device *companion; > struct { > void *handle; > } __UNIQUE_ID_rh_kabi_hide31; > union { }; > }; > }; > > Finally, it will cause the btf algorithm to de-duplication efficiency is not > high, and time-consuming. For example, running ./pahole -J > vmlinux-3.10.0-1160.el7.x86_64 without --kabi_prefix flag, the running time > is: > real 8m28.912s > user 8m27.271s > sys 0m1.471s > And the size of the generated btf segment is 30678240 bytes. > > After adding the patch, running ./pahole > --kabi_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. > The running time of the command is: > real 0m19.634s > user 0m18.457s > sys 0m1.169s > And the size of the generated btf segment is 3117719 bytes. > > --- > > v1:https://lore.kernel.org/dwarves/CAEf4Bzazh4RNCY3rGRRXfO2wJ7DSiMx8w+61B_hjhu9FrOffpQ@mail.gmail.com > v1->v2: > --Change btf_prefix to --kabi_prefix. > --Add man page. > --Add space after if and comma. > > man-pages/pahole.1 | 4 ++++ > pahole.c | 10 ++++++++++ > pahole_strings.h | 2 ++ > strings.c | 9 ++++++++- > 4 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 > index a10738f..2659fe6 100644 > --- a/man-pages/pahole.1 > +++ b/man-pages/pahole.1 > @@ -340,6 +340,10 @@ Show a traditional string version, i.e.: "v1.18". > Show a numeric only version, suitable for use in Makefiles and scripts > where > one wants to know what if the installed version has some feature, i.e.: 118 > instead of "v1.18". > > +.TP > +.B \-\-kabi_prefix=STRING > +When the prefix of the string is STRING, treat the string as STRING. > + > .SH NOTES > > To enable the generation of debugging information in the Linux kernel build > diff --git a/pahole.c b/pahole.c > index dc40ccf..6a700d9 100644 > --- a/pahole.c > +++ b/pahole.c > @@ -24,6 +24,7 @@ > #include "btf_encoder.h" > #include "libbtf.h" > #include "lib/bpf/src/libbpf.h" > +#include "pahole_strings.h" > > static bool btf_encode; > static bool ctf_encode; > @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; > #define ARGP_btf_gen_floats 322 > #define ARGP_btf_gen_all 323 > #define ARGP_with_flexible_array 324 > +#define ARGP_kabi_prefix 325 > > static const struct argp_option pahole__options[] = { > { > @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { > .doc = "Path to the base BTF file", > }, > { > + .name = "kabi_prefix", > + .key = ARGP_kabi_prefix, > + .arg = "STRING", > + .doc = "When the prefix of the string is STRING, treat the string as > STRING.", > + }, > + { > .name = "btf_encode", > .key = 'J', > .doc = "Encode as BTF", > @@ -1297,6 +1305,8 @@ static error_t pahole__options_parser(int key, char > *arg, > btf_encode_force = true; break; > case ARGP_btf_base: > base_btf_file = arg; break; > + case ARGP_kabi_prefix: > + kabi_prefix = arg; break; > case ARGP_numeric_version: > print_numeric_version = true; break; > case ARGP_btf_gen_floats: > diff --git a/pahole_strings.h b/pahole_strings.h > index 522fbf2..a836ba8 100644 > --- a/pahole_strings.h > +++ b/pahole_strings.h > @@ -14,6 +14,8 @@ struct strings { > struct btf *btf; > }; > > +extern const char *kabi_prefix; > + > struct strings *strings__new(void); > > void strings__delete(struct strings *strings); > diff --git a/strings.c b/strings.c > index d37f49d..d1a54ec 100644 > --- a/strings.c > +++ b/strings.c > @@ -17,6 +17,8 @@ > #include "dutil.h" > #include "lib/bpf/src/libbpf.h" > > +const char *kabi_prefix; > + > struct strings *strings__new(void) > { > struct strings *strs = malloc(sizeof(*strs)); > @@ -48,7 +50,12 @@ strings_t strings__add(struct strings *strs, const char > *str) > if (str == NULL) > return 0; > > - index = btf__add_str(strs->btf, str); > + if (kabi_prefix && > + strncmp(str, kabi_prefix, strlen(kabi_prefix)) == 0) > + index = btf__add_str(strs->btf, kabi_prefix); > + else > + index = btf__add_str(strs->btf, str); > + > if (index < 0) > return 0; > > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <5D76A4F3-6F5A-4061-A274-34FFE5CBA338@gmail.com>]
* Re: [PATCH v2] pahole: Add --kabi_prefix flag [not found] ` <5D76A4F3-6F5A-4061-A274-34FFE5CBA338@gmail.com> @ 2021-05-19 21:18 ` Jiri Olsa 2021-05-19 21:36 ` Arnaldo 0 siblings, 1 reply; 17+ messages in thread From: Jiri Olsa @ 2021-05-19 21:18 UTC (permalink / raw) To: Arnaldo; +Cc: Shuyi Cheng, Andrii Nakryiko, dwarves, wenan.mao, Jiri Olsa On Wed, May 19, 2021 at 05:23:31PM -0300, Arnaldo wrote: > > > On May 19, 2021 5:07:03 PM GMT-03:00, Jiri Olsa <jolsa@redhat.com> wrote: > >On Wed, May 19, 2021 at 10:44:44AM +0800, Shuyi Cheng wrote: > >> To solve problems similar to _RH_KABI_REPLACE. The > >_RH_KABI_REPLACE(_orig, > >> _new) macros perserve size alignment and kabi agreement between _orig > >and > >> _new.Below is the definition of this macro: > >> > >> # define _RH_KABI_REPLACE(_orig, _new) \ > >> union { \ > >> _new; \ > >> struct { \ > >> _orig; \ > >> } __UNIQUE_ID(rh_kabi_hide); \ > >> __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ > >> } > > > >hi, > >that macro sounds familiar ;-) I think this should be already > >solved directly in the header file by this one: > > > >https://gitlab.com/cki-project/kernel-ark/-/commit/331be9c5a436057ee852075c102d9d90a9046a30 > > > > But this means newer kernels will not have that issue, so perhaps we should just document this? I don't think there's released kernel with this issue > > Or would it be worth the effort of having a --ignore_prefix option for the people using all the kernels where this is applicable to and leave the option lingering there just in case? this is specific to rh kernels and fixed by that patch above, I wouldn't expect seeing it again jirka ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] pahole: Add --kabi_prefix flag 2021-05-19 21:18 ` Jiri Olsa @ 2021-05-19 21:36 ` Arnaldo 0 siblings, 0 replies; 17+ messages in thread From: Arnaldo @ 2021-05-19 21:36 UTC (permalink / raw) To: Jiri Olsa; +Cc: Shuyi Cheng, Andrii Nakryiko, dwarves, wenan.mao, Jiri Olsa On May 19, 2021 6:18:14 PM GMT-03:00, Jiri Olsa <jolsa@redhat.com> wrote: >On Wed, May 19, 2021 at 05:23:31PM -0300, Arnaldo wrote: >> >> >> On May 19, 2021 5:07:03 PM GMT-03:00, Jiri Olsa <jolsa@redhat.com> >wrote: >> >On Wed, May 19, 2021 at 10:44:44AM +0800, Shuyi Cheng wrote: >> >> To solve problems similar to _RH_KABI_REPLACE. The >> >_RH_KABI_REPLACE(_orig, >> >> _new) macros perserve size alignment and kabi agreement between >_orig >> >and >> >> _new.Below is the definition of this macro: >> >> >> >> # define _RH_KABI_REPLACE(_orig, _new) \ >> >> union { \ >> >> _new; \ >> >> struct { \ >> >> _orig; \ >> >> } __UNIQUE_ID(rh_kabi_hide); \ >> >> __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ >> >> } >> > >> >hi, >> >that macro sounds familiar ;-) I think this should be already >> >solved directly in the header file by this one: >> > >> >>https://gitlab.com/cki-project/kernel-ark/-/commit/331be9c5a436057ee852075c102d9d90a9046a30 >> > >> >> But this means newer kernels will not have that issue, so perhaps we >should just document this? > >I don't think there's released kernel with this issue > >> >> Or would it be worth the effort of having a --ignore_prefix option >for the people using all the kernels where this is applicable to and >leave the option lingering there just in case? > >this is specific to rh kernels and fixed by that patch above, >I wouldn't expect seeing it again Cool, case closed then :-) > >jirka -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] pahole: Add --kabi_prefix flag 2021-05-19 20:07 ` Jiri Olsa [not found] ` <5D76A4F3-6F5A-4061-A274-34FFE5CBA338@gmail.com> @ 2021-05-20 10:27 ` Shuyi Cheng 2021-05-20 11:49 ` Jiri Olsa 1 sibling, 1 reply; 17+ messages in thread From: Shuyi Cheng @ 2021-05-20 10:27 UTC (permalink / raw) To: Jiri Olsa Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, dwarves, wenan.mao, Jiri Olsa On 5/20/21 4:07 AM, Jiri Olsa wrote: > On Wed, May 19, 2021 at 10:44:44AM +0800, Shuyi Cheng wrote: >> To solve problems similar to _RH_KABI_REPLACE. The _RH_KABI_REPLACE(_orig, >> _new) macros perserve size alignment and kabi agreement between _orig and >> _new.Below is the definition of this macro: >> >> # define _RH_KABI_REPLACE(_orig, _new) \ >> union { \ >> _new; \ >> struct { \ >> _orig; \ >> } __UNIQUE_ID(rh_kabi_hide); \ >> __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ >> } > > hi, > that macro sounds familiar ;-) I think this should be already > solved directly in the header file by this one: > > https://gitlab.com/cki-project/kernel-ark/-/commit/331be9c5a436057ee852075c102d9d90a9046a30 > > jirka > Well, this patch solves this problem very well from the kernel level. But there are many mirrors [here](http://debuginfo.centos.org/7/x86_64/) that still have this problem. And these mirrors support ebpf, so it is very important to effectively extract the btf segment from these mirrors. :-) >> >> >> __UNIQUE_ID uses the __COUNTER__ macro, and the __COUNTER__ macro is >> automatically incremented by 1 every time it is precompiled. Therefore, in >> different compilation units, the same structure has different names.Here is >> a concrete example: >> >> struct acpi_dev_node { >> union { >> struct acpi_device *companion; >> struct { >> void *handle; >> } __UNIQUE_ID_rh_kabi_hide29; >> union { }; >> }; >> }; >> struct acpi_dev_node { >> union { >> struct acpi_device *companion; >> struct { >> void *handle; >> } __UNIQUE_ID_rh_kabi_hide31; >> union { }; >> }; >> }; >> >> Finally, it will cause the btf algorithm to de-duplication efficiency is not >> high, and time-consuming. For example, running ./pahole -J >> vmlinux-3.10.0-1160.el7.x86_64 without --kabi_prefix flag, the running time >> is: >> real 8m28.912s >> user 8m27.271s >> sys 0m1.471s >> And the size of the generated btf segment is 30678240 bytes. >> >> After adding the patch, running ./pahole >> --kabi_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. >> The running time of the command is: >> real 0m19.634s >> user 0m18.457s >> sys 0m1.169s >> And the size of the generated btf segment is 3117719 bytes. >> >> --- >> >> v1:https://lore.kernel.org/dwarves/CAEf4Bzazh4RNCY3rGRRXfO2wJ7DSiMx8w+61B_hjhu9FrOffpQ@mail.gmail.com >> v1->v2: >> --Change btf_prefix to --kabi_prefix. >> --Add man page. >> --Add space after if and comma. >> >> man-pages/pahole.1 | 4 ++++ >> pahole.c | 10 ++++++++++ >> pahole_strings.h | 2 ++ >> strings.c | 9 ++++++++- >> 4 files changed, 24 insertions(+), 1 deletion(-) >> >> diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 >> index a10738f..2659fe6 100644 >> --- a/man-pages/pahole.1 >> +++ b/man-pages/pahole.1 >> @@ -340,6 +340,10 @@ Show a traditional string version, i.e.: "v1.18". >> Show a numeric only version, suitable for use in Makefiles and scripts >> where >> one wants to know what if the installed version has some feature, i.e.: 118 >> instead of "v1.18". >> >> +.TP >> +.B \-\-kabi_prefix=STRING >> +When the prefix of the string is STRING, treat the string as STRING. >> + >> .SH NOTES >> >> To enable the generation of debugging information in the Linux kernel build >> diff --git a/pahole.c b/pahole.c >> index dc40ccf..6a700d9 100644 >> --- a/pahole.c >> +++ b/pahole.c >> @@ -24,6 +24,7 @@ >> #include "btf_encoder.h" >> #include "libbtf.h" >> #include "lib/bpf/src/libbpf.h" >> +#include "pahole_strings.h" >> >> static bool btf_encode; >> static bool ctf_encode; >> @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; >> #define ARGP_btf_gen_floats 322 >> #define ARGP_btf_gen_all 323 >> #define ARGP_with_flexible_array 324 >> +#define ARGP_kabi_prefix 325 >> >> static const struct argp_option pahole__options[] = { >> { >> @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { >> .doc = "Path to the base BTF file", >> }, >> { >> + .name = "kabi_prefix", >> + .key = ARGP_kabi_prefix, >> + .arg = "STRING", >> + .doc = "When the prefix of the string is STRING, treat the string as >> STRING.", >> + }, >> + { >> .name = "btf_encode", >> .key = 'J', >> .doc = "Encode as BTF", >> @@ -1297,6 +1305,8 @@ static error_t pahole__options_parser(int key, char >> *arg, >> btf_encode_force = true; break; >> case ARGP_btf_base: >> base_btf_file = arg; break; >> + case ARGP_kabi_prefix: >> + kabi_prefix = arg; break; >> case ARGP_numeric_version: >> print_numeric_version = true; break; >> case ARGP_btf_gen_floats: >> diff --git a/pahole_strings.h b/pahole_strings.h >> index 522fbf2..a836ba8 100644 >> --- a/pahole_strings.h >> +++ b/pahole_strings.h >> @@ -14,6 +14,8 @@ struct strings { >> struct btf *btf; >> }; >> >> +extern const char *kabi_prefix; >> + >> struct strings *strings__new(void); >> >> void strings__delete(struct strings *strings); >> diff --git a/strings.c b/strings.c >> index d37f49d..d1a54ec 100644 >> --- a/strings.c >> +++ b/strings.c >> @@ -17,6 +17,8 @@ >> #include "dutil.h" >> #include "lib/bpf/src/libbpf.h" >> >> +const char *kabi_prefix; >> + >> struct strings *strings__new(void) >> { >> struct strings *strs = malloc(sizeof(*strs)); >> @@ -48,7 +50,12 @@ strings_t strings__add(struct strings *strs, const char >> *str) >> if (str == NULL) >> return 0; >> >> - index = btf__add_str(strs->btf, str); >> + if (kabi_prefix && >> + strncmp(str, kabi_prefix, strlen(kabi_prefix)) == 0) >> + index = btf__add_str(strs->btf, kabi_prefix); >> + else >> + index = btf__add_str(strs->btf, str); >> + >> if (index < 0) >> return 0; >> >> -- >> 1.8.3.1 >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] pahole: Add --kabi_prefix flag 2021-05-20 10:27 ` Shuyi Cheng @ 2021-05-20 11:49 ` Jiri Olsa 2021-05-20 12:08 ` Shuyi Cheng 0 siblings, 1 reply; 17+ messages in thread From: Jiri Olsa @ 2021-05-20 11:49 UTC (permalink / raw) To: Shuyi Cheng Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, dwarves, wenan.mao, Jiri Olsa On Thu, May 20, 2021 at 06:27:48PM +0800, Shuyi Cheng wrote: > > > On 5/20/21 4:07 AM, Jiri Olsa wrote: > > On Wed, May 19, 2021 at 10:44:44AM +0800, Shuyi Cheng wrote: > > > To solve problems similar to _RH_KABI_REPLACE. The _RH_KABI_REPLACE(_orig, > > > _new) macros perserve size alignment and kabi agreement between _orig and > > > _new.Below is the definition of this macro: > > > > > > # define _RH_KABI_REPLACE(_orig, _new) \ > > > union { \ > > > _new; \ > > > struct { \ > > > _orig; \ > > > } __UNIQUE_ID(rh_kabi_hide); \ > > > __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ > > > } > > > > hi, > > that macro sounds familiar ;-) I think this should be already > > solved directly in the header file by this one: > > > > https://gitlab.com/cki-project/kernel-ark/-/commit/331be9c5a436057ee852075c102d9d90a9046a30 > > > > jirka > > > > Well, this patch solves this problem very well from the kernel level. But > there are many mirrors [here](http://debuginfo.centos.org/7/x86_64/) that > still have this problem. And these mirrors support ebpf, so it is very > important to effectively extract the btf segment from these mirrors. :-) hum, the link shows 3.10.* centos kernel, right? AFAIK there's no BTF support in those kernels.. but maybe I'm missing some backporting channel, could you please point me to related sources? jirka > > > > > > > > > > __UNIQUE_ID uses the __COUNTER__ macro, and the __COUNTER__ macro is > > > automatically incremented by 1 every time it is precompiled. Therefore, in > > > different compilation units, the same structure has different names.Here is > > > a concrete example: > > > > > > struct acpi_dev_node { > > > union { > > > struct acpi_device *companion; > > > struct { > > > void *handle; > > > } __UNIQUE_ID_rh_kabi_hide29; > > > union { }; > > > }; > > > }; > > > struct acpi_dev_node { > > > union { > > > struct acpi_device *companion; > > > struct { > > > void *handle; > > > } __UNIQUE_ID_rh_kabi_hide31; > > > union { }; > > > }; > > > }; > > > > > > Finally, it will cause the btf algorithm to de-duplication efficiency is not > > > high, and time-consuming. For example, running ./pahole -J > > > vmlinux-3.10.0-1160.el7.x86_64 without --kabi_prefix flag, the running time > > > is: > > > real 8m28.912s > > > user 8m27.271s > > > sys 0m1.471s > > > And the size of the generated btf segment is 30678240 bytes. > > > > > > After adding the patch, running ./pahole > > > --kabi_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. > > > The running time of the command is: > > > real 0m19.634s > > > user 0m18.457s > > > sys 0m1.169s > > > And the size of the generated btf segment is 3117719 bytes. > > > > > > --- > > > > > > v1:https://lore.kernel.org/dwarves/CAEf4Bzazh4RNCY3rGRRXfO2wJ7DSiMx8w+61B_hjhu9FrOffpQ@mail.gmail.com > > > v1->v2: > > > --Change btf_prefix to --kabi_prefix. > > > --Add man page. > > > --Add space after if and comma. > > > > > > man-pages/pahole.1 | 4 ++++ > > > pahole.c | 10 ++++++++++ > > > pahole_strings.h | 2 ++ > > > strings.c | 9 ++++++++- > > > 4 files changed, 24 insertions(+), 1 deletion(-) > > > > > > diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 > > > index a10738f..2659fe6 100644 > > > --- a/man-pages/pahole.1 > > > +++ b/man-pages/pahole.1 > > > @@ -340,6 +340,10 @@ Show a traditional string version, i.e.: "v1.18". > > > Show a numeric only version, suitable for use in Makefiles and scripts > > > where > > > one wants to know what if the installed version has some feature, i.e.: 118 > > > instead of "v1.18". > > > > > > +.TP > > > +.B \-\-kabi_prefix=STRING > > > +When the prefix of the string is STRING, treat the string as STRING. > > > + > > > .SH NOTES > > > > > > To enable the generation of debugging information in the Linux kernel build > > > diff --git a/pahole.c b/pahole.c > > > index dc40ccf..6a700d9 100644 > > > --- a/pahole.c > > > +++ b/pahole.c > > > @@ -24,6 +24,7 @@ > > > #include "btf_encoder.h" > > > #include "libbtf.h" > > > #include "lib/bpf/src/libbpf.h" > > > +#include "pahole_strings.h" > > > > > > static bool btf_encode; > > > static bool ctf_encode; > > > @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; > > > #define ARGP_btf_gen_floats 322 > > > #define ARGP_btf_gen_all 323 > > > #define ARGP_with_flexible_array 324 > > > +#define ARGP_kabi_prefix 325 > > > > > > static const struct argp_option pahole__options[] = { > > > { > > > @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { > > > .doc = "Path to the base BTF file", > > > }, > > > { > > > + .name = "kabi_prefix", > > > + .key = ARGP_kabi_prefix, > > > + .arg = "STRING", > > > + .doc = "When the prefix of the string is STRING, treat the string as > > > STRING.", > > > + }, > > > + { > > > .name = "btf_encode", > > > .key = 'J', > > > .doc = "Encode as BTF", > > > @@ -1297,6 +1305,8 @@ static error_t pahole__options_parser(int key, char > > > *arg, > > > btf_encode_force = true; break; > > > case ARGP_btf_base: > > > base_btf_file = arg; break; > > > + case ARGP_kabi_prefix: > > > + kabi_prefix = arg; break; > > > case ARGP_numeric_version: > > > print_numeric_version = true; break; > > > case ARGP_btf_gen_floats: > > > diff --git a/pahole_strings.h b/pahole_strings.h > > > index 522fbf2..a836ba8 100644 > > > --- a/pahole_strings.h > > > +++ b/pahole_strings.h > > > @@ -14,6 +14,8 @@ struct strings { > > > struct btf *btf; > > > }; > > > > > > +extern const char *kabi_prefix; > > > + > > > struct strings *strings__new(void); > > > > > > void strings__delete(struct strings *strings); > > > diff --git a/strings.c b/strings.c > > > index d37f49d..d1a54ec 100644 > > > --- a/strings.c > > > +++ b/strings.c > > > @@ -17,6 +17,8 @@ > > > #include "dutil.h" > > > #include "lib/bpf/src/libbpf.h" > > > > > > +const char *kabi_prefix; > > > + > > > struct strings *strings__new(void) > > > { > > > struct strings *strs = malloc(sizeof(*strs)); > > > @@ -48,7 +50,12 @@ strings_t strings__add(struct strings *strs, const char > > > *str) > > > if (str == NULL) > > > return 0; > > > > > > - index = btf__add_str(strs->btf, str); > > > + if (kabi_prefix && > > > + strncmp(str, kabi_prefix, strlen(kabi_prefix)) == 0) > > > + index = btf__add_str(strs->btf, kabi_prefix); > > > + else > > > + index = btf__add_str(strs->btf, str); > > > + > > > if (index < 0) > > > return 0; > > > > > > -- > > > 1.8.3.1 > > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] pahole: Add --kabi_prefix flag 2021-05-20 11:49 ` Jiri Olsa @ 2021-05-20 12:08 ` Shuyi Cheng 2021-05-20 12:18 ` Jiri Olsa 0 siblings, 1 reply; 17+ messages in thread From: Shuyi Cheng @ 2021-05-20 12:08 UTC (permalink / raw) To: Jiri Olsa Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, dwarves, wenan.mao, Jiri Olsa On 5/20/21 7:49 PM, Jiri Olsa wrote: > On Thu, May 20, 2021 at 06:27:48PM +0800, Shuyi Cheng wrote: >> >> >> On 5/20/21 4:07 AM, Jiri Olsa wrote: >>> On Wed, May 19, 2021 at 10:44:44AM +0800, Shuyi Cheng wrote: >>>> To solve problems similar to _RH_KABI_REPLACE. The _RH_KABI_REPLACE(_orig, >>>> _new) macros perserve size alignment and kabi agreement between _orig and >>>> _new.Below is the definition of this macro: >>>> >>>> # define _RH_KABI_REPLACE(_orig, _new) \ >>>> union { \ >>>> _new; \ >>>> struct { \ >>>> _orig; \ >>>> } __UNIQUE_ID(rh_kabi_hide); \ >>>> __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ >>>> } >>> >>> hi, >>> that macro sounds familiar ;-) I think this should be already >>> solved directly in the header file by this one: >>> >>> https://gitlab.com/cki-project/kernel-ark/-/commit/331be9c5a436057ee852075c102d9d90a9046a30 >>> >>> jirka >>> >> >> Well, this patch solves this problem very well from the kernel level. But >> there are many mirrors [here](http://debuginfo.centos.org/7/x86_64/) that >> still have this problem. And these mirrors support ebpf, so it is very >> important to effectively extract the btf segment from these mirrors. :-) > > hum, the link shows 3.10.* centos kernel, right? > AFAIK there's no BTF support in those kernels.. > > but maybe I'm missing some backporting channel, > could you please point me to related sources? > > jirka > Yes, it is the centos kernel of version 3.10. This version of the centos kernel supports ebpf. As far as I think, BTF features don't depend on the kernel much. Only when using the STRUCT OPS feature does the kernel support BTF, see [here](https://github.com/libbpf/libbpf/blob/57375504c6c9766d23f178c40f71bf5e8df9363d/src/libbpf.c#L2549) and [here](https://www.spinics.net/lists/netdev/msg637060.html). Therefore, in the 3.10 version of the Centos kernel, with the help of the vmlinux btf segment, libbpf CO-RE can be easily supported. >> >>>> >>>> >>>> __UNIQUE_ID uses the __COUNTER__ macro, and the __COUNTER__ macro is >>>> automatically incremented by 1 every time it is precompiled. Therefore, in >>>> different compilation units, the same structure has different names.Here is >>>> a concrete example: >>>> >>>> struct acpi_dev_node { >>>> union { >>>> struct acpi_device *companion; >>>> struct { >>>> void *handle; >>>> } __UNIQUE_ID_rh_kabi_hide29; >>>> union { }; >>>> }; >>>> }; >>>> struct acpi_dev_node { >>>> union { >>>> struct acpi_device *companion; >>>> struct { >>>> void *handle; >>>> } __UNIQUE_ID_rh_kabi_hide31; >>>> union { }; >>>> }; >>>> }; >>>> >>>> Finally, it will cause the btf algorithm to de-duplication efficiency is not >>>> high, and time-consuming. For example, running ./pahole -J >>>> vmlinux-3.10.0-1160.el7.x86_64 without --kabi_prefix flag, the running time >>>> is: >>>> real 8m28.912s >>>> user 8m27.271s >>>> sys 0m1.471s >>>> And the size of the generated btf segment is 30678240 bytes. >>>> >>>> After adding the patch, running ./pahole >>>> --kabi_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. >>>> The running time of the command is: >>>> real 0m19.634s >>>> user 0m18.457s >>>> sys 0m1.169s >>>> And the size of the generated btf segment is 3117719 bytes. >>>> >>>> --- >>>> >>>> v1:https://lore.kernel.org/dwarves/CAEf4Bzazh4RNCY3rGRRXfO2wJ7DSiMx8w+61B_hjhu9FrOffpQ@mail.gmail.com >>>> v1->v2: >>>> --Change btf_prefix to --kabi_prefix. >>>> --Add man page. >>>> --Add space after if and comma. >>>> >>>> man-pages/pahole.1 | 4 ++++ >>>> pahole.c | 10 ++++++++++ >>>> pahole_strings.h | 2 ++ >>>> strings.c | 9 ++++++++- >>>> 4 files changed, 24 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 >>>> index a10738f..2659fe6 100644 >>>> --- a/man-pages/pahole.1 >>>> +++ b/man-pages/pahole.1 >>>> @@ -340,6 +340,10 @@ Show a traditional string version, i.e.: "v1.18". >>>> Show a numeric only version, suitable for use in Makefiles and scripts >>>> where >>>> one wants to know what if the installed version has some feature, i.e.: 118 >>>> instead of "v1.18". >>>> >>>> +.TP >>>> +.B \-\-kabi_prefix=STRING >>>> +When the prefix of the string is STRING, treat the string as STRING. >>>> + >>>> .SH NOTES >>>> >>>> To enable the generation of debugging information in the Linux kernel build >>>> diff --git a/pahole.c b/pahole.c >>>> index dc40ccf..6a700d9 100644 >>>> --- a/pahole.c >>>> +++ b/pahole.c >>>> @@ -24,6 +24,7 @@ >>>> #include "btf_encoder.h" >>>> #include "libbtf.h" >>>> #include "lib/bpf/src/libbpf.h" >>>> +#include "pahole_strings.h" >>>> >>>> static bool btf_encode; >>>> static bool ctf_encode; >>>> @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; >>>> #define ARGP_btf_gen_floats 322 >>>> #define ARGP_btf_gen_all 323 >>>> #define ARGP_with_flexible_array 324 >>>> +#define ARGP_kabi_prefix 325 >>>> >>>> static const struct argp_option pahole__options[] = { >>>> { >>>> @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { >>>> .doc = "Path to the base BTF file", >>>> }, >>>> { >>>> + .name = "kabi_prefix", >>>> + .key = ARGP_kabi_prefix, >>>> + .arg = "STRING", >>>> + .doc = "When the prefix of the string is STRING, treat the string as >>>> STRING.", >>>> + }, >>>> + { >>>> .name = "btf_encode", >>>> .key = 'J', >>>> .doc = "Encode as BTF", >>>> @@ -1297,6 +1305,8 @@ static error_t pahole__options_parser(int key, char >>>> *arg, >>>> btf_encode_force = true; break; >>>> case ARGP_btf_base: >>>> base_btf_file = arg; break; >>>> + case ARGP_kabi_prefix: >>>> + kabi_prefix = arg; break; >>>> case ARGP_numeric_version: >>>> print_numeric_version = true; break; >>>> case ARGP_btf_gen_floats: >>>> diff --git a/pahole_strings.h b/pahole_strings.h >>>> index 522fbf2..a836ba8 100644 >>>> --- a/pahole_strings.h >>>> +++ b/pahole_strings.h >>>> @@ -14,6 +14,8 @@ struct strings { >>>> struct btf *btf; >>>> }; >>>> >>>> +extern const char *kabi_prefix; >>>> + >>>> struct strings *strings__new(void); >>>> >>>> void strings__delete(struct strings *strings); >>>> diff --git a/strings.c b/strings.c >>>> index d37f49d..d1a54ec 100644 >>>> --- a/strings.c >>>> +++ b/strings.c >>>> @@ -17,6 +17,8 @@ >>>> #include "dutil.h" >>>> #include "lib/bpf/src/libbpf.h" >>>> >>>> +const char *kabi_prefix; >>>> + >>>> struct strings *strings__new(void) >>>> { >>>> struct strings *strs = malloc(sizeof(*strs)); >>>> @@ -48,7 +50,12 @@ strings_t strings__add(struct strings *strs, const char >>>> *str) >>>> if (str == NULL) >>>> return 0; >>>> >>>> - index = btf__add_str(strs->btf, str); >>>> + if (kabi_prefix && >>>> + strncmp(str, kabi_prefix, strlen(kabi_prefix)) == 0) >>>> + index = btf__add_str(strs->btf, kabi_prefix); >>>> + else >>>> + index = btf__add_str(strs->btf, str); >>>> + >>>> if (index < 0) >>>> return 0; >>>> >>>> -- >>>> 1.8.3.1 >>>> >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] pahole: Add --kabi_prefix flag 2021-05-20 12:08 ` Shuyi Cheng @ 2021-05-20 12:18 ` Jiri Olsa 2021-05-20 12:30 ` Shuyi Cheng 0 siblings, 1 reply; 17+ messages in thread From: Jiri Olsa @ 2021-05-20 12:18 UTC (permalink / raw) To: Shuyi Cheng Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, dwarves, wenan.mao, Jiri Olsa On Thu, May 20, 2021 at 08:08:44PM +0800, Shuyi Cheng wrote: > > > On 5/20/21 7:49 PM, Jiri Olsa wrote: > > On Thu, May 20, 2021 at 06:27:48PM +0800, Shuyi Cheng wrote: > > > > > > > > > On 5/20/21 4:07 AM, Jiri Olsa wrote: > > > > On Wed, May 19, 2021 at 10:44:44AM +0800, Shuyi Cheng wrote: > > > > > To solve problems similar to _RH_KABI_REPLACE. The _RH_KABI_REPLACE(_orig, > > > > > _new) macros perserve size alignment and kabi agreement between _orig and > > > > > _new.Below is the definition of this macro: > > > > > > > > > > # define _RH_KABI_REPLACE(_orig, _new) \ > > > > > union { \ > > > > > _new; \ > > > > > struct { \ > > > > > _orig; \ > > > > > } __UNIQUE_ID(rh_kabi_hide); \ > > > > > __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ > > > > > } > > > > > > > > hi, > > > > that macro sounds familiar ;-) I think this should be already > > > > solved directly in the header file by this one: > > > > > > > > https://gitlab.com/cki-project/kernel-ark/-/commit/331be9c5a436057ee852075c102d9d90a9046a30 > > > > > > > > jirka > > > > > > > > > > Well, this patch solves this problem very well from the kernel level. But > > > there are many mirrors [here](http://debuginfo.centos.org/7/x86_64/) that > > > still have this problem. And these mirrors support ebpf, so it is very > > > important to effectively extract the btf segment from these mirrors. :-) > > > > hum, the link shows 3.10.* centos kernel, right? > > AFAIK there's no BTF support in those kernels.. > > > > but maybe I'm missing some backporting channel, > > could you please point me to related sources? > > > > jirka > > > > Yes, it is the centos kernel of version 3.10. This version of the centos > kernel supports ebpf. As far as I think, BTF features don't depend on the > kernel much. Only when using the STRUCT OPS feature does the kernel support > BTF, see [here](https://github.com/libbpf/libbpf/blob/57375504c6c9766d23f178c40f71bf5e8df9363d/src/libbpf.c#L2549) > and [here](https://www.spinics.net/lists/netdev/msg637060.html). Therefore, > in the 3.10 version of the Centos kernel, with the help of the vmlinux btf > segment, libbpf CO-RE can be easily supported. ok, so you're generating BTF for centos 3.10 kernel? I think that fix would be easy kernel backport, but I guess we can go with pahole option just as well, if backport is not an option for you jirka ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] pahole: Add --kabi_prefix flag 2021-05-20 12:18 ` Jiri Olsa @ 2021-05-20 12:30 ` Shuyi Cheng 0 siblings, 0 replies; 17+ messages in thread From: Shuyi Cheng @ 2021-05-20 12:30 UTC (permalink / raw) To: Jiri Olsa Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, dwarves, wenan.mao, Jiri Olsa On 5/20/21 8:18 PM, Jiri Olsa wrote: > On Thu, May 20, 2021 at 08:08:44PM +0800, Shuyi Cheng wrote: >> >> >> On 5/20/21 7:49 PM, Jiri Olsa wrote: >>> On Thu, May 20, 2021 at 06:27:48PM +0800, Shuyi Cheng wrote: >>>> >>>> >>>> On 5/20/21 4:07 AM, Jiri Olsa wrote: >>>>> On Wed, May 19, 2021 at 10:44:44AM +0800, Shuyi Cheng wrote: >>>>>> To solve problems similar to _RH_KABI_REPLACE. The _RH_KABI_REPLACE(_orig, >>>>>> _new) macros perserve size alignment and kabi agreement between _orig and >>>>>> _new.Below is the definition of this macro: >>>>>> >>>>>> # define _RH_KABI_REPLACE(_orig, _new) \ >>>>>> union { \ >>>>>> _new; \ >>>>>> struct { \ >>>>>> _orig; \ >>>>>> } __UNIQUE_ID(rh_kabi_hide); \ >>>>>> __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ >>>>>> } >>>>> >>>>> hi, >>>>> that macro sounds familiar ;-) I think this should be already >>>>> solved directly in the header file by this one: >>>>> >>>>> https://gitlab.com/cki-project/kernel-ark/-/commit/331be9c5a436057ee852075c102d9d90a9046a30 >>>>> >>>>> jirka >>>>> >>>> >>>> Well, this patch solves this problem very well from the kernel level. But >>>> there are many mirrors [here](http://debuginfo.centos.org/7/x86_64/) that >>>> still have this problem. And these mirrors support ebpf, so it is very >>>> important to effectively extract the btf segment from these mirrors. :-) >>> >>> hum, the link shows 3.10.* centos kernel, right? >>> AFAIK there's no BTF support in those kernels.. >>> >>> but maybe I'm missing some backporting channel, >>> could you please point me to related sources? >>> >>> jirka >>> >> >> Yes, it is the centos kernel of version 3.10. This version of the centos >> kernel supports ebpf. As far as I think, BTF features don't depend on the >> kernel much. Only when using the STRUCT OPS feature does the kernel support >> BTF, see [here](https://github.com/libbpf/libbpf/blob/57375504c6c9766d23f178c40f71bf5e8df9363d/src/libbpf.c#L2549) >> and [here](https://www.spinics.net/lists/netdev/msg637060.html). Therefore, >> in the 3.10 version of the Centos kernel, with the help of the vmlinux btf >> segment, libbpf CO-RE can be easily supported. > > ok, so you're generating BTF for centos 3.10 kernel? > > I think that fix would be easy kernel backport, but I guess > we can go with pahole option just as well, if backport is > not an option for you > > jirka > Yes, I am generating btf segment for centos 3.10 kernel. There are two main reasons for adding this patch: 1. There are too many kernel versions in centos 3.10, and a certain technical foundation is required to compile the kernel. 2. In my opinion, many people are using pahole to generate vmlinux btf segment, and there are still a large number of users on centos. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] pahole: Add --kabi_prefix flag 2021-05-19 2:44 ` [PATCH v2] pahole: Add --kabi_prefix flag Shuyi Cheng 2021-05-19 20:07 ` Jiri Olsa @ 2021-05-20 15:25 ` Jiri Olsa 2021-05-21 1:44 ` [PATCH v3] " Shuyi Cheng 1 sibling, 1 reply; 17+ messages in thread From: Jiri Olsa @ 2021-05-20 15:25 UTC (permalink / raw) To: Shuyi Cheng Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, dwarves, wenan.mao, Jiri Olsa On Wed, May 19, 2021 at 10:44:44AM +0800, Shuyi Cheng wrote: SNIP > where > one wants to know what if the installed version has some feature, i.e.: 118 > instead of "v1.18". > > +.TP > +.B \-\-kabi_prefix=STRING > +When the prefix of the string is STRING, treat the string as STRING. > + > .SH NOTES > > To enable the generation of debugging information in the Linux kernel build > diff --git a/pahole.c b/pahole.c > index dc40ccf..6a700d9 100644 > --- a/pahole.c > +++ b/pahole.c > @@ -24,6 +24,7 @@ > #include "btf_encoder.h" > #include "libbtf.h" > #include "lib/bpf/src/libbpf.h" > +#include "pahole_strings.h" > > static bool btf_encode; > static bool ctf_encode; > @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; > #define ARGP_btf_gen_floats 322 > #define ARGP_btf_gen_all 323 > #define ARGP_with_flexible_array 324 > +#define ARGP_kabi_prefix 325 > > static const struct argp_option pahole__options[] = { > { > @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { > .doc = "Path to the base BTF file", > }, > { > + .name = "kabi_prefix", > + .key = ARGP_kabi_prefix, > + .arg = "STRING", > + .doc = "When the prefix of the string is STRING, treat the string as > STRING.", please keep the '=' indentation > + }, > + { > .name = "btf_encode", > .key = 'J', > .doc = "Encode as BTF", > @@ -1297,6 +1305,8 @@ static error_t pahole__options_parser(int key, char > *arg, > btf_encode_force = true; break; > case ARGP_btf_base: > base_btf_file = arg; break; > + case ARGP_kabi_prefix: > + kabi_prefix = arg; break; > case ARGP_numeric_version: > print_numeric_version = true; break; > case ARGP_btf_gen_floats: > diff --git a/pahole_strings.h b/pahole_strings.h > index 522fbf2..a836ba8 100644 > --- a/pahole_strings.h > +++ b/pahole_strings.h > @@ -14,6 +14,8 @@ struct strings { > struct btf *btf; > }; > > +extern const char *kabi_prefix; > + > struct strings *strings__new(void); > > void strings__delete(struct strings *strings); > diff --git a/strings.c b/strings.c > index d37f49d..d1a54ec 100644 > --- a/strings.c > +++ b/strings.c > @@ -17,6 +17,8 @@ > #include "dutil.h" > #include "lib/bpf/src/libbpf.h" > > +const char *kabi_prefix; > + > struct strings *strings__new(void) > { > struct strings *strs = malloc(sizeof(*strs)); > @@ -48,7 +50,12 @@ strings_t strings__add(struct strings *strs, const char > *str) > if (str == NULL) > return 0; > > - index = btf__add_str(strs->btf, str); > + if (kabi_prefix && > + strncmp(str, kabi_prefix, strlen(kabi_prefix)) == 0) > + index = btf__add_str(strs->btf, kabi_prefix); > + else > + index = btf__add_str(strs->btf, str); > + ^^^ extra whitespace, other than that looks ok Acked-by: Jiri Olsa <jolsa@redhat.com> jirka > if (index < 0) > return 0; > > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3] pahole: Add --kabi_prefix flag 2021-05-20 15:25 ` Jiri Olsa @ 2021-05-21 1:44 ` Shuyi Cheng 2021-05-27 16:43 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 17+ messages in thread From: Shuyi Cheng @ 2021-05-21 1:44 UTC (permalink / raw) To: Jiri Olsa Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo, dwarves, wenan.mao, Jiri Olsa To solve problems similar to _RH_KABI_REPLACE. The _RH_KABI_REPLACE(_orig, _new) macros perserve size alignment and kabi agreement between _orig and _new.Below is the definition of this macro: # define _RH_KABI_REPLACE(_orig, _new) \ union { \ _new; \ struct { \ _orig; \ } __UNIQUE_ID(rh_kabi_hide); \ __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ } __UNIQUE_ID uses the __COUNTER__ macro, and the __COUNTER__ macro is automatically incremented by 1 every time it is precompiled. Therefore, in different compilation units, the same structure has different names.Here is a concrete example: struct acpi_dev_node { union { struct acpi_device *companion; struct { void *handle; } __UNIQUE_ID_rh_kabi_hide29; union { }; }; }; struct acpi_dev_node { union { struct acpi_device *companion; struct { void *handle; } __UNIQUE_ID_rh_kabi_hide31; union { }; }; }; Finally, it will cause the btf algorithm to de-duplication efficiency is not high, and time-consuming. For example, running ./pahole -J vmlinux-3.10.0-1160.el7.x86_64 without --kabi_prefix flag, the running time is: real 8m28.912s user 8m27.271s sys 0m1.471s And the size of the generated btf segment is 30678240 bytes. After adding the patch, running ./pahole --kabi_prefix=__UNIQUE_ID_rh_kabi_hide -J vmlinux-3.10.0-1160.el7.x86_64. The running time of the command is: real 0m19.634s user 0m18.457s sys 0m1.169s And the size of the generated btf segment is 3117719 bytes. Acked-by: Jiri Olsa <jolsa@redhat.com> Cc: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com> Cc: Wenan Mao <wenan.mao@linux.alibaba.com> Signed-off-by: Shuyi Cheng <chengshuyi@linux.alibaba.com> --- v1: https://lore.kernel.org/dwarves/a249699a-7c6c-4d4a-71b0-87981c8d229e@linux.alibaba.com/ v1->v2: -- Change btf_prefix to --kabi_prefix. -- Add man page. -- Add space after if and comma. v2: https://lore.kernel.org/dwarves/ec80d351-ac2f-5c48-c339-c2c503d36d68@linux.alibaba.com/ v2->v3: -- Keep the '=' indentation. -- Remove extra whitespace. man-pages/pahole.1 | 4 ++++ pahole.c | 10 ++++++++++ pahole_strings.h | 2 ++ strings.c | 9 ++++++++- 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 index a10738f..2659fe6 100644 --- a/man-pages/pahole.1 +++ b/man-pages/pahole.1 @@ -340,6 +340,10 @@ Show a traditional string version, i.e.: "v1.18". Show a numeric only version, suitable for use in Makefiles and scripts where one wants to know what if the installed version has some feature, i.e.: 118 instead of "v1.18". +.TP +.B \-\-kabi_prefix=STRING +When the prefix of the string is STRING, treat the string as STRING. + .SH NOTES To enable the generation of debugging information in the Linux kernel build diff --git a/pahole.c b/pahole.c index dc40ccf..f2406e4 100644 --- a/pahole.c +++ b/pahole.c @@ -24,6 +24,7 @@ #include "btf_encoder.h" #include "libbtf.h" #include "lib/bpf/src/libbpf.h" +#include "pahole_strings.h" static bool btf_encode; static bool ctf_encode; @@ -855,6 +856,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; #define ARGP_btf_gen_floats 322 #define ARGP_btf_gen_all 323 #define ARGP_with_flexible_array 324 +#define ARGP_kabi_prefix 325 static const struct argp_option pahole__options[] = { { @@ -1140,6 +1142,12 @@ static const struct argp_option pahole__options[] = { .doc = "Path to the base BTF file", }, { + .name = "kabi_prefix", + .key = ARGP_kabi_prefix, + .arg = "STRING", + .doc = "When the prefix of the string is STRING, treat the string as STRING.", + }, + { .name = "btf_encode", .key = 'J', .doc = "Encode as BTF", @@ -1297,6 +1305,8 @@ static error_t pahole__options_parser(int key, char *arg, btf_encode_force = true; break; case ARGP_btf_base: base_btf_file = arg; break; + case ARGP_kabi_prefix: + kabi_prefix = arg; break; case ARGP_numeric_version: print_numeric_version = true; break; case ARGP_btf_gen_floats: diff --git a/pahole_strings.h b/pahole_strings.h index 522fbf2..a836ba8 100644 --- a/pahole_strings.h +++ b/pahole_strings.h @@ -14,6 +14,8 @@ struct strings { struct btf *btf; }; +extern const char *kabi_prefix; + struct strings *strings__new(void); void strings__delete(struct strings *strings); diff --git a/strings.c b/strings.c index d37f49d..d5eb3a1 100644 --- a/strings.c +++ b/strings.c @@ -17,6 +17,8 @@ #include "dutil.h" #include "lib/bpf/src/libbpf.h" +const char *kabi_prefix; + struct strings *strings__new(void) { struct strings *strs = malloc(sizeof(*strs)); @@ -48,7 +50,12 @@ strings_t strings__add(struct strings *strs, const char *str) if (str == NULL) return 0; - index = btf__add_str(strs->btf, str); + if (kabi_prefix && + strncmp(str, kabi_prefix, strlen(kabi_prefix)) == 0) + index = btf__add_str(strs->btf, kabi_prefix); + else + index = btf__add_str(strs->btf, str); + if (index < 0) return 0; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3] pahole: Add --kabi_prefix flag 2021-05-21 1:44 ` [PATCH v3] " Shuyi Cheng @ 2021-05-27 16:43 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 17+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-05-27 16:43 UTC (permalink / raw) To: Shuyi Cheng Cc: Jiri Olsa, Andrii Nakryiko, Arnaldo Carvalho de Melo, dwarves, wenan.mao, Jiri Olsa Em Fri, May 21, 2021 at 09:44:20AM +0800, Shuyi Cheng escreveu: > To solve problems similar to _RH_KABI_REPLACE. The _RH_KABI_REPLACE(_orig, > _new) macros perserve size alignment and kabi agreement between _orig and > _new.Below is the definition of this macro: > > # define _RH_KABI_REPLACE(_orig, _new) \ > union { \ > _new; \ > struct { \ > _orig; \ > } __UNIQUE_ID(rh_kabi_hide); \ > __RH_KABI_CHECK_SIZE_ALIGN(_orig, _new); \ > } Thanks, applied. - Arnaldo ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-05-27 16:43 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-17 12:06 [PATCH] btf: Add --btf_prefix flag 程书意
2021-05-17 14:42 ` Arnaldo Carvalho de Melo
2021-05-18 9:02 ` chengshuyi
2021-05-18 12:49 ` Arnaldo Carvalho de Melo
2021-05-18 18:30 ` Andrii Nakryiko
2021-05-19 2:44 ` [PATCH v2] pahole: Add --kabi_prefix flag Shuyi Cheng
2021-05-19 20:07 ` Jiri Olsa
[not found] ` <5D76A4F3-6F5A-4061-A274-34FFE5CBA338@gmail.com>
2021-05-19 21:18 ` Jiri Olsa
2021-05-19 21:36 ` Arnaldo
2021-05-20 10:27 ` Shuyi Cheng
2021-05-20 11:49 ` Jiri Olsa
2021-05-20 12:08 ` Shuyi Cheng
2021-05-20 12:18 ` Jiri Olsa
2021-05-20 12:30 ` Shuyi Cheng
2021-05-20 15:25 ` Jiri Olsa
2021-05-21 1:44 ` [PATCH v3] " Shuyi Cheng
2021-05-27 16:43 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.