[PATCH 1/2] riscv: stacktrace: do not report the walker's own frame from arch_stack_walk()
flat view
COOLING9d
From: Karl Mehltretter <hidden>
Date: 2026-10-10 13:27:23
Also in:
linux-kselftest, linux-riscv, lkml
Subsystem:
risc-v architecture, the rest · Maintainers:
Paul Walmsley, Palmer Dabbelt, Albert Ou, Linus Torvalds
stack_trace_save() uses skipnr + 1 because arch_stack_walk() is expected
to first report the return into stack_trace_save(). With FRAME_POINTER,
the RISC-V arch_stack_walk() calls walk_stackframe(), which starts from
its own frame and first reports the return into arch_stack_walk().
stack_trace_save() therefore starts one frame too early. For example,
page_owner records begin at __set_page_owner() instead of the allocation
site.
Move the walker into an always-inlined helper and call it directly from
arch_stack_walk(). Keep walk_stackframe() as a wrapper for its existing
callers.
return_address() skips level + 3 entries, while the arm64 implementation
skips level + 2. After removing the extra frame, use level + 2 so
ftrace_return_address() and CALLER_ADDR results do not shift by one caller.
Fixes: 6a00ef449370 ("riscv: eliminate unreliable __builtin_frame_address(1)")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <redacted>
---
Tested with the KUnit suite from patch 2 on QEMU's virt machine, clang 22
(LLVM=1), on af32da41b032:
mainline stack_trace_save 0/3, return_address 2/2
stacktrace.c change alone stack_trace_save 3/3, return_address 0/2
this patch stack_trace_save 3/3, return_address 2/2
A BeagleV Ahead (TH1520) booted with this patch passes all five cases.
The stack_trace_save() result on mainline (0/3) was also reproduced with
GCC 15.2 and QEMU 10.2.1.
Without CONFIG_FRAME_POINTER the walker scans the stack for anything that
looks like a text address and its start frame is not exact before or
after this patch. What return_address() returns there is unchanged: the
walk loses one leading entry and the skip count goes down by one.
The reliable unwinder series [1] rewrites this walker and has the correct
start frame for stack_trace_save() as a side effect, but leaves
return_address() at level + 3. This patch conflicts with 5/7 of that
series; if that goes in first, only the return_address.c hunk is needed.
[1] https://lore.kernel.org/r/20260914092648.51254-1-xueshuai@linux.alibaba.com (local)
arch/riscv/kernel/return_address.c | 2 +-
arch/riscv/kernel/stacktrace.c | 40 +++++++++++++++++++++++++++++-----
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/kernel/return_address.c b/arch/riscv/kernel/return_address.c
index c8115ec8fb30..4da1cfd249d3 100644
--- a/arch/riscv/kernel/return_address.c
+++ b/arch/riscv/kernel/return_address.c@@ -33,7 +33,7 @@ noinline void *return_address(unsigned int level) { struct return_address_data data; - data.level = level + 3; + data.level = level + 2; data.addr = NULL; arch_stack_walk(save_return_addr, &data, current, NULL);
diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index c7555447149b..28c5744889ba 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c@@ -45,8 +45,16 @@ static inline int fp_is_valid(unsigned long fp, unsigned long sp) return !(fp < low || fp > high || fp & 0x07); } -void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, - bool (*fn)(void *, unsigned long), void *arg) +/* + * Always inlined so that the frame the walk starts from is the caller's own, + * and the first entry reported for the current task is the caller's return + * address. arch_stack_walk() relies on that to report the same frames as + * the other architectures, see the comment there. + */ +static __always_inline void __walk_stackframe(struct task_struct *task, + struct pt_regs *regs, + bool (*fn)(void *, unsigned long), + void *arg) { unsigned long fp, sp, pc; int graph_idx = 0;
@@ -59,6 +67,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, } else if (task == NULL || task == current) { fp = (unsigned long)__builtin_frame_address(0); sp = current_stack_pointer; + /* Placeholder for the frame the walk starts from, never reported. */ pc = (unsigned long)walk_stackframe; level = -1; } else {
@@ -102,10 +111,18 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, } } +void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, + bool (*fn)(void *, unsigned long), void *arg) +{ + __walk_stackframe(task, regs, fn, arg); +} + #else /* !CONFIG_FRAME_POINTER */ -void notrace walk_stackframe(struct task_struct *task, - struct pt_regs *regs, bool (*fn)(void *, unsigned long), void *arg) +static __always_inline void __walk_stackframe(struct task_struct *task, + struct pt_regs *regs, + bool (*fn)(void *, unsigned long), + void *arg) { unsigned long sp, pc; unsigned long *ksp;
@@ -133,6 +150,12 @@ void notrace walk_stackframe(struct task_struct *task, } } +void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs, + bool (*fn)(void *, unsigned long), void *arg) +{ + __walk_stackframe(task, regs, fn, arg); +} + #endif /* CONFIG_FRAME_POINTER */ static bool print_trace_address(void *arg, unsigned long pc)
@@ -176,10 +199,17 @@ unsigned long __get_wchan(struct task_struct *task) return pc; } +/* + * The generic code expects the first entry for the current task to be the + * return address of arch_stack_walk() itself, the way x86 and arm64 report + * it, and stack_trace_save() sizes its skip count accordingly. Walking from + * inside a called walk_stackframe() adds that function's own frame on top, + * so inline the walker here and start from this frame instead. + */ noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, struct task_struct *task, struct pt_regs *regs) { - walk_stackframe(task, regs, consume_entry, cookie); + __walk_stackframe(task, regs, consume_entry, cookie); } /*