On 10/10/26 2:59 AM, Karl Mehltretter wrote:
On Mon, Sep 14, 2026 at 05:26:46PM +0800, Shuai Xue wrote:+noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, + void *cookie, struct task_struct *task, + struct pt_regs *regs) +{ + struct kunwind_consume_entry_data data = { + .consume_entry = consume_entry, + .cookie = cookie, + }; + + kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs); +}Hi Shuai Xue, With this patch and FRAME_POINTER enabled, return_address() returns the caller one level further up than asked for. return_address() in arch/riscv/kernel/return_address.c skips level + 3 entries from arch_stack_walk(). The arm64 original skips level + 2. The third entry exists on mainline because the walk starts inside walk_stackframe() and first reports the return into arch_stack_walk(). With 5/7 the walk starts at the caller of arch_stack_walk(), so level + 3 now skips a real caller. CALLER_ADDR1 and up are built on return_address(). The irqsoff tracer and the preemptirq tracepoints use them. A KUnit test that calls ftrace_return_address() from leaf() <- middle() <- outer() <- check() returns: af32da41b032 with series with series, level + 2 level 0 middle outer middle level 1 outer check outer The change to level + 2 has to depend on FRAME_POINTER. Without it the series keeps walk_stackframe() and the result is the same as on mainline. There, level + 2 makes return_address(0) return an address inside return_address().
Hi, Karl, Nice point. Thank you for the thorough review and the KUnit test. The analysis is spot on, and your table matches my reading of the code exactly.
The same extra entry makes stack_trace_save() start one frame too early on mainline. 5/7 fixes that. I have a small fix for it on mainline, meant for stable kernels, which conflicts with 5/7. I would post it with you in Cc. Tell me if you would rather take the return_address.c change into 5/7 first.
Please go ahead with your mainline/stable fix for the stack_trace_save() off-by-one as an independent patch. You are right that this is a separate mainline bug: stack_trace_save() hardcodes .skip = skipnr + 1, i.e. it expects the first entry reported by arch_stack_walk() to be the return into stack_trace_save() itself, and mainline's walk_stackframe() breaks that contract by reporting the return into arch_stack_walk() first. A 7-patch series is not a vehicle for stable, so the minimal fix is the right approach there. Please keep me in Cc when you post it, I will review and test it.
Tested with clang 22 and the series applied on 52318cf0fa6e. The "with series" column is the same on QEMU's virt machine and on a BeagleV Ahead (TH1520). Everything else ran on QEMU only.
Thanks again for testing on real hardware (BeagleV Ahead) in addition to QEMU, that is much appreciated. Best regards, Shuai Xue
