On Mon, Oct 27, 2025 at 01:52:18PM +0000, [email protected] wrote:
>
> ```
> commit 26b06def1956497a15eecf22787d5430fad2fed6
> Author: Jiri Olsa <[email protected]>
>
> Revert "perf/x86: Always store regs->ip in perf_callchain_kernel()"
>
> This commit reverts a 2019 fix that ensured regs->ip is stored when
> using non-HW regs (regs populated by perf_arch_fetch_caller_regs). The
> revert claims the IP is captured during unwinding, but analysis suggests
> otherwise.
>
> > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> > index 745caa6c1..fa6c47b50 100644
> > --- a/arch/x86/events/core.c
> > +++ b/arch/x86/events/core.c
> > @@ -2789,13 +2789,13 @@ perf_callchain_kernel(struct
> > perf_callchain_entry_ctx *entry, struct pt_regs *re
> > return;
> > }
> >
> > - if (perf_callchain_store(entry, regs->ip))
> > - return;
> > -
> > - if (perf_hw_regs(regs))
> > + if (perf_hw_regs(regs)) {
> > + if (perf_callchain_store(entry, regs->ip))
> > + return;
> > unwind_start(&state, current, regs, NULL);
> > - else
> > + } else {
> > unwind_start(&state, current, NULL, (void *)regs->sp);
> > + }
>
> When perf_hw_regs(regs) returns false (which happens when regs comes from
> perf_arch_fetch_caller_regs since it sets regs->flags=0), the else branch
> calls unwind_start with regs=NULL. Can this lose the regs->ip value?
>
> Looking at the code flow:
>
> perf_arch_fetch_caller_regs() sets:
> - regs->ip = CALLER_ADDR0 (the caller's instruction pointer)
> - regs->flags = 0 (no X86_EFLAGS_FIXED)
>
> perf_hw_regs() checks:
> - returns (regs->flags & X86_EFLAGS_FIXED), which is false
>
> The else branch executes:
> - unwind_start(&state, current, NULL, (void *)regs->sp)
>
> In __unwind_start() at arch/x86/kernel/unwind_orc.c, when regs=NULL:
> - Takes the "else if (task == current)" path (line 711)
> - Initializes state->ip via inline asm from current location
> - This is __unwind_start's IP, not the original regs->ip
> - Then advances to first_frame at regs->sp (lines 755-758)
>
> The original regs->ip value (captured by perf_arch_fetch_caller_regs) is
> never stored in the callchain. This was the exact issue fixed by commit
IIUC it is, unwinder will eventually unwind to it
> 83f44ae0f8af in 2019, which stated:
>
> "The stacktrace_map_raw_tp BPF selftest is failing because the RIP
> saved by perf_arch_fetch_caller_regs() isn't getting saved by
> perf_callchain_kernel()."
>
> Does this revert re-introduce the BPF selftest failure that was fixed in
> 2019? The test tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
> still exists in the kernel tree.
stacktrace_map_raw_tp does not check what's the first ip of the
stacktrace and it passes with or without this change
I can see double entries on current code and just one with this change:
# bpftrace -e 'tracepoint:sched:sched_process_exec { print(kstack()); }'
Attaching 1 probe...
bprm_execve+1767
bprm_execve+1767
do_execveat_common.isra.0+425
__x64_sys_execve+56
do_syscall_64+133
entry_SYSCALL_64_after_hwframe+118
thanks,
jirka
