Re: [PATCH v1]: perf/x86: store user space frame-pointer value on a sample

[Alexey Budankov <alexey.budankov@linux.intel.com>]

On 07.04.2018 9:18, Alexey Budankov wrote:
> On 06.04.2018 22:53, Andi Kleen wrote:
>> On Fri, Apr 06, 2018 at 10:06:26PM +0300, Alexey Budankov wrote:
>>> On 06.04.2018 18:31, Andi Kleen wrote:
>>>>> diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
>>>>> index e47b2dbbdef3..9284048cf5b0 100644
>>>>> --- a/arch/x86/kernel/perf_regs.c
>>>>> +++ b/arch/x86/kernel/perf_regs.c
>>>>> @@ -157,6 +157,15 @@ void perf_get_regs_user(struct perf_regs *regs_user,
>>>>>  	 */
>>>>>  	regs_user_copy->bx = -1;
>>>>>  	regs_user_copy->bp = -1;
>>>>> +	if (user_64bit_mode(user_regs)) {
>>>>
>>>> Why is it 64bit only? Should work on 32bit too.
>>>
>>> bp register is a part of i386 syscall ABI 
>>> (http://man7.org/linux/man-pages/man2/syscall.2.html) 
>>> so not sure if it will make any sense for 32bit processes. 
>>
>> Both 32bit and 64bit use the same frame pointer, if they
>> use frame pointer.
> 
> Well let me check the same scenario for 32bit binary.

Here is what I have when profiling 32bit process on the patched 64bit 
kernel w/o 32bit frame-pointer exposure:

vmlinux ! try_to_wake_up - [unknown source file]
vmlinux ! wake_up_q + 0x3e - [unknown source file]
vmlinux ! futex_wake + 0x141 - [unknown source file]
vmlinux ! do_futex + 0x49b - [unknown source file]
vmlinux ! compat_SyS_futex + 0x123 - [unknown source file]
vmlinux ! do_fast_syscall_32 + 0xb9 - [unknown source file]
vmlinux ! entry_SYSENTER_compat + 0x7e - [unknown source file]
==> [vdso] ! __kernel_vsyscall + 0x8 - [unknown source file]
==> libc-2.26.so ! syscall + 0x26 - [unknown source file]
==> futex32-fp ! main + 0xba - [unknown source file]
==> libc-2.26.so ! __libc_start_main + 0xf2 - [unknown source file]

so stack is unwound till the top. However if I enable 32bit exposure 
then the stack looks like this:

vmlinux ! try_to_wake_up - [unknown source file]
vmlinux ! wake_up_q + 0x3e - [unknown source file]
vmlinux ! futex_wake + 0x141 - [unknown source file]
vmlinux ! do_futex + 0x49b - [unknown source file]
vmlinux ! compat_SyS_futex + 0x123 - [unknown source file]
vmlinux ! do_fast_syscall_32 + 0xb9 - [unknown source file]
vmlinux ! entry_SYSENTER_compat + 0x7e - [unknown source file]
==> [vdso] ! [vdso] + 0x1058 - [unknown source file]
==> vmlinux ! [Skipped stack frame(s)] + 0x1 - [unknown source file]

and x86_64 perf report --stdio shows this:

...
unwind: target platform=x86 is not supported
...
# Samples: 140K of event 'cycles'
# Event count (approx.): 93688193797
#
# Children      Self  Command     Shared Object     Symbol                                        
# ........  ........  ..........  ................  .........................
#
    86.00%    14.40%  futex32-fp  [kernel.vmlinux]  [k] entry_SYSENTER_compat
            |
            ---entry_SYSENTER_compat
               |          
                --71.60%--do_fast_syscall_32
                          |          
                          |--54.62%--compat_sys_futex
                          |          |          
                          |           --53.67%--do_futex

I am not sure it is worth exposing frame pointer for 32bit too.

-Alexey

> If the issue exists for it too and is fixed by the exposing bp
> then it is obviously worth this improvement.
> 
> -Alexey
> 
>>
>> -Andi
>>
> 
>