Re: [RFC][PATCH] unwind: Add stacktrace_setup system call

[Steven Rostedt <rostedt@kernel.org>]

On Fri, 8 May 2026 09:46:30 +0200
Jens Remus <jremus@linux.ibm.com> wrote:

> >   STACKTRACE_REGISTER_SFRAME - This registers the sframe
> >   STACKTRACE_UNREGISTER_SFRAME - This removes the sframe
> > 
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>  
> 
> 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