[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20210824013834.6bnpn4vdyynvwqme@kashmir.localdomain>
Date: Mon, 23 Aug 2021 18:38:34 -0700
From: Daniel Xu <dxu@...uu.xyz>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: bpf <bpf@...r.kernel.org>, Yonghong Song <yhs@...com>,
Andrii Nakryiko <andriin@...com>,
Kernel Team <kernel-team@...com>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH bpf-next 1/2] bpf: Add bpf_task_pt_regs() helper
On Thu, Aug 19, 2021 at 01:27:16PM -0700, Andrii Nakryiko wrote:
> On Wed, Aug 18, 2021 at 4:42 PM Daniel Xu <dxu@...uu.xyz> wrote:
> >
> > The motivation behind this helper is to access userspace pt_regs in a
> > kprobe handler.
> >
> > uprobe's ctx is the userspace pt_regs. kprobe's ctx is the kernelspace
> > pt_regs. bpf_task_pt_regs() allows accessing userspace pt_regs in a
> > kprobe handler. The final case (kernelspace pt_regs in uprobe) is
> > pretty rare (usermode helper) so I think that can be solved later if
> > necessary.
> >
> > More concretely, this helper is useful in doing BPF-based DWARF stack
> > unwinding. Currently the kernel can only do framepointer based stack
> > unwinds for userspace code. This is because the DWARF state machines are
> > too fragile to be computed in kernelspace [0]. The idea behind
> > DWARF-based stack unwinds w/ BPF is to copy a chunk of the userspace
> > stack (while in prog context) and send it up to userspace for unwinding
> > (probably with libunwind) [1]. This would effectively enable profiling
> > applications with -fomit-frame-pointer using kprobes and uprobes.
> >
> > [0]: https://lkml.org/lkml/2012/2/10/356
> > [1]: https://github.com/danobi/bpf-dwarf-walk
> >
> > Signed-off-by: Daniel Xu <dxu@...uu.xyz>
> > ---
>
> Seems like a really useful thing. Few notes:
>
> 1. Given this is user pt_regs, should we call it bpf_get_user_pt_regs()?
I'm not 100% sure, but it seems to me that task_pt_regs() works for
kernel threads too. I see in arch/x86/kernel/smpboot.c that
task_pt_regs() is being used on the idle thread (which I think is a
kernel thread).
> 2. Would it be safe to enable it for all types of programs, not just
> kprobe/tp/raw_tp/perf? Why limit the list?
Oh I didn't realize I put a limit on it. I'll look closer.
> 3. It seems like it's the sixth declaration of BTF_ID for task_struct,
> maybe it's time to consolidate them?
Ok, will consolidate.
[...]
Thanks,
Daniel
Powered by blists - more mailing lists