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.
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Indu Bhagat <indu.bhagat@oracle.com>
Cc: "Jose E. Marchesi" <jemarch@gnu.org>
Cc: Beau Belgrave <beaub@linux.microsoft.com>
Cc: Jens Remus <jremus@linux.ibm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Sam James <sam@gentoo.org>
Cc: Kees Cook <kees@kernel.org>
Cc: "Carlos O'Donell" <codonell@redhat.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
arch/Kconfig | 1 +
include/linux/unwind_user_types.h | 4 ++-
kernel/unwind/user.c | 41 +++++++++++++++++++++++++++----
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 69fcabf53088..277b87af949f 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -453,6 +453,7 @@ config HAVE_UNWIND_USER_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 938f7e623332..ee0ce855e045 100644
--- a/include/linux/unwind_user_types.h
+++ b/include/linux/unwind_user_types.h
@@ -9,7 +9,8 @@
* available.
*/
enum unwind_user_type_bits {
- UNWIND_USER_TYPE_FP_BIT = 0,
+ UNWIND_USER_TYPE_SFRAME_BIT = 0,
+ UNWIND_USER_TYPE_FP_BIT = 1,
NR_UNWIND_USER_TYPE_BITS,
};
@@ -17,6 +18,7 @@ enum unwind_user_type_bits {
enum unwind_user_type {
/* Type "none" for the start of stack walk iteration. */
UNWIND_USER_TYPE_NONE = 0,
+ UNWIND_USER_TYPE_SFRAME = BIT(UNWIND_USER_TYPE_SFRAME_BIT),
UNWIND_USER_TYPE_FP = BIT(UNWIND_USER_TYPE_FP_BIT),
};
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index 696004ee956a..f6c543cb255b 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>
#define for_each_user_frame(state) \
for (unwind_user_start(state); !(state)->done; unwind_user_next(state))
@@ -26,12 +27,10 @@ get_user_word(unsigned long *word, unsigned long base, int off, unsigned int ws)
return get_user(*word, addr);
}
-static int unwind_user_next_fp(struct unwind_user_state *state)
+static int unwind_user_next_common(struct unwind_user_state *state,
+ const struct unwind_user_frame *frame,
+ struct pt_regs *regs)
{
- const struct unwind_user_frame fp_frame = {
- ARCH_INIT_USER_FP_FRAME(state->ws)
- };
- const struct unwind_user_frame *frame = &fp_frame;
unsigned long cfa, fp, ra;
if (frame->use_fp) {
@@ -67,6 +66,26 @@ static int unwind_user_next_fp(struct unwind_user_state *state)
return 0;
}
+static int unwind_user_next_sframe(struct unwind_user_state *state)
+{
+ struct unwind_user_frame _frame, *frame;
+
+ /* sframe expects the frame to be local storage */
+ frame = &_frame;
+ if (sframe_find(state->ip, frame))
+ return -ENOENT;
+ return unwind_user_next_common(state, frame, task_pt_regs(current));
+}
+
+static int unwind_user_next_fp(struct unwind_user_state *state)
+{
+ const struct unwind_user_frame fp_frame = {
+ ARCH_INIT_USER_FP_FRAME(state->ws)
+ };
+
+ return unwind_user_next_common(state, &fp_frame, task_pt_regs(current));
+}
+
static int unwind_user_next(struct unwind_user_state *state)
{
unsigned long iter_mask = state->available_types;
@@ -80,6 +99,16 @@ static int unwind_user_next(struct unwind_user_state *state)
state->current_type = type;
switch (type) {
+ case UNWIND_USER_TYPE_SFRAME:
+ switch (unwind_user_next_sframe(state)) {
+ case 0:
+ return 0;
+ case -ENOENT:
+ continue; /* Try next method. */
+ default:
+ state->done = true;
+ }
+ break;
case UNWIND_USER_TYPE_FP:
if (!unwind_user_next_fp(state))
return 0;
@@ -108,6 +137,8 @@ static int unwind_user_start(struct unwind_user_state *state)
return -EINVAL;
}
+ if (current_has_sframe())
+ state->available_types |= UNWIND_USER_TYPE_SFRAME;
if (IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP))
state->available_types |= UNWIND_USER_TYPE_FP;
--
2.48.1On Wed, Oct 22, 2025 at 04:43:19PM +0200, Jens Remus wrote:
> @@ -26,12 +27,10 @@ get_user_word(unsigned long *word, unsigned long base, int off, unsigned int ws)
> return get_user(*word, addr);
> }
>
> -static int unwind_user_next_fp(struct unwind_user_state *state)
> +static int unwind_user_next_common(struct unwind_user_state *state,
> + const struct unwind_user_frame *frame,
> + struct pt_regs *regs)
> {
What is pt_regs for? AFAICT it isn't actually used in any of the
following patches.
> - const struct unwind_user_frame fp_frame = {
> - ARCH_INIT_USER_FP_FRAME(state->ws)
> - };
> - const struct unwind_user_frame *frame = &fp_frame;
> unsigned long cfa, fp, ra;
>
> if (frame->use_fp) {
> @@ -67,6 +66,26 @@ static int unwind_user_next_fp(struct unwind_user_state *state)
> return 0;
> }
>
> +static int unwind_user_next_sframe(struct unwind_user_state *state)
> +{
> + struct unwind_user_frame _frame, *frame;
> +
> + /* sframe expects the frame to be local storage */
> + frame = &_frame;
> + if (sframe_find(state->ip, frame))
> + return -ENOENT;
> + return unwind_user_next_common(state, frame, task_pt_regs(current));
> +}
Would it not be simpler to write:
static int unwind_user_next_sframe(struct unwind_user_state *state)
{
struct unwind_user_frame frame;
/* sframe expects the frame to be local storage */
if (sframe_find(state->ip, &frame))
return -ENOENT;
return unwind_user_next_common(state, &frame, task_pt_regs(current));
}
hmm?
> +static int unwind_user_next_fp(struct unwind_user_state *state)
> +{
> + const struct unwind_user_frame fp_frame = {
> + ARCH_INIT_USER_FP_FRAME(state->ws)
> + };
> +
> + return unwind_user_next_common(state, &fp_frame, task_pt_regs(current));
> +}
> +
> static int unwind_user_next(struct unwind_user_state *state)
> {
> unsigned long iter_mask = state->available_types;
> @@ -80,6 +99,16 @@ static int unwind_user_next(struct unwind_user_state *state)
>
> state->current_type = type;
> switch (type) {
> + case UNWIND_USER_TYPE_SFRAME:
> + switch (unwind_user_next_sframe(state)) {
> + case 0:
> + return 0;
> + case -ENOENT:
> + continue; /* Try next method. */
> + default:
> + state->done = true;
> + }
> + break;
Should it remove SFRAME from state->available_types at this point?
On 10/24/2025 3:44 PM, Peter Zijlstra wrote:
> On Wed, Oct 22, 2025 at 04:43:19PM +0200, Jens Remus wrote:
>
>> @@ -26,12 +27,10 @@ get_user_word(unsigned long *word, unsigned long base, int off, unsigned int ws)
>> return get_user(*word, addr);
>> }
>>
>> -static int unwind_user_next_fp(struct unwind_user_state *state)
>> +static int unwind_user_next_common(struct unwind_user_state *state,
>> + const struct unwind_user_frame *frame,
>> + struct pt_regs *regs)
>> {
>
> What is pt_regs for? AFAICT it isn't actually used in any of the
> following patches.
Good catch! No idea. It started to appear in v9 of the series:
[PATCH v8 06/12] unwind_user/sframe: Wire up unwind_user to sframe
https://lore.kernel.org/all/20250708021159.386608979@kernel.org/
[PATCH v9 06/11] unwind_user/sframe: Wire up unwind_user to sframe
https://lore.kernel.org/all/20250717012936.619600891@kernel.org/
My s390 support for unwind user sframe will make use of it, but it
should better be introduced there then.
@Steven: Any idea why you added pt_regs? Your v9 even had this other
instance of unused pt_regs:
+static struct unwind_user_frame *get_fp_frame(struct pt_regs *regs)
+{
+ return &fp_frame;
+}
>> @@ -67,6 +66,26 @@ static int unwind_user_next_fp(struct unwind_user_state *state)
>> return 0;
>> }
>>
>> +static int unwind_user_next_sframe(struct unwind_user_state *state)
>> +{
>> + struct unwind_user_frame _frame, *frame;
>> +
>> + /* sframe expects the frame to be local storage */
>> + frame = &_frame;
>> + if (sframe_find(state->ip, frame))
>> + return -ENOENT;
>> + return unwind_user_next_common(state, frame, task_pt_regs(current));
>> +}
>
> Would it not be simpler to write:
>
> static int unwind_user_next_sframe(struct unwind_user_state *state)
> {
> struct unwind_user_frame frame;
>
> /* sframe expects the frame to be local storage */
> if (sframe_find(state->ip, &frame))
> return -ENOENT;
> return unwind_user_next_common(state, &frame, task_pt_regs(current));
> }
>
> hmm?
I agree. Must have been a leftover from changes from v8 to v9.
>> @@ -80,6 +99,16 @@ static int unwind_user_next(struct unwind_user_state *state)
>>
>> state->current_type = type;
>> switch (type) {
>> + case UNWIND_USER_TYPE_SFRAME:
>> + switch (unwind_user_next_sframe(state)) {
>> + case 0:
>> + return 0;
>> + case -ENOENT:
>> + continue; /* Try next method. */
>> + default:
>> + state->done = true;
>> + }
>> + break;
>
> Should it remove SFRAME from state->available_types at this point?
In the -ENOENT case? If the reason is that there was either no SFrame
section or no SFrame information (SFrame FRE) for the IP, then SFRAME
could potentially be successful with the next IP in the call chain.
Provided the other unwind methods do correctly unwind both SP and FP.
@Steven: What is your opinion on this?
Thanks and regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com
IBM
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
On Fri, 24 Oct 2025 16:29:07 +0200
Jens Remus <jremus@linux.ibm.com> wrote:
> @Steven: Any idea why you added pt_regs? Your v9 even had this other
> instance of unused pt_regs:
>
> +static struct unwind_user_frame *get_fp_frame(struct pt_regs *regs)
> +{
> + return &fp_frame;
> +}
According to the history:
https://lore.kernel.org/linux-trace-kernel/20250717012848.927473176@kernel.org/
Which has:
Changes since v8: https://lore.kernel.org/linux-trace-kernel/20250708021115.894007410@kernel.org/
- Rebased on the changes by Mathieu in the kernel/unwind/user.c file
https://lore.kernel.org/all/20250710164301.3094-2-mathieu.desnoyers@efficios.com/
It looks like it came in from Mathieu's updates, which was trying to deal
with compat. But then after noticing that compat wasn't working on my tests
boxes, I removed it. The removal failed to notice that regs is now unused.
-- Steve
© 2016 - 2026 Red Hat, Inc.