On Fri, Mar 05, 2021 at 07:58:09PM +0900, Masami Hiramatsu wrote:
> On Fri, 5 Mar 2021 18:28:06 +0900
> Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
>
> > Hi Daniel,
> >
> > On Thu, 4 Mar 2021 16:07:52 -0800
> > Daniel Xu <dxu@xxxxxxxxx> wrote:
> >
> > > Getting a stack trace from inside a kretprobe used to work with frame
> > > pointer stack walks. After the default unwinder was switched to ORC,
> > > stack traces broke because ORC did not know how to skip the
> > > `kretprobe_trampoline` "frame".
> > >
> > > Frame based stack walks used to work with kretprobes because
> > > `kretprobe_trampoline` does not set up a new call frame. Thus, the frame
> > > pointer based unwinder could walk directly to the kretprobe'd caller.
> > >
> > > For example, this stack is walked incorrectly with ORC + kretprobe:
> > >
> > > # bpftrace -e 'kretprobe:do_nanosleep { @[kstack] = count() }'
> > > Attaching 1 probe...
> > > ^C
> > >
> > > @[
> > > kretprobe_trampoline+0
> > > ]: 1
> > >
> > > After this patch, the stack is walked correctly:
> > >
> > > # bpftrace -e 'kretprobe:do_nanosleep { @[kstack] = count() }'
> > > Attaching 1 probe...
> > > ^C
> > >
> > > @[
> > > kretprobe_trampoline+0
> > > __x64_sys_nanosleep+150
> > > do_syscall_64+51
> > > entry_SYSCALL_64_after_hwframe+68
> > > ]: 12
> > >
> > > Fixes: fc72ae40e303 ("x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit")
> > > Signed-off-by: Daniel Xu <dxu@xxxxxxxxx>
> >
> > OK, basically good, but this is messy, and doing much more than fixing issue.
Thanks for taking a look!
> BTW, is this a regression? or CONFIG_UNWINDER_ORC has this issue before?
> It seems that the above commit just changed the default unwinder. This means
> OCR stack unwinder has this bug before that commit.
I see your point -- I suppose it depends on point of view. Viewed from
userspace, a change in kernel defaults means that one kernel worked and
the next one didn't -- all without the user doing anything. Consider it
from the POV of a typical linux user who just takes whatever the distro
gives them and doesn't compile their own kernels.
>From the kernel point of view, you're also right. ORC didn't regress, it
was always broken for this particular use case. But as a primarily
userspace developer, I would consider this a kernel regression.
> If you choose the CONFIG_UNWINDER_FRAME_POINTER, it might work again.
Yes, I've confirmed that switching to CONFIG_UNWINDER_FRAME_POINTER does
fix the issue. But it's a non-starter for production machines b/c the
perf regression is too significant.
Thanks,
Daniel
