Re: [PATCH] perf: libdw support for powerpc

Re: [PATCH] perf: libdw support for powerpc

[Naveen N. Rao]

Paolo Bonzini wrote:
> The ARM and x86 architectures already use libdw, and it is useful to
> have as much common code for the unwinder as possible.  Porting PPC
> to libdw only needs an architecture-specific hook to move the register
> state from perf to libdw.

Thanks. Ravi has had a similar patch locally, but from what I=20
understand, there are issues with libdw based unwinding on powerpc64. I=20
gave this a quick test and I don't see the user-space call trace being=20
unwinded properly with libdw.

With the same perf.data, with libunwind:

bswap 19286 134866.199717:     321955 cycles:=20
        c0000000000b5e60 [unknown] ([unknown])
                   b31bc pSeries_lpar_hpte_invalidate (/boot/vmlinux)
                   76f10 flush_hash_page (/boot/vmlinux)
                   7ddec hpte_need_flush (/boot/vmlinux)
                  2d9f64 ptep_clear_flush (/boot/vmlinux)
                  2c24dc wp_page_copy (/boot/vmlinux)
                  2c5c44 do_wp_page (/boot/vmlinux)
                  2cabcc __handle_mm_fault (/boot/vmlinux)
                  2cbbe8 handle_mm_fault (/boot/vmlinux)
                   71074 do_page_fault (/boot/vmlinux)
                   1b078 handle_page_fault (/boot/vmlinux)
                    9958 _dl_map_object_from_fd (/lib/powerpc64le-linux-gnu=
/ld-2.24.so)
                    e70c _dl_map_object (/lib/powerpc64le-linux-gnu/ld-2.24=
.so)
                   18da4 openaux (/lib/powerpc64le-linux-gnu/ld-2.24.so)
                   1c068 _dl_catch_error (/lib/powerpc64le-linux-gnu/ld-2.2=
4.so)
                   19430 _dl_map_object_deps (/lib/powerpc64le-linux-gnu/ld=
-2.24.so)
                    41dc dl_main (/lib/powerpc64le-linux-gnu/ld-2.24.so)
                   29954 _dl_sysdep_start (/lib/powerpc64le-linux-gnu/ld-2.=
24.so)
                    1be0 _dl_start_final (/lib/powerpc64le-linux-gnu/ld-2.2=
4.so)
                    6080 _dl_start (/lib/powerpc64le-linux-gnu/ld-2.24.so)
                    1398 _start (/lib/powerpc64le-linux-gnu/ld-2.24.so)

... and with libdw:

bswap 19286 134866.199717:     321955 cycles:=20
        c0000000000b5e60 [unknown] ([unknown])
                   b31bc pSeries_lpar_hpte_invalidate (/boot/vmlinux)
                   76f10 flush_hash_page (/boot/vmlinux)
		   7ddec hpte_need_flush (/boot/vmlinux)
                  2d9f64 ptep_clear_flush (/boot/vmlinux)
                  2c24dc wp_page_copy (/boot/vmlinux)
                  2c5c44 do_wp_page (/boot/vmlinux)
                  2cabcc __handle_mm_fault (/boot/vmlinux)
                  2cbbe8 handle_mm_fault (/boot/vmlinux)
		   71074 do_page_fault (/boot/vmlinux)
                   1b078 handle_page_fault (/boot/vmlinux)

Ravi is OOO currently can will have more to say on this once he is back=20
next week.


>=20
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tools/perf/Makefile.config                  |  2 +-
>  tools/perf/arch/powerpc/util/Build          |  2 +
>  tools/perf/arch/powerpc/util/unwind-libdw.c | 73 +++++++++++++++++++++++=
++++++
>  3 files changed, 76 insertions(+), 1 deletion(-)
>  create mode 100644 tools/perf/arch/powerpc/util/unwind-libdw.c
>=20
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index f67ae83..342c57c 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -61,7 +61,7 @@ endif
>  # Disable it on all other architectures in case libdw unwind
>  # support is detected in system. Add supported architectures
>  # to the check.
> -ifneq ($(ARCH),$(filter $(ARCH),x86 arm))
> +ifneq ($(ARCH),$(filter $(ARCH),x86 arm powerpc))
>    NO_LIBDW_DWARF_UNWIND :=3D 1
>  endif
> =20
> diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc=
/util/Build
> index 90ad64b..2e65953 100644
> --- a/tools/perf/arch/powerpc/util/Build
> +++ b/tools/perf/arch/powerpc/util/Build
> @@ -5,4 +5,6 @@ libperf-y +=3D perf_regs.o
> =20
>  libperf-$(CONFIG_DWARF) +=3D dwarf-regs.o
>  libperf-$(CONFIG_DWARF) +=3D skip-callchain-idx.o
> +
>  libperf-$(CONFIG_LIBUNWIND) +=3D unwind-libunwind.o
> +libperf-$(CONFIG_LIBDW_DWARF_UNWIND) +=3D unwind-libdw.o
> diff --git a/tools/perf/arch/powerpc/util/unwind-libdw.c b/tools/perf/arc=
h/powerpc/util/unwind-libdw.c
> new file mode 100644
> index 0000000..c61e1d8
> --- /dev/null
> +++ b/tools/perf/arch/powerpc/util/unwind-libdw.c
> @@ -0,0 +1,73 @@
> +#include <elfutils/libdwfl.h>
> +#include "../../util/util.h"
> +#include "../../util/unwind-libdw.h"
> +#include "../../util/perf_regs.h"

As an aside, we also need to include "event.h", without which there are=20
a few compile errors.


- Naveen

> +
> +/* See backends/ppc_initreg.c and backends/ppc_regs.c in elfutils.  */
> +static const int special_regs[3][2] =3D {
> +	{ 65, PERF_REG_POWERPC_LINK },
> +	{ 101, PERF_REG_POWERPC_XER },
> +	{ 109, PERF_REG_POWERPC_CTR },
> +};
> +
> +bool libdw__arch_set_initial_registers(Dwfl_Thread *thread, void *arg)
> +{
> +	struct unwind_info *ui =3D arg;
> +	struct regs_dump *user_regs =3D &ui->sample->user_regs;
> +	Dwarf_Word dwarf_regs[32], dwarf_nip;
> +	size_t i;
> +
> +#define REG(r) ({						\
> +	Dwarf_Word val =3D 0;					\
> +	perf_reg_value(&val, user_regs, PERF_REG_POWERPC_##r);	\
> +	val;							\
> +})
> +
> +	dwarf_regs[0]  =3D REG(R0);
> +	dwarf_regs[1]  =3D REG(R1);
> +	dwarf_regs[2]  =3D REG(R2);
> +	dwarf_regs[3]  =3D REG(R3);
> +	dwarf_regs[4]  =3D REG(R4);
> +	dwarf_regs[5]  =3D REG(R5);
> +	dwarf_regs[6]  =3D REG(R6);
> +	dwarf_regs[7]  =3D REG(R7);
> +	dwarf_regs[8]  =3D REG(R8);
> +	dwarf_regs[9]  =3D REG(R9);
> +	dwarf_regs[10] =3D REG(R10);
> +	dwarf_regs[11] =3D REG(R11);
> +	dwarf_regs[12] =3D REG(R12);
> +	dwarf_regs[13] =3D REG(R13);
> +	dwarf_regs[14] =3D REG(R14);
> +	dwarf_regs[15] =3D REG(R15);
> +	dwarf_regs[16] =3D REG(R16);
> +	dwarf_regs[17] =3D REG(R17);
> +	dwarf_regs[18] =3D REG(R18);
> +	dwarf_regs[19] =3D REG(R19);
> +	dwarf_regs[20] =3D REG(R20);
> +	dwarf_regs[21] =3D REG(R21);
> +	dwarf_regs[22] =3D REG(R22);
> +	dwarf_regs[23] =3D REG(R23);
> +	dwarf_regs[24] =3D REG(R24);
> +	dwarf_regs[25] =3D REG(R25);
> +	dwarf_regs[26] =3D REG(R26);
> +	dwarf_regs[27] =3D REG(R27);
> +	dwarf_regs[28] =3D REG(R28);
> +	dwarf_regs[29] =3D REG(R29);
> +	dwarf_regs[30] =3D REG(R30);
> +	dwarf_regs[31] =3D REG(R31);
> +	if (!dwfl_thread_state_registers(thread, 0, 32, dwarf_regs))
> +		return false;
> +
> +	dwarf_nip =3D REG(NIP);
> +	dwfl_thread_state_register_pc(thread, dwarf_nip);
> +	for (i =3D 0; i < ARRAY_SIZE(special_regs); i++) {
> +		Dwarf_Word val =3D 0;
> +		perf_reg_value(&val, user_regs, special_regs[i][1]);
> +		if (!dwfl_thread_state_registers(thread,
> +						 special_regs[i][0], 1,
> +						 &val))
> +			return false;
> +	}
> +
> +	return true;
> +}
> --=20
> 1.8.3.1
>=20
>=20
>=20
=

Re: [PATCH] perf: libdw support for powerpc

[Paolo Bonzini]

On 18/05/2017 20:19, Naveen N. Rao wrote:
> Paolo Bonzini wrote:
>> The ARM and x86 architectures already use libdw, and it is useful to
>> have as much common code for the unwinder as possible.  Porting PPC
>> to libdw only needs an architecture-specific hook to move the register
>> state from perf to libdw.
> 
> Thanks. Ravi has had a similar patch locally, but from what I
> understand, there are issues with libdw based unwinding on powerpc64. I
> gave this a quick test and I don't see the user-space call trace being
> unwinded properly with libdw.

I don't see that problem:

-   99,98%     0,00%  dd       libc-2.17.so                    [.] generic_start_main.isra.0
     generic_start_main.isra.0
   - main
      - 99,97% iread
         - 97,82% sys_read
            - 96,97% extract_entropy_user
                 89,44% powernv_get_random_long
                 4,63% sha_transform
                 2,07% extract_buf
           1,15% _raw_spin_lock_irqsave
           0,51% extract_buf

(This is "perf record dd if=/dev/urandom of=/dev/null bs=512").

Can you copy the contents of tools/perf/.config-detected here?

>> diff --git a/tools/perf/arch/powerpc/util/unwind-libdw.c
>> b/tools/perf/arch/powerpc/util/unwind-libdw.c
>> new file mode 100644
>> index 0000000..c61e1d8
>> --- /dev/null
>> +++ b/tools/perf/arch/powerpc/util/unwind-libdw.c
>> @@ -0,0 +1,73 @@
>> +#include <elfutils/libdwfl.h>
>> +#include "../../util/util.h"
>> +#include "../../util/unwind-libdw.h"
>> +#include "../../util/perf_regs.h"
> 
> As an aside, we also need to include "event.h", without which there are
> a few compile errors.

Oops, I was compiling this on 4.11.  Sorry.

Paolo

Re: [PATCH] perf: libdw support for powerpc

[Paolo Bonzini]

On 18/05/2017 20:48, Paolo Bonzini wrote:
> 
> 
> On 18/05/2017 20:19, Naveen N. Rao wrote:
>> Paolo Bonzini wrote:
>>> The ARM and x86 architectures already use libdw, and it is useful to
>>> have as much common code for the unwinder as possible.  Porting PPC
>>> to libdw only needs an architecture-specific hook to move the register
>>> state from perf to libdw.
>>
>> Thanks. Ravi has had a similar patch locally, but from what I
>> understand, there are issues with libdw based unwinding on powerpc64. I
>> gave this a quick test and I don't see the user-space call trace being
>> unwinded properly with libdw.
> 
> I don't see that problem:
> 
> -   99,98%     0,00%  dd       libc-2.17.so                    [.] generic_start_main.isra.0
>      generic_start_main.isra.0
>    - main
>       - 99,97% iread
>          - 97,82% sys_read
>             - 96,97% extract_entropy_user
>                  89,44% powernv_get_random_long
>                  4,63% sha_transform
>                  2,07% extract_buf
>            1,15% _raw_spin_lock_irqsave
>            0,51% extract_buf
> 
> (This is "perf record dd if=/dev/urandom of=/dev/null bs=512").
> 
> Can you copy the contents of tools/perf/.config-detected here?

FWIW it works fine for me here with latest Linus tree and your suggested
change to fix compilation:

--- a/tools/perf/arch/powerpc/util/unwind-libdw.c
+++ b/tools/perf/arch/powerpc/util/unwind-libdw.c
@@ -1,7 +1,7 @@
 #include <elfutils/libdwfl.h>
-#include "../../util/util.h"
 #include "../../util/unwind-libdw.h"
 #include "../../util/perf_regs.h"
+#include "../../util/event.h"

 /* See backends/ppc_initreg.c and backends/ppc_regs.c in elfutils.  */
 static const int special_regs[3][2] = {

Thanks,

Paolo