* Re: [PATCH v1] btf_encoder: Handle DW_TAG_variable that has DW_AT_specification [not found] ` <DEC4CC81-88CE-4476-A631-2BBB6E922F5C@gmail.com> @ 2020-09-30 6:52 ` Hao Luo 2020-10-01 15:47 ` Alexei Starovoitov 0 siblings, 1 reply; 8+ messages in thread From: Hao Luo @ 2020-09-30 6:52 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, dwarves, Yonghong Song Arnaldo, Is this patch ready to be merged into Pahole's master branch? Alexei is testing the kernel patches that need this patch. Please let me know if there is anything I can do to help merging. Thank you, Hao On Wed, Aug 26, 2020 at 6:56 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > > > On August 26, 2020 3:35:17 PM GMT-03:00, Hao Luo <haoluo@google.com> wrote: > >Arnaldo, > > > >On Wed, Aug 26, 2020 at 6:12 AM Arnaldo Carvalho de Melo > ><acme@kernel.org> wrote: > >> > >> Em Mon, Aug 24, 2020 at 05:45:23PM -0700, Hao Luo escreveu: > >> > It is found on gcc 8.2 that global percpu variables generate the > >> > following dwarf entry in the cu where the variable is defined[1]. > >> > > >> > Take the global variable "bpf_prog_active" defined in > >> > kernel/bpf/syscall.c as an example. The debug info for syscall.c > >> > has two dwarf entries for "bpf_prog_active". > >> > > >[...] > >> > >> Interesting, here I get, with binutils' readelf: > >> > >> [root@quaco perf]# readelf -wi > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > >bpf_prog_active > >> [root@quaco perf]# > >> > >> Just one, as: > >> > >> [root@quaco perf]# readelf -wi > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active -B1 -A8 > >> <1><f6a0>: Abbrev Number: 103 (DW_TAG_variable) > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > >bpf_prog_active > >> <f6a5> DW_AT_decl_file : 11 > >> <f6a6> DW_AT_decl_line : 1008 > >> <f6a8> DW_AT_decl_column : 1 > >> <f6a9> DW_AT_type : <0xcf> > >> <f6ad> DW_AT_external : 1 > >> <f6ad> DW_AT_declaration : 1 > >> <1><f6ad>: Abbrev Number: 103 (DW_TAG_variable) > >> <f6ae> DW_AT_name : (indirect string, offset: 0x3a5d): > >bpf_stats_enabled_mutex > >> [root@quaco perf]# > >> > >> I get what you have when I use elfutils' readelf: > >> > >> [root@quaco perf]# eu-readelf -winfo > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > >> name (strp) "bpf_prog_active" > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > >> [root@quaco perf]# > >> > >> [root@quaco perf]# eu-readelf -winfo > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B1 -A8 > >\"bpf_prog_active\" > >> [ f6a0] variable abbrev: 103 > >> name (strp) "bpf_prog_active" > >> decl_file (data1) bpf.h (11) > >> decl_line (data2) 1008 > >> decl_column (data1) 1 > >> type (ref4) [ cf] > >> external (flag_present) yes > >> declaration (flag_present) yes > >> [ f6ad] variable abbrev: 103 > >> name (strp) "bpf_stats_enabled_mutex" > >> [root@quaco perf]# > >> > >> And: > >> > >> [root@quaco perf]# eu-readelf -winfo > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B5 \<bpf_prog_active\> > >> [ 1bdf5] variable abbrev: 212 > >> specification (ref4) [ f6a0] > >> decl_file (data1) syscall.c (1) > >> decl_line (data1) 43 > >> location (exprloc) > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > >> [root@quaco perf]# > >> > > > >In binutils readelf, there is a extra entry > > Not here, tomorrow I'll triple check. > > > > > <1><1b24c>: Abbrev Number: 195 (DW_TAG_variable) > > <1b24e> DW_AT_specification: <0xf335> > > <1b252> DW_AT_decl_file : 1 > > <1b253> DW_AT_decl_line : 43 > > <1b254> DW_AT_location : 9 byte block: 3 0 0 0 0 0 0 0 0 > > (DW_OP_addr: 0) > > > >which points to > > > > <1><f335>: Abbrev Number: 95 (DW_TAG_variable) > > <f336> DW_AT_name : (indirect string, offset: 0xb37a): > >bpf_prog_active > > > >It just doesn't have the string 'bpf_prog_active', annotating entry. > >So eu-readelf and binutils readelf have the same results. > > > >> > Note that second DW_TAG_variable entry contains specification that > >> > points to the first entry. > >> > >> So you are not considering the first when encoding since it is just a > >> DW_AT_declaration, considers the second, as it should be, and then > >needs > >> to go see its DW_AT_specification, right? > >> > >> Sounds correct, applying, will test further and then push out, > >> > > > >Yes, exactly. The var tags to be considered are those that either have > >DW_AT_specification or not have DW_AT_declaration. This makes sure > >btf_encoder works correctly on both old and new gcc. > > > >> Thanks, > >> > >> - Arnaldo > > > >Suggested by Yonghong, I tested this change on a larger set of > >compilers this time and works correctly. See below. > > > >Could you also add 'Reported-by: Yonghong Song <yhs@fb.com>'? I should > >have done that when sending out this patch. The credit goes to > >Yonghong. > > Sure, and I'll add your results with different computers, for the record. > > Thanks, > > - Arnaldo > > > >Thank you, > >Hao > > > > clang 10: > > [67] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > [20168] VAR 'bpf_prog_active' type_id=67, linkage=global-alloc > > > > clang 9: > > [64] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > [19789] VAR 'bpf_prog_active' type_id=64, linkage=global-alloc > > > > gcc 10.2 > > [18] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > [20319] VAR 'bpf_prog_active' type_id=18, linkage=global-alloc > > > > gcc 9.3: > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > [21085] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > gcc 8 > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > [21084] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > gcc 6.2 > > [22] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > [21083] VAR 'bpf_prog_active' type_id=22, linkage=global-alloc > > > > gcc 4.9 > > [17] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > [20410] VAR 'bpf_prog_active' type_id=17, linkage=global-alloc > > -- > Sent from my Android device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] btf_encoder: Handle DW_TAG_variable that has DW_AT_specification 2020-09-30 6:52 ` [PATCH v1] btf_encoder: Handle DW_TAG_variable that has DW_AT_specification Hao Luo @ 2020-10-01 15:47 ` Alexei Starovoitov 2020-10-01 18:24 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 8+ messages in thread From: Alexei Starovoitov @ 2020-10-01 15:47 UTC (permalink / raw) To: Hao Luo, Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo Cc: Arnaldo Carvalho de Melo, Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau, dwarves, Yonghong Song, bpf Arnaldo, ping. Is anything blocking this fix from merging? The kernel patches are stalled waiting on the pahole. Thanks On Tue, Sep 29, 2020 at 11:52 PM Hao Luo <haoluo@google.com> wrote: > > Arnaldo, > > Is this patch ready to be merged into Pahole's master branch? Alexei > is testing the kernel patches that need this patch. Please let me know > if there is anything I can do to help merging. > > Thank you, > Hao > > On Wed, Aug 26, 2020 at 6:56 PM Arnaldo Carvalho de Melo > <arnaldo.melo@gmail.com> wrote: > > > > > > > > On August 26, 2020 3:35:17 PM GMT-03:00, Hao Luo <haoluo@google.com> wrote: > > >Arnaldo, > > > > > >On Wed, Aug 26, 2020 at 6:12 AM Arnaldo Carvalho de Melo > > ><acme@kernel.org> wrote: > > >> > > >> Em Mon, Aug 24, 2020 at 05:45:23PM -0700, Hao Luo escreveu: > > >> > It is found on gcc 8.2 that global percpu variables generate the > > >> > following dwarf entry in the cu where the variable is defined[1]. > > >> > > > >> > Take the global variable "bpf_prog_active" defined in > > >> > kernel/bpf/syscall.c as an example. The debug info for syscall.c > > >> > has two dwarf entries for "bpf_prog_active". > > >> > > > >[...] > > >> > > >> Interesting, here I get, with binutils' readelf: > > >> > > >> [root@quaco perf]# readelf -wi > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > > >bpf_prog_active > > >> [root@quaco perf]# > > >> > > >> Just one, as: > > >> > > >> [root@quaco perf]# readelf -wi > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active -B1 -A8 > > >> <1><f6a0>: Abbrev Number: 103 (DW_TAG_variable) > > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > > >bpf_prog_active > > >> <f6a5> DW_AT_decl_file : 11 > > >> <f6a6> DW_AT_decl_line : 1008 > > >> <f6a8> DW_AT_decl_column : 1 > > >> <f6a9> DW_AT_type : <0xcf> > > >> <f6ad> DW_AT_external : 1 > > >> <f6ad> DW_AT_declaration : 1 > > >> <1><f6ad>: Abbrev Number: 103 (DW_TAG_variable) > > >> <f6ae> DW_AT_name : (indirect string, offset: 0x3a5d): > > >bpf_stats_enabled_mutex > > >> [root@quaco perf]# > > >> > > >> I get what you have when I use elfutils' readelf: > > >> > > >> [root@quaco perf]# eu-readelf -winfo > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > > >> name (strp) "bpf_prog_active" > > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > > >> [root@quaco perf]# > > >> > > >> [root@quaco perf]# eu-readelf -winfo > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B1 -A8 > > >\"bpf_prog_active\" > > >> [ f6a0] variable abbrev: 103 > > >> name (strp) "bpf_prog_active" > > >> decl_file (data1) bpf.h (11) > > >> decl_line (data2) 1008 > > >> decl_column (data1) 1 > > >> type (ref4) [ cf] > > >> external (flag_present) yes > > >> declaration (flag_present) yes > > >> [ f6ad] variable abbrev: 103 > > >> name (strp) "bpf_stats_enabled_mutex" > > >> [root@quaco perf]# > > >> > > >> And: > > >> > > >> [root@quaco perf]# eu-readelf -winfo > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B5 \<bpf_prog_active\> > > >> [ 1bdf5] variable abbrev: 212 > > >> specification (ref4) [ f6a0] > > >> decl_file (data1) syscall.c (1) > > >> decl_line (data1) 43 > > >> location (exprloc) > > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > > >> [root@quaco perf]# > > >> > > > > > >In binutils readelf, there is a extra entry > > > > Not here, tomorrow I'll triple check. > > > > > > > > <1><1b24c>: Abbrev Number: 195 (DW_TAG_variable) > > > <1b24e> DW_AT_specification: <0xf335> > > > <1b252> DW_AT_decl_file : 1 > > > <1b253> DW_AT_decl_line : 43 > > > <1b254> DW_AT_location : 9 byte block: 3 0 0 0 0 0 0 0 0 > > > (DW_OP_addr: 0) > > > > > >which points to > > > > > > <1><f335>: Abbrev Number: 95 (DW_TAG_variable) > > > <f336> DW_AT_name : (indirect string, offset: 0xb37a): > > >bpf_prog_active > > > > > >It just doesn't have the string 'bpf_prog_active', annotating entry. > > >So eu-readelf and binutils readelf have the same results. > > > > > >> > Note that second DW_TAG_variable entry contains specification that > > >> > points to the first entry. > > >> > > >> So you are not considering the first when encoding since it is just a > > >> DW_AT_declaration, considers the second, as it should be, and then > > >needs > > >> to go see its DW_AT_specification, right? > > >> > > >> Sounds correct, applying, will test further and then push out, > > >> > > > > > >Yes, exactly. The var tags to be considered are those that either have > > >DW_AT_specification or not have DW_AT_declaration. This makes sure > > >btf_encoder works correctly on both old and new gcc. > > > > > >> Thanks, > > >> > > >> - Arnaldo > > > > > >Suggested by Yonghong, I tested this change on a larger set of > > >compilers this time and works correctly. See below. > > > > > >Could you also add 'Reported-by: Yonghong Song <yhs@fb.com>'? I should > > >have done that when sending out this patch. The credit goes to > > >Yonghong. > > > > Sure, and I'll add your results with different computers, for the record. > > > > Thanks, > > > > - Arnaldo > > > > > >Thank you, > > >Hao > > > > > > clang 10: > > > [67] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > [20168] VAR 'bpf_prog_active' type_id=67, linkage=global-alloc > > > > > > clang 9: > > > [64] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > [19789] VAR 'bpf_prog_active' type_id=64, linkage=global-alloc > > > > > > gcc 10.2 > > > [18] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > [20319] VAR 'bpf_prog_active' type_id=18, linkage=global-alloc > > > > > > gcc 9.3: > > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > [21085] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > > > gcc 8 > > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > [21084] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > > > gcc 6.2 > > > [22] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > [21083] VAR 'bpf_prog_active' type_id=22, linkage=global-alloc > > > > > > gcc 4.9 > > > [17] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > [20410] VAR 'bpf_prog_active' type_id=17, linkage=global-alloc > > > > -- > > Sent from my Android device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] btf_encoder: Handle DW_TAG_variable that has DW_AT_specification 2020-10-01 15:47 ` Alexei Starovoitov @ 2020-10-01 18:24 ` Arnaldo Carvalho de Melo 2020-10-01 18:40 ` Hao Luo 0 siblings, 1 reply; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-10-01 18:24 UTC (permalink / raw) To: Alexei Starovoitov Cc: Hao Luo, Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo, Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau, dwarves, Yonghong Song, bpf Em Thu, Oct 01, 2020 at 08:47:51AM -0700, Alexei Starovoitov escreveu: > Arnaldo, > > ping. > Is anything blocking this fix from merging? > The kernel patches are stalled waiting on the pahole. Applied locally, testing now, will push to the main branch ASAP. - Arnaldo > Thanks > On Tue, Sep 29, 2020 at 11:52 PM Hao Luo <haoluo@google.com> wrote: > > > > Arnaldo, > > > > Is this patch ready to be merged into Pahole's master branch? Alexei > > is testing the kernel patches that need this patch. Please let me know > > if there is anything I can do to help merging. > > > > Thank you, > > Hao > > > > On Wed, Aug 26, 2020 at 6:56 PM Arnaldo Carvalho de Melo > > <arnaldo.melo@gmail.com> wrote: > > > > > > > > > > > > On August 26, 2020 3:35:17 PM GMT-03:00, Hao Luo <haoluo@google.com> wrote: > > > >Arnaldo, > > > > > > > >On Wed, Aug 26, 2020 at 6:12 AM Arnaldo Carvalho de Melo > > > ><acme@kernel.org> wrote: > > > >> > > > >> Em Mon, Aug 24, 2020 at 05:45:23PM -0700, Hao Luo escreveu: > > > >> > It is found on gcc 8.2 that global percpu variables generate the > > > >> > following dwarf entry in the cu where the variable is defined[1]. > > > >> > > > > >> > Take the global variable "bpf_prog_active" defined in > > > >> > kernel/bpf/syscall.c as an example. The debug info for syscall.c > > > >> > has two dwarf entries for "bpf_prog_active". > > > >> > > > > >[...] > > > >> > > > >> Interesting, here I get, with binutils' readelf: > > > >> > > > >> [root@quaco perf]# readelf -wi > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > > > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > > > >bpf_prog_active > > > >> [root@quaco perf]# > > > >> > > > >> Just one, as: > > > >> > > > >> [root@quaco perf]# readelf -wi > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active -B1 -A8 > > > >> <1><f6a0>: Abbrev Number: 103 (DW_TAG_variable) > > > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > > > >bpf_prog_active > > > >> <f6a5> DW_AT_decl_file : 11 > > > >> <f6a6> DW_AT_decl_line : 1008 > > > >> <f6a8> DW_AT_decl_column : 1 > > > >> <f6a9> DW_AT_type : <0xcf> > > > >> <f6ad> DW_AT_external : 1 > > > >> <f6ad> DW_AT_declaration : 1 > > > >> <1><f6ad>: Abbrev Number: 103 (DW_TAG_variable) > > > >> <f6ae> DW_AT_name : (indirect string, offset: 0x3a5d): > > > >bpf_stats_enabled_mutex > > > >> [root@quaco perf]# > > > >> > > > >> I get what you have when I use elfutils' readelf: > > > >> > > > >> [root@quaco perf]# eu-readelf -winfo > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > > > >> name (strp) "bpf_prog_active" > > > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > > > >> [root@quaco perf]# > > > >> > > > >> [root@quaco perf]# eu-readelf -winfo > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B1 -A8 > > > >\"bpf_prog_active\" > > > >> [ f6a0] variable abbrev: 103 > > > >> name (strp) "bpf_prog_active" > > > >> decl_file (data1) bpf.h (11) > > > >> decl_line (data2) 1008 > > > >> decl_column (data1) 1 > > > >> type (ref4) [ cf] > > > >> external (flag_present) yes > > > >> declaration (flag_present) yes > > > >> [ f6ad] variable abbrev: 103 > > > >> name (strp) "bpf_stats_enabled_mutex" > > > >> [root@quaco perf]# > > > >> > > > >> And: > > > >> > > > >> [root@quaco perf]# eu-readelf -winfo > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B5 \<bpf_prog_active\> > > > >> [ 1bdf5] variable abbrev: 212 > > > >> specification (ref4) [ f6a0] > > > >> decl_file (data1) syscall.c (1) > > > >> decl_line (data1) 43 > > > >> location (exprloc) > > > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > > > >> [root@quaco perf]# > > > >> > > > > > > > >In binutils readelf, there is a extra entry > > > > > > Not here, tomorrow I'll triple check. > > > > > > > > > > > <1><1b24c>: Abbrev Number: 195 (DW_TAG_variable) > > > > <1b24e> DW_AT_specification: <0xf335> > > > > <1b252> DW_AT_decl_file : 1 > > > > <1b253> DW_AT_decl_line : 43 > > > > <1b254> DW_AT_location : 9 byte block: 3 0 0 0 0 0 0 0 0 > > > > (DW_OP_addr: 0) > > > > > > > >which points to > > > > > > > > <1><f335>: Abbrev Number: 95 (DW_TAG_variable) > > > > <f336> DW_AT_name : (indirect string, offset: 0xb37a): > > > >bpf_prog_active > > > > > > > >It just doesn't have the string 'bpf_prog_active', annotating entry. > > > >So eu-readelf and binutils readelf have the same results. > > > > > > > >> > Note that second DW_TAG_variable entry contains specification that > > > >> > points to the first entry. > > > >> > > > >> So you are not considering the first when encoding since it is just a > > > >> DW_AT_declaration, considers the second, as it should be, and then > > > >needs > > > >> to go see its DW_AT_specification, right? > > > >> > > > >> Sounds correct, applying, will test further and then push out, > > > >> > > > > > > > >Yes, exactly. The var tags to be considered are those that either have > > > >DW_AT_specification or not have DW_AT_declaration. This makes sure > > > >btf_encoder works correctly on both old and new gcc. > > > > > > > >> Thanks, > > > >> > > > >> - Arnaldo > > > > > > > >Suggested by Yonghong, I tested this change on a larger set of > > > >compilers this time and works correctly. See below. > > > > > > > >Could you also add 'Reported-by: Yonghong Song <yhs@fb.com>'? I should > > > >have done that when sending out this patch. The credit goes to > > > >Yonghong. > > > > > > Sure, and I'll add your results with different computers, for the record. > > > > > > Thanks, > > > > > > - Arnaldo > > > > > > > >Thank you, > > > >Hao > > > > > > > > clang 10: > > > > [67] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > [20168] VAR 'bpf_prog_active' type_id=67, linkage=global-alloc > > > > > > > > clang 9: > > > > [64] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > [19789] VAR 'bpf_prog_active' type_id=64, linkage=global-alloc > > > > > > > > gcc 10.2 > > > > [18] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > [20319] VAR 'bpf_prog_active' type_id=18, linkage=global-alloc > > > > > > > > gcc 9.3: > > > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > [21085] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > > > > > gcc 8 > > > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > [21084] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > > > > > gcc 6.2 > > > > [22] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > [21083] VAR 'bpf_prog_active' type_id=22, linkage=global-alloc > > > > > > > > gcc 4.9 > > > > [17] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > [20410] VAR 'bpf_prog_active' type_id=17, linkage=global-alloc > > > > > > -- > > > Sent from my Android device with K-9 Mail. Please excuse my brevity. -- - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] btf_encoder: Handle DW_TAG_variable that has DW_AT_specification 2020-10-01 18:24 ` Arnaldo Carvalho de Melo @ 2020-10-01 18:40 ` Hao Luo 2020-10-01 20:27 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 8+ messages in thread From: Hao Luo @ 2020-10-01 18:40 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Alexei Starovoitov, Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo, Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau, dwarves, Yonghong Song, bpf Thanks! On Thu, Oct 1, 2020 at 11:24 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Thu, Oct 01, 2020 at 08:47:51AM -0700, Alexei Starovoitov escreveu: > > Arnaldo, > > > > ping. > > Is anything blocking this fix from merging? > > The kernel patches are stalled waiting on the pahole. > > Applied locally, testing now, will push to the main branch ASAP. > > - Arnaldo > > > Thanks > > On Tue, Sep 29, 2020 at 11:52 PM Hao Luo <haoluo@google.com> wrote: > > > > > > Arnaldo, > > > > > > Is this patch ready to be merged into Pahole's master branch? Alexei > > > is testing the kernel patches that need this patch. Please let me know > > > if there is anything I can do to help merging. > > > > > > Thank you, > > > Hao > > > > > > On Wed, Aug 26, 2020 at 6:56 PM Arnaldo Carvalho de Melo > > > <arnaldo.melo@gmail.com> wrote: > > > > > > > > > > > > > > > > On August 26, 2020 3:35:17 PM GMT-03:00, Hao Luo <haoluo@google.com> wrote: > > > > >Arnaldo, > > > > > > > > > >On Wed, Aug 26, 2020 at 6:12 AM Arnaldo Carvalho de Melo > > > > ><acme@kernel.org> wrote: > > > > >> > > > > >> Em Mon, Aug 24, 2020 at 05:45:23PM -0700, Hao Luo escreveu: > > > > >> > It is found on gcc 8.2 that global percpu variables generate the > > > > >> > following dwarf entry in the cu where the variable is defined[1]. > > > > >> > > > > > >> > Take the global variable "bpf_prog_active" defined in > > > > >> > kernel/bpf/syscall.c as an example. The debug info for syscall.c > > > > >> > has two dwarf entries for "bpf_prog_active". > > > > >> > > > > > >[...] > > > > >> > > > > >> Interesting, here I get, with binutils' readelf: > > > > >> > > > > >> [root@quaco perf]# readelf -wi > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > > > > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > > > > >bpf_prog_active > > > > >> [root@quaco perf]# > > > > >> > > > > >> Just one, as: > > > > >> > > > > >> [root@quaco perf]# readelf -wi > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active -B1 -A8 > > > > >> <1><f6a0>: Abbrev Number: 103 (DW_TAG_variable) > > > > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > > > > >bpf_prog_active > > > > >> <f6a5> DW_AT_decl_file : 11 > > > > >> <f6a6> DW_AT_decl_line : 1008 > > > > >> <f6a8> DW_AT_decl_column : 1 > > > > >> <f6a9> DW_AT_type : <0xcf> > > > > >> <f6ad> DW_AT_external : 1 > > > > >> <f6ad> DW_AT_declaration : 1 > > > > >> <1><f6ad>: Abbrev Number: 103 (DW_TAG_variable) > > > > >> <f6ae> DW_AT_name : (indirect string, offset: 0x3a5d): > > > > >bpf_stats_enabled_mutex > > > > >> [root@quaco perf]# > > > > >> > > > > >> I get what you have when I use elfutils' readelf: > > > > >> > > > > >> [root@quaco perf]# eu-readelf -winfo > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > > > > >> name (strp) "bpf_prog_active" > > > > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > > > > >> [root@quaco perf]# > > > > >> > > > > >> [root@quaco perf]# eu-readelf -winfo > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B1 -A8 > > > > >\"bpf_prog_active\" > > > > >> [ f6a0] variable abbrev: 103 > > > > >> name (strp) "bpf_prog_active" > > > > >> decl_file (data1) bpf.h (11) > > > > >> decl_line (data2) 1008 > > > > >> decl_column (data1) 1 > > > > >> type (ref4) [ cf] > > > > >> external (flag_present) yes > > > > >> declaration (flag_present) yes > > > > >> [ f6ad] variable abbrev: 103 > > > > >> name (strp) "bpf_stats_enabled_mutex" > > > > >> [root@quaco perf]# > > > > >> > > > > >> And: > > > > >> > > > > >> [root@quaco perf]# eu-readelf -winfo > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B5 \<bpf_prog_active\> > > > > >> [ 1bdf5] variable abbrev: 212 > > > > >> specification (ref4) [ f6a0] > > > > >> decl_file (data1) syscall.c (1) > > > > >> decl_line (data1) 43 > > > > >> location (exprloc) > > > > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > > > > >> [root@quaco perf]# > > > > >> > > > > > > > > > >In binutils readelf, there is a extra entry > > > > > > > > Not here, tomorrow I'll triple check. > > > > > > > > > > > > > > <1><1b24c>: Abbrev Number: 195 (DW_TAG_variable) > > > > > <1b24e> DW_AT_specification: <0xf335> > > > > > <1b252> DW_AT_decl_file : 1 > > > > > <1b253> DW_AT_decl_line : 43 > > > > > <1b254> DW_AT_location : 9 byte block: 3 0 0 0 0 0 0 0 0 > > > > > (DW_OP_addr: 0) > > > > > > > > > >which points to > > > > > > > > > > <1><f335>: Abbrev Number: 95 (DW_TAG_variable) > > > > > <f336> DW_AT_name : (indirect string, offset: 0xb37a): > > > > >bpf_prog_active > > > > > > > > > >It just doesn't have the string 'bpf_prog_active', annotating entry. > > > > >So eu-readelf and binutils readelf have the same results. > > > > > > > > > >> > Note that second DW_TAG_variable entry contains specification that > > > > >> > points to the first entry. > > > > >> > > > > >> So you are not considering the first when encoding since it is just a > > > > >> DW_AT_declaration, considers the second, as it should be, and then > > > > >needs > > > > >> to go see its DW_AT_specification, right? > > > > >> > > > > >> Sounds correct, applying, will test further and then push out, > > > > >> > > > > > > > > > >Yes, exactly. The var tags to be considered are those that either have > > > > >DW_AT_specification or not have DW_AT_declaration. This makes sure > > > > >btf_encoder works correctly on both old and new gcc. > > > > > > > > > >> Thanks, > > > > >> > > > > >> - Arnaldo > > > > > > > > > >Suggested by Yonghong, I tested this change on a larger set of > > > > >compilers this time and works correctly. See below. > > > > > > > > > >Could you also add 'Reported-by: Yonghong Song <yhs@fb.com>'? I should > > > > >have done that when sending out this patch. The credit goes to > > > > >Yonghong. > > > > > > > > Sure, and I'll add your results with different computers, for the record. > > > > > > > > Thanks, > > > > > > > > - Arnaldo > > > > > > > > > >Thank you, > > > > >Hao > > > > > > > > > > clang 10: > > > > > [67] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > [20168] VAR 'bpf_prog_active' type_id=67, linkage=global-alloc > > > > > > > > > > clang 9: > > > > > [64] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > [19789] VAR 'bpf_prog_active' type_id=64, linkage=global-alloc > > > > > > > > > > gcc 10.2 > > > > > [18] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > [20319] VAR 'bpf_prog_active' type_id=18, linkage=global-alloc > > > > > > > > > > gcc 9.3: > > > > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > [21085] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > > > > > > > gcc 8 > > > > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > [21084] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > > > > > > > gcc 6.2 > > > > > [22] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > [21083] VAR 'bpf_prog_active' type_id=22, linkage=global-alloc > > > > > > > > > > gcc 4.9 > > > > > [17] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > [20410] VAR 'bpf_prog_active' type_id=17, linkage=global-alloc > > > > > > > > -- > > > > Sent from my Android device with K-9 Mail. Please excuse my brevity. > > -- > > - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] btf_encoder: Handle DW_TAG_variable that has DW_AT_specification 2020-10-01 18:40 ` Hao Luo @ 2020-10-01 20:27 ` Arnaldo Carvalho de Melo 2020-10-01 20:57 ` Hao Luo 0 siblings, 1 reply; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-10-01 20:27 UTC (permalink / raw) To: Hao Luo Cc: Alexei Starovoitov, Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo, Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau, dwarves, Yonghong Song, bpf Em Thu, Oct 01, 2020 at 11:40:25AM -0700, Hao Luo escreveu: > Thanks! I must just apologise not having this in an officially released version yet, getting constantly postponed due to bug reports about corner cases and some features I got carried away working on, I'll fast pace a new version to avoid getting in the way of the larger eBPF effort. - Arnaldo > On Thu, Oct 1, 2020 at 11:24 AM Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > > > Em Thu, Oct 01, 2020 at 08:47:51AM -0700, Alexei Starovoitov escreveu: > > > Arnaldo, > > > > > > ping. > > > Is anything blocking this fix from merging? > > > The kernel patches are stalled waiting on the pahole. > > > > Applied locally, testing now, will push to the main branch ASAP. > > > > - Arnaldo > > > > > Thanks > > > On Tue, Sep 29, 2020 at 11:52 PM Hao Luo <haoluo@google.com> wrote: > > > > > > > > Arnaldo, > > > > > > > > Is this patch ready to be merged into Pahole's master branch? Alexei > > > > is testing the kernel patches that need this patch. Please let me know > > > > if there is anything I can do to help merging. > > > > > > > > Thank you, > > > > Hao > > > > > > > > On Wed, Aug 26, 2020 at 6:56 PM Arnaldo Carvalho de Melo > > > > <arnaldo.melo@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > On August 26, 2020 3:35:17 PM GMT-03:00, Hao Luo <haoluo@google.com> wrote: > > > > > >Arnaldo, > > > > > > > > > > > >On Wed, Aug 26, 2020 at 6:12 AM Arnaldo Carvalho de Melo > > > > > ><acme@kernel.org> wrote: > > > > > >> > > > > > >> Em Mon, Aug 24, 2020 at 05:45:23PM -0700, Hao Luo escreveu: > > > > > >> > It is found on gcc 8.2 that global percpu variables generate the > > > > > >> > following dwarf entry in the cu where the variable is defined[1]. > > > > > >> > > > > > > >> > Take the global variable "bpf_prog_active" defined in > > > > > >> > kernel/bpf/syscall.c as an example. The debug info for syscall.c > > > > > >> > has two dwarf entries for "bpf_prog_active". > > > > > >> > > > > > > >[...] > > > > > >> > > > > > >> Interesting, here I get, with binutils' readelf: > > > > > >> > > > > > >> [root@quaco perf]# readelf -wi > > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > > > > > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > > > > > >bpf_prog_active > > > > > >> [root@quaco perf]# > > > > > >> > > > > > >> Just one, as: > > > > > >> > > > > > >> [root@quaco perf]# readelf -wi > > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active -B1 -A8 > > > > > >> <1><f6a0>: Abbrev Number: 103 (DW_TAG_variable) > > > > > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > > > > > >bpf_prog_active > > > > > >> <f6a5> DW_AT_decl_file : 11 > > > > > >> <f6a6> DW_AT_decl_line : 1008 > > > > > >> <f6a8> DW_AT_decl_column : 1 > > > > > >> <f6a9> DW_AT_type : <0xcf> > > > > > >> <f6ad> DW_AT_external : 1 > > > > > >> <f6ad> DW_AT_declaration : 1 > > > > > >> <1><f6ad>: Abbrev Number: 103 (DW_TAG_variable) > > > > > >> <f6ae> DW_AT_name : (indirect string, offset: 0x3a5d): > > > > > >bpf_stats_enabled_mutex > > > > > >> [root@quaco perf]# > > > > > >> > > > > > >> I get what you have when I use elfutils' readelf: > > > > > >> > > > > > >> [root@quaco perf]# eu-readelf -winfo > > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > > > > > >> name (strp) "bpf_prog_active" > > > > > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > > > > > >> [root@quaco perf]# > > > > > >> > > > > > >> [root@quaco perf]# eu-readelf -winfo > > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B1 -A8 > > > > > >\"bpf_prog_active\" > > > > > >> [ f6a0] variable abbrev: 103 > > > > > >> name (strp) "bpf_prog_active" > > > > > >> decl_file (data1) bpf.h (11) > > > > > >> decl_line (data2) 1008 > > > > > >> decl_column (data1) 1 > > > > > >> type (ref4) [ cf] > > > > > >> external (flag_present) yes > > > > > >> declaration (flag_present) yes > > > > > >> [ f6ad] variable abbrev: 103 > > > > > >> name (strp) "bpf_stats_enabled_mutex" > > > > > >> [root@quaco perf]# > > > > > >> > > > > > >> And: > > > > > >> > > > > > >> [root@quaco perf]# eu-readelf -winfo > > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B5 \<bpf_prog_active\> > > > > > >> [ 1bdf5] variable abbrev: 212 > > > > > >> specification (ref4) [ f6a0] > > > > > >> decl_file (data1) syscall.c (1) > > > > > >> decl_line (data1) 43 > > > > > >> location (exprloc) > > > > > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > > > > > >> [root@quaco perf]# > > > > > >> > > > > > > > > > > > >In binutils readelf, there is a extra entry > > > > > > > > > > Not here, tomorrow I'll triple check. > > > > > > > > > > > > > > > > > <1><1b24c>: Abbrev Number: 195 (DW_TAG_variable) > > > > > > <1b24e> DW_AT_specification: <0xf335> > > > > > > <1b252> DW_AT_decl_file : 1 > > > > > > <1b253> DW_AT_decl_line : 43 > > > > > > <1b254> DW_AT_location : 9 byte block: 3 0 0 0 0 0 0 0 0 > > > > > > (DW_OP_addr: 0) > > > > > > > > > > > >which points to > > > > > > > > > > > > <1><f335>: Abbrev Number: 95 (DW_TAG_variable) > > > > > > <f336> DW_AT_name : (indirect string, offset: 0xb37a): > > > > > >bpf_prog_active > > > > > > > > > > > >It just doesn't have the string 'bpf_prog_active', annotating entry. > > > > > >So eu-readelf and binutils readelf have the same results. > > > > > > > > > > > >> > Note that second DW_TAG_variable entry contains specification that > > > > > >> > points to the first entry. > > > > > >> > > > > > >> So you are not considering the first when encoding since it is just a > > > > > >> DW_AT_declaration, considers the second, as it should be, and then > > > > > >needs > > > > > >> to go see its DW_AT_specification, right? > > > > > >> > > > > > >> Sounds correct, applying, will test further and then push out, > > > > > >> > > > > > > > > > > > >Yes, exactly. The var tags to be considered are those that either have > > > > > >DW_AT_specification or not have DW_AT_declaration. This makes sure > > > > > >btf_encoder works correctly on both old and new gcc. > > > > > > > > > > > >> Thanks, > > > > > >> > > > > > >> - Arnaldo > > > > > > > > > > > >Suggested by Yonghong, I tested this change on a larger set of > > > > > >compilers this time and works correctly. See below. > > > > > > > > > > > >Could you also add 'Reported-by: Yonghong Song <yhs@fb.com>'? I should > > > > > >have done that when sending out this patch. The credit goes to > > > > > >Yonghong. > > > > > > > > > > Sure, and I'll add your results with different computers, for the record. > > > > > > > > > > Thanks, > > > > > > > > > > - Arnaldo > > > > > > > > > > > >Thank you, > > > > > >Hao > > > > > > > > > > > > clang 10: > > > > > > [67] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > [20168] VAR 'bpf_prog_active' type_id=67, linkage=global-alloc > > > > > > > > > > > > clang 9: > > > > > > [64] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > [19789] VAR 'bpf_prog_active' type_id=64, linkage=global-alloc > > > > > > > > > > > > gcc 10.2 > > > > > > [18] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > [20319] VAR 'bpf_prog_active' type_id=18, linkage=global-alloc > > > > > > > > > > > > gcc 9.3: > > > > > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > [21085] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > > > > > > > > > gcc 8 > > > > > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > [21084] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > > > > > > > > > gcc 6.2 > > > > > > [22] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > [21083] VAR 'bpf_prog_active' type_id=22, linkage=global-alloc > > > > > > > > > > > > gcc 4.9 > > > > > > [17] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > [20410] VAR 'bpf_prog_active' type_id=17, linkage=global-alloc > > > > > > > > > > -- > > > > > Sent from my Android device with K-9 Mail. Please excuse my brevity. > > > > -- > > > > - Arnaldo -- - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] btf_encoder: Handle DW_TAG_variable that has DW_AT_specification 2020-10-01 20:27 ` Arnaldo Carvalho de Melo @ 2020-10-01 20:57 ` Hao Luo 2020-10-01 21:07 ` Alexei Starovoitov 0 siblings, 1 reply; 8+ messages in thread From: Hao Luo @ 2020-10-01 20:57 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Alexei Starovoitov, Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo, Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau, dwarves, Yonghong Song, bpf Arnaldo, thanks for the update. In that case, I think on the kernel side I need to skip encoding percpu vars for this pahole release, and re-enable for the next pahole release. (assuming the flag for opt-out is in this release). Alexei, do you have any better idea? Hao On Thu, Oct 1, 2020 at 1:27 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Thu, Oct 01, 2020 at 11:40:25AM -0700, Hao Luo escreveu: > > Thanks! > > I must just apologise not having this in an officially released version > yet, getting constantly postponed due to bug reports about corner cases > and some features I got carried away working on, I'll fast pace a new > version to avoid getting in the way of the larger eBPF effort. > > - Arnaldo > > > On Thu, Oct 1, 2020 at 11:24 AM Arnaldo Carvalho de Melo > > <acme@kernel.org> wrote: > > > > > > Em Thu, Oct 01, 2020 at 08:47:51AM -0700, Alexei Starovoitov escreveu: > > > > Arnaldo, > > > > > > > > ping. > > > > Is anything blocking this fix from merging? > > > > The kernel patches are stalled waiting on the pahole. > > > > > > Applied locally, testing now, will push to the main branch ASAP. > > > > > > - Arnaldo > > > > > > > Thanks > > > > On Tue, Sep 29, 2020 at 11:52 PM Hao Luo <haoluo@google.com> wrote: > > > > > > > > > > Arnaldo, > > > > > > > > > > Is this patch ready to be merged into Pahole's master branch? Alexei > > > > > is testing the kernel patches that need this patch. Please let me know > > > > > if there is anything I can do to help merging. > > > > > > > > > > Thank you, > > > > > Hao > > > > > > > > > > On Wed, Aug 26, 2020 at 6:56 PM Arnaldo Carvalho de Melo > > > > > <arnaldo.melo@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > On August 26, 2020 3:35:17 PM GMT-03:00, Hao Luo <haoluo@google.com> wrote: > > > > > > >Arnaldo, > > > > > > > > > > > > > >On Wed, Aug 26, 2020 at 6:12 AM Arnaldo Carvalho de Melo > > > > > > ><acme@kernel.org> wrote: > > > > > > >> > > > > > > >> Em Mon, Aug 24, 2020 at 05:45:23PM -0700, Hao Luo escreveu: > > > > > > >> > It is found on gcc 8.2 that global percpu variables generate the > > > > > > >> > following dwarf entry in the cu where the variable is defined[1]. > > > > > > >> > > > > > > > >> > Take the global variable "bpf_prog_active" defined in > > > > > > >> > kernel/bpf/syscall.c as an example. The debug info for syscall.c > > > > > > >> > has two dwarf entries for "bpf_prog_active". > > > > > > >> > > > > > > > >[...] > > > > > > >> > > > > > > >> Interesting, here I get, with binutils' readelf: > > > > > > >> > > > > > > >> [root@quaco perf]# readelf -wi > > > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > > > > > > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > > > > > > >bpf_prog_active > > > > > > >> [root@quaco perf]# > > > > > > >> > > > > > > >> Just one, as: > > > > > > >> > > > > > > >> [root@quaco perf]# readelf -wi > > > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active -B1 -A8 > > > > > > >> <1><f6a0>: Abbrev Number: 103 (DW_TAG_variable) > > > > > > >> <f6a1> DW_AT_name : (indirect string, offset: 0xb70d): > > > > > > >bpf_prog_active > > > > > > >> <f6a5> DW_AT_decl_file : 11 > > > > > > >> <f6a6> DW_AT_decl_line : 1008 > > > > > > >> <f6a8> DW_AT_decl_column : 1 > > > > > > >> <f6a9> DW_AT_type : <0xcf> > > > > > > >> <f6ad> DW_AT_external : 1 > > > > > > >> <f6ad> DW_AT_declaration : 1 > > > > > > >> <1><f6ad>: Abbrev Number: 103 (DW_TAG_variable) > > > > > > >> <f6ae> DW_AT_name : (indirect string, offset: 0x3a5d): > > > > > > >bpf_stats_enabled_mutex > > > > > > >> [root@quaco perf]# > > > > > > >> > > > > > > >> I get what you have when I use elfutils' readelf: > > > > > > >> > > > > > > >> [root@quaco perf]# eu-readelf -winfo > > > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep bpf_prog_active > > > > > > >> name (strp) "bpf_prog_active" > > > > > > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > > > > > > >> [root@quaco perf]# > > > > > > >> > > > > > > >> [root@quaco perf]# eu-readelf -winfo > > > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B1 -A8 > > > > > > >\"bpf_prog_active\" > > > > > > >> [ f6a0] variable abbrev: 103 > > > > > > >> name (strp) "bpf_prog_active" > > > > > > >> decl_file (data1) bpf.h (11) > > > > > > >> decl_line (data2) 1008 > > > > > > >> decl_column (data1) 1 > > > > > > >> type (ref4) [ cf] > > > > > > >> external (flag_present) yes > > > > > > >> declaration (flag_present) yes > > > > > > >> [ f6ad] variable abbrev: 103 > > > > > > >> name (strp) "bpf_stats_enabled_mutex" > > > > > > >> [root@quaco perf]# > > > > > > >> > > > > > > >> And: > > > > > > >> > > > > > > >> [root@quaco perf]# eu-readelf -winfo > > > > > > >../build/v5.8-rc5+/kernel/bpf/syscall.o | grep -B5 \<bpf_prog_active\> > > > > > > >> [ 1bdf5] variable abbrev: 212 > > > > > > >> specification (ref4) [ f6a0] > > > > > > >> decl_file (data1) syscall.c (1) > > > > > > >> decl_line (data1) 43 > > > > > > >> location (exprloc) > > > > > > >> [ 0] addr .data..percpu+0 <bpf_prog_active> > > > > > > >> [root@quaco perf]# > > > > > > >> > > > > > > > > > > > > > >In binutils readelf, there is a extra entry > > > > > > > > > > > > Not here, tomorrow I'll triple check. > > > > > > > > > > > > > > > > > > > > <1><1b24c>: Abbrev Number: 195 (DW_TAG_variable) > > > > > > > <1b24e> DW_AT_specification: <0xf335> > > > > > > > <1b252> DW_AT_decl_file : 1 > > > > > > > <1b253> DW_AT_decl_line : 43 > > > > > > > <1b254> DW_AT_location : 9 byte block: 3 0 0 0 0 0 0 0 0 > > > > > > > (DW_OP_addr: 0) > > > > > > > > > > > > > >which points to > > > > > > > > > > > > > > <1><f335>: Abbrev Number: 95 (DW_TAG_variable) > > > > > > > <f336> DW_AT_name : (indirect string, offset: 0xb37a): > > > > > > >bpf_prog_active > > > > > > > > > > > > > >It just doesn't have the string 'bpf_prog_active', annotating entry. > > > > > > >So eu-readelf and binutils readelf have the same results. > > > > > > > > > > > > > >> > Note that second DW_TAG_variable entry contains specification that > > > > > > >> > points to the first entry. > > > > > > >> > > > > > > >> So you are not considering the first when encoding since it is just a > > > > > > >> DW_AT_declaration, considers the second, as it should be, and then > > > > > > >needs > > > > > > >> to go see its DW_AT_specification, right? > > > > > > >> > > > > > > >> Sounds correct, applying, will test further and then push out, > > > > > > >> > > > > > > > > > > > > > >Yes, exactly. The var tags to be considered are those that either have > > > > > > >DW_AT_specification or not have DW_AT_declaration. This makes sure > > > > > > >btf_encoder works correctly on both old and new gcc. > > > > > > > > > > > > > >> Thanks, > > > > > > >> > > > > > > >> - Arnaldo > > > > > > > > > > > > > >Suggested by Yonghong, I tested this change on a larger set of > > > > > > >compilers this time and works correctly. See below. > > > > > > > > > > > > > >Could you also add 'Reported-by: Yonghong Song <yhs@fb.com>'? I should > > > > > > >have done that when sending out this patch. The credit goes to > > > > > > >Yonghong. > > > > > > > > > > > > Sure, and I'll add your results with different computers, for the record. > > > > > > > > > > > > Thanks, > > > > > > > > > > > > - Arnaldo > > > > > > > > > > > > > >Thank you, > > > > > > >Hao > > > > > > > > > > > > > > clang 10: > > > > > > > [67] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > > [20168] VAR 'bpf_prog_active' type_id=67, linkage=global-alloc > > > > > > > > > > > > > > clang 9: > > > > > > > [64] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > > [19789] VAR 'bpf_prog_active' type_id=64, linkage=global-alloc > > > > > > > > > > > > > > gcc 10.2 > > > > > > > [18] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > > [20319] VAR 'bpf_prog_active' type_id=18, linkage=global-alloc > > > > > > > > > > > > > > gcc 9.3: > > > > > > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > > [21085] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > > > > > > > > > > > gcc 8 > > > > > > > [21] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > > [21084] VAR 'bpf_prog_active' type_id=21, linkage=global-alloc > > > > > > > > > > > > > > gcc 6.2 > > > > > > > [22] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > > [21083] VAR 'bpf_prog_active' type_id=22, linkage=global-alloc > > > > > > > > > > > > > > gcc 4.9 > > > > > > > [17] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > > > > > > [20410] VAR 'bpf_prog_active' type_id=17, linkage=global-alloc > > > > > > > > > > > > -- > > > > > > Sent from my Android device with K-9 Mail. Please excuse my brevity. > > > > > > -- > > > > > > - Arnaldo > > -- > > - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] btf_encoder: Handle DW_TAG_variable that has DW_AT_specification 2020-10-01 20:57 ` Hao Luo @ 2020-10-01 21:07 ` Alexei Starovoitov 2020-10-01 21:31 ` Hao Luo 0 siblings, 1 reply; 8+ messages in thread From: Alexei Starovoitov @ 2020-10-01 21:07 UTC (permalink / raw) To: Hao Luo Cc: Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo, Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau, dwarves, Yonghong Song, bpf On Thu, Oct 1, 2020 at 1:57 PM Hao Luo <haoluo@google.com> wrote: > > Arnaldo, thanks for the update. In that case, I think on the kernel > side I need to skip encoding percpu vars for this pahole release, and > re-enable for the next pahole release. (assuming the flag for opt-out > is in this release). Alexei, do you have any better idea? I'm not following. Let's get this fix landed in pahole and release new 1.18 with it. The opt-out flag is orthogonal. I can be done in 1.19 or whenever. With your kernel patches the kernel will reject percpu vars when pahole is too old, because they will not be found in vmlinux btf, so I don't see any compatibility issues. There is no need to bump the required min version of pahole in scripts/link-vmlinux.sh. It can stay as v1.16. We only need clean verifier message that percpu BTF is not found and the kernel needs to be rebuilt with pahole 1.18. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] btf_encoder: Handle DW_TAG_variable that has DW_AT_specification 2020-10-01 21:07 ` Alexei Starovoitov @ 2020-10-01 21:31 ` Hao Luo 0 siblings, 0 replies; 8+ messages in thread From: Hao Luo @ 2020-10-01 21:31 UTC (permalink / raw) To: Alexei Starovoitov Cc: Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo, Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau, dwarves, Yonghong Song, bpf On Thu, Oct 1, 2020 at 2:07 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Thu, Oct 1, 2020 at 1:57 PM Hao Luo <haoluo@google.com> wrote: > > > > Arnaldo, thanks for the update. In that case, I think on the kernel > > side I need to skip encoding percpu vars for this pahole release, and > > re-enable for the next pahole release. (assuming the flag for opt-out > > is in this release). Alexei, do you have any better idea? > > I'm not following. Let's get this fix landed in pahole and > release new 1.18 with it. Sorry, I misunderstood Arnaldo. I thought what he was saying was, not having this fix in an officially released version (I took this as the new 1.18) and will get 1.19 quicker. It would be ideal if we can have this in 1.18. Sorry for my bad reading. > The opt-out flag is orthogonal. I can be done in 1.19 or whenever. > With your kernel patches the kernel will reject percpu vars when pahole > is too old, because they will not be found in vmlinux btf, > so I don't see any compatibility issues. > There is no need to bump the required min version of pahole in > scripts/link-vmlinux.sh. > It can stay as v1.16. We only need clean verifier message that percpu BTF > is not found and the kernel needs to be rebuilt with pahole 1.18. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-01 21:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200825004523.1353133-1-haoluo@google.com>
[not found] ` <20200826131143.GF1059382@kernel.org>
[not found] ` <CA+khW7jf7Z=sMC1u5eyn6XOZDTFJaNjV-D0ogvQSyUGSKjC3LQ@mail.gmail.com>
[not found] ` <DEC4CC81-88CE-4476-A631-2BBB6E922F5C@gmail.com>
2020-09-30 6:52 ` [PATCH v1] btf_encoder: Handle DW_TAG_variable that has DW_AT_specification Hao Luo
2020-10-01 15:47 ` Alexei Starovoitov
2020-10-01 18:24 ` Arnaldo Carvalho de Melo
2020-10-01 18:40 ` Hao Luo
2020-10-01 20:27 ` Arnaldo Carvalho de Melo
2020-10-01 20:57 ` Hao Luo
2020-10-01 21:07 ` Alexei Starovoitov
2020-10-01 21:31 ` Hao Luo
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).