* [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
* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).