Re: [PATCH v8 06/12] unwind_user/sframe: Wire up unwind_user to sframe

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2025-07-07 22:11, Steven Rostedt wrote:
> From: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> Now that the sframe infrastructure is fully in place, make it work by
> hooking it up to the unwind_user interface.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>   arch/Kconfig                      |  1 +
>   include/linux/unwind_user_types.h |  1 +
>   kernel/unwind/user.c              | 25 ++++++++++++++++++++++---
>   3 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index c54d35e2f860..0c6056ef13de 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -448,6 +448,7 @@ config HAVE_UNWIND_USER_COMPAT_FP
>   
>   config HAVE_UNWIND_USER_SFRAME
>   	bool
> +	select UNWIND_USER
>   
>   config HAVE_PERF_REGS
>   	bool
> diff --git a/include/linux/unwind_user_types.h b/include/linux/unwind_user_types.h
> index 0b6563951ca4..4d50476e950e 100644
> --- a/include/linux/unwind_user_types.h
> +++ b/include/linux/unwind_user_types.h
> @@ -13,6 +13,7 @@ enum unwind_user_type {
>   	UNWIND_USER_TYPE_NONE,
>   	UNWIND_USER_TYPE_FP,
>   	UNWIND_USER_TYPE_COMPAT_FP,
> +	UNWIND_USER_TYPE_SFRAME,
>   };
>   
>   struct unwind_stacktrace {
> diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
> index 249d9e32fad7..6e7ca9f1293a 100644
> --- a/kernel/unwind/user.c
> +++ b/kernel/unwind/user.c
> @@ -7,6 +7,7 @@
>   #include <linux/sched/task_stack.h>
>   #include <linux/unwind_user.h>
>   #include <linux/uaccess.h>
> +#include <linux/sframe.h>
>   
>   static struct unwind_user_frame fp_frame = {
>   	ARCH_INIT_USER_FP_FRAME
> @@ -31,6 +32,12 @@ static inline bool compat_fp_state(struct unwind_user_state *state)
>   	       state->type == UNWIND_USER_TYPE_COMPAT_FP;
>   }
>   
> +static inline bool sframe_state(struct unwind_user_state *state)
> +{
> +	return IS_ENABLED(CONFIG_HAVE_UNWIND_USER_SFRAME) &&
> +	       state->type == UNWIND_USER_TYPE_SFRAME;
> +}
> +
>   #define unwind_get_user_long(to, from, state)				\
>   ({									\
>   	int __ret;							\
> @@ -44,18 +51,28 @@ static inline bool compat_fp_state(struct unwind_user_state *state)
>   static int unwind_user_next(struct unwind_user_state *state)
>   {
>   	struct unwind_user_frame *frame;
> +	struct unwind_user_frame _frame;
>   	unsigned long cfa = 0, fp, ra = 0;
>   	unsigned int shift;
>   
>   	if (state->done)
>   		return -EINVAL;
>   
> -	if (compat_fp_state(state))
> +	if (compat_fp_state(state)) {
>   		frame = &compat_fp_frame;
> -	else if (fp_state(state))
> +	} else if (sframe_state(state)) {
> +		/* sframe expects the frame to be local storage */
> +		frame = &_frame;
> +		if (sframe_find(state->ip, frame)) {
> +			if (!IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP))
> +				goto done;
> +			frame = &fp_frame;
> +		}
> +	} else if (fp_state(state)) {
>   		frame = &fp_frame;
> -	else
> +	} else {
>   		goto done;
> +	}
>   
>   	if (frame->use_fp) {
>   		if (state->fp < state->sp)
> @@ -111,6 +128,8 @@ static int unwind_user_start(struct unwind_user_state *state)
>   
>   	if (IS_ENABLED(CONFIG_HAVE_UNWIND_USER_COMPAT_FP) && in_compat_mode(regs))
>   		state->type = UNWIND_USER_TYPE_COMPAT_FP;
> +	else if (current_has_sframe())
> +		state->type = UNWIND_USER_TYPE_SFRAME;

I think you'll want to update the state->type during the
traversal (in next()), because depending on whether
sframe is available for a given memory area of code
or not, the next() function can use either frame pointers
or sframe during the same traversal. It would be good
to know which is used after each specific call to next().

Thanks,

Mathieu

>   	else if (IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP))
>   		state->type = UNWIND_USER_TYPE_FP;
>   	else


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com