Re: [PATCH v4 12/39] unwind_user: Add frame pointer support
On Fri, 24 Jan 2025 10:16:10 -0800 Josh Poimboeuf wrote: > > Do you plan to reuse this logic for stack unwinding done by perf > > subsystem in perf_callchain_user()? See is_uprobe_at_func_entry() > > parts and also fixup_uretprobe_trampoline_entries() for some of the > > quirks that have to be taken into account when doing frame > > pointer-based unwinding. It would be great not to lose those in this > > new reimplementation. > > > > Not sure what's the best way to avoid duplicating the logic, but I > > thought I'd bring that up. > > Indeed! That was on the todo list and somehow evaporated. I'm getting ready to post an update of these patches, but I want to mention that this has not been addressed, and I'm replying here to make sure it stays on the radar. -- Steve
Re: [PATCH v4 12/39] unwind_user: Add frame pointer support
On Fri, Jan 24, 2025 at 09:59:37AM -0800, Andrii Nakryiko wrote: > On Tue, Jan 21, 2025 at 6:32 PM Josh Poimboeuf wrote: > > > > Add optional support for user space frame pointer unwinding. If > > supported, the arch needs to enable CONFIG_HAVE_UNWIND_USER_FP and > > define ARCH_INIT_USER_FP_FRAME. > > > > By encoding the frame offsets in struct unwind_user_frame, much of this > > code can also be reused for future unwinder implementations like sframe. > > > > Signed-off-by: Josh Poimboeuf > > --- > > arch/Kconfig | 4 +++ > > include/asm-generic/unwind_user.h | 9 ++ > > include/linux/unwind_user_types.h | 1 + > > kernel/unwind/user.c | 49 +-- > > 4 files changed, 60 insertions(+), 3 deletions(-) > > create mode 100644 include/asm-generic/unwind_user.h > > > > Do you plan to reuse this logic for stack unwinding done by perf > subsystem in perf_callchain_user()? See is_uprobe_at_func_entry() > parts and also fixup_uretprobe_trampoline_entries() for some of the > quirks that have to be taken into account when doing frame > pointer-based unwinding. It would be great not to lose those in this > new reimplementation. > > Not sure what's the best way to avoid duplicating the logic, but I > thought I'd bring that up. Indeed! That was on the todo list and somehow evaporated. -- Josh
Re: [PATCH v4 12/39] unwind_user: Add frame pointer support
On Tue, Jan 21, 2025 at 6:32 PM Josh Poimboeuf wrote: > > Add optional support for user space frame pointer unwinding. If > supported, the arch needs to enable CONFIG_HAVE_UNWIND_USER_FP and > define ARCH_INIT_USER_FP_FRAME. > > By encoding the frame offsets in struct unwind_user_frame, much of this > code can also be reused for future unwinder implementations like sframe. > > Signed-off-by: Josh Poimboeuf > --- > arch/Kconfig | 4 +++ > include/asm-generic/unwind_user.h | 9 ++ > include/linux/unwind_user_types.h | 1 + > kernel/unwind/user.c | 49 +-- > 4 files changed, 60 insertions(+), 3 deletions(-) > create mode 100644 include/asm-generic/unwind_user.h > Do you plan to reuse this logic for stack unwinding done by perf subsystem in perf_callchain_user()? See is_uprobe_at_func_entry() parts and also fixup_uretprobe_trampoline_entries() for some of the quirks that have to be taken into account when doing frame pointer-based unwinding. It would be great not to lose those in this new reimplementation. Not sure what's the best way to avoid duplicating the logic, but I thought I'd bring that up. > diff --git a/arch/Kconfig b/arch/Kconfig > index c6fa2b3ecbc6..cf996cbb8142 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -438,6 +438,10 @@ config HAVE_HARDLOCKUP_DETECTOR_ARCH > config UNWIND_USER > bool > > +config HAVE_UNWIND_USER_FP > + bool > + select UNWIND_USER > + > config AS_SFRAME > def_bool $(as-instr,.cfi_sections > .sframe\n.cfi_startproc\n.cfi_endproc) > [...]
