On Fri, 8 May 2026 09:46:30 +0200 Jens Remus <[email protected]> wrote:
> > STACKTRACE_REGISTER_SFRAME - This registers the sframe > > STACKTRACE_UNREGISTER_SFRAME - This removes the sframe > > > > Signed-off-by: Steven Rostedt <[email protected]> > > LGTM. Some comments/questions below. Note, after talking with people at LSF/MM/BPF, I plan on completely changing this system call into two distinct ones, and only for sframes. I'll be sending that later this week. > > > diff --git a/include/uapi/linux/stacktrace.h > > b/include/uapi/linux/stacktrace.h > > > @@ -0,0 +1,10 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */ > > +#ifndef _UAPI_LINUX_STACKTRACE_H > > +#define _UAPI_LINUX_STACKTRACE_H > > + > > +enum stacktrace_setup_types { > > + STACKTRACE_REGISTER_SFRAME = 1, > > + STACKTRACE_UNREGISTER_SFRAME = 2, > > +}; > > + > > +#endif /* _UAPI_LINUX_STACKTRACE_H */ > > > diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c > > Having the syscall live in kernel/unwind/sframe.c means it is only > available if config option HAVE_UNWIND_USER_SFRAME is selected (which > triggers sframe.o to be built and linked into the kernel), which makes > sense as long as it only implements sframe-specific functionality. > I suppose it could be moved elsewhere if non-sframe use cases would > arise in the future? The new system calls will only be for sframes. Other unwinders will need to implement their own system calls. > > Would Dylan need to guard it when introducing HAVE_UNWIND_KERNEL_SFRAME? > Provided the syscall fails with -ENOSYS if not implemented (e.g. when > HAVE_UNWIND_USER_SFRAME is not enabled) the dummy implementations of > sframe_add_section() and sframe_remove_section() in linux/sframe.h also > return -ENOSYS, so the user observable behavior would be the same and > it would not matter. Do you agree? I'll reply to that when Dylan's patches get closer to acceptance ;-) > > > @@ -12,8 +12,10 @@ > > #include <linux/mm.h> > > #include <linux/string_helpers.h> > > #include <linux/sframe.h> > > +#include <linux/syscalls.h> > > #include <asm/unwind_user_sframe.h> > > #include <linux/unwind_user_types.h> > > +#include <uapi/linux/stacktrace.h> > > > > #include "sframe.h" > > #include "sframe_debug.h" > > @@ -838,3 +840,38 @@ void sframe_free_mm(struct mm_struct *mm) > > > > mtree_destroy(&mm->sframe_mt); > > } > > + > > +/** > > + * sys_stacktrace_setup - register an address for user space stacktrace > > walking. > > + * @op: Type of operation to perform > > + * @addr_start: The virtual address of the stacktrace information > > + * @addr_length: The length of the stacktrace information > > + * @text_start: The virtual address of the text that @addr_start represents > > + * @text_length: The length of teh text > > + * > > + * This system call is used by dynamic library utilities to inform the > > kernel > > + * of meta data that it loaded that can be used by the kernel to know how > > + * to stack walk the given text locations. > > + * > > + * Currently only sframes are supported, but in the future, this may be > > used > > + * to tell the kernel about JIT code which will most likely have a > > different > > + * format. > > + * > > + * The type command may be extended and parameters may be used for other > > + * purposes. > > + * > > + * Return: 0 if successful, otherwise a negative error. > > + */ > > +SYSCALL_DEFINE5(stacktrace_setup, int, op, unsigned long, addr_start, > > + unsigned long, addr_length, unsigned long, text_start, > > + unsigned long, text_length) > > Would it make sense to keep the parameters generic from start, similar > to how it is done in prctl()? Or can this be changed later, if the need > arises? With discussions at LSF/MM/BPF I'll have the system call parameters be a pointer to a structure, and a size of that structure. All the API will then be part of the structure. Thanks for reviewing, -- Steve > > SYSCALL_DEFINE5(stacktrace_setup, int, op, unsigned long, arg2, > unsigned long, arg3, unsigned long, arg4, unsigned long, arg5) > > > +{ > > + switch (op) { > > + case STACKTRACE_REGISTER_SFRAME: > > + return sframe_add_section(addr_start, addr_start + addr_length, > > + text_start, text_start+text_length); > > Nit: > text_start, text_start + text_length); > > > + case STACKTRACE_UNREGISTER_SFRAME: > > + return sframe_remove_section(addr_start); > > + } > > + return -EINVAL; > > +} > Thanks and regards, > Jens
