On Sat, Apr 06, 2019 at 01:05:55AM +0800, Kairui Song wrote:
> On Sat, Apr 6, 2019 at 12:57 AM Josh Poimboeuf <jpoim...@redhat.com> wrote:
> >
> > On Fri, Apr 05, 2019 at 11:13:02PM +0800, Kairui Song wrote:
> > > Hi Josh, thanks for the review, I tried again, using latest upstream
> > > kernel commit ea2cec24c8d429ee6f99040e4eb6c7ad627fe777:
> > > # uname -a
> > > Linux localhost.localdomain 5.1.0-rc3+ #29 SMP Fri Apr 5 22:53:05 CST
> > > 2019 x86_64 x86_64 x86_64 GNU/Linux
> > >
> > > Having following config:
> > > > CONFIG_UNWINDER_ORC=y
> > > > # CONFIG_UNWINDER_FRAME_POINTER is not set
> > > and CONFIG_FRAME_POINTER is off too.
> > >
> > > Then record something with perf (also latest upstream version):
> > > ./perf record -g -e kmem:* -c 1
> > >
> > > Interrupt it, then view the output:
> > > perf script | less
> > >
> > > Then I notice the stacktrace in kernle is incomplete like following.
> > > Did I miss anything?
> > > --------------
> > > lvmetad 617 [000] 55.600786: kmem:kfree:
> > > call_site=ffffffffb219e269 ptr=(nil)
> > > ffffffffb22b2d1c kfree+0x11c
> > > (/lib/modules/5.1.0-rc3+/build/vmlinux)
> > > 7fba7e58fd0f __select+0x5f (/usr/lib64/libc-2.28.so)
> > >
> > > kworker/u2:5-rp 171 [000] 55.628529:
> > > kmem:kmem_cache_alloc: call_site=ffffffffb20e963d
> > > ptr=0xffffa07f39c581e0 bytes_req=80 bytes_alloc=80
> > > gfp_flags=GFP_ATOMIC
> > > ffffffffb22b0dec kmem_cache_alloc+0x13c
> > > (/lib/modules/5.1.0-rc3+/build/vmlinux)
> > > -------------
> > >
> > > And for the patch, I debugged the problem, and found how it happend:
> > > The reason is that we use following code for fetching the registers on
> > > a trace point:
> > > ...snip...
> > > #define perf_arch_fetch_caller_regs(regs, __ip) { \
> > > (regs)->ip = (__ip); \
> > > (regs)->bp = caller_frame_pointer(); \
> > > (regs)->cs = __KERNEL_CS;
> > > ...snip...
> >
> > Thanks, I was able to recreate. It only happens when unwinding from a
> > tracepoint. I haven't investigated yet, but
> > perf_arch_fetch_caller_regs() looks highly suspect, since it's doing
> > (regs)->bp = caller_frame_pointer(), even for ORC.
> >
> > My only explanation for how your patch works is that RBP just happens to
> > point to somewhere higher on the stack, causing the unwinder to start at
> > a semi-random location. I suspect the real "fix" is that you're no
> > longer passing the regs to unwind_start().
> >
>
> Yes that's right. Simply not passing regs to unwind_start will let the
> unwind start from the perf sample handling functions, and introduce a
> lot of "noise", so I let it skipped the frames until it reached the
> frame of the trace point. The regs->bp should still points to the
> stack base of the function which get called in the tracepoint that
> trigger perf sample, so let unwinder skip all the frames above it made
> it work.
Ah, now I think I understand, thanks. perf_arch_fetch_caller_regs()
puts it in regs->bp, and then perf_callchain_kernel() reads that value
to tell the unwinder where to start dumping the stack trace. I guess
that explains why your patch works, though it still seems very odd that
perf_arch_fetch_caller_regs() is using regs->bp to store the frame
address. Maybe regs->sp would be more appropriate.
--
Josh
