Re: [PATCH 1/3] Revert "perf/x86: Always store regs->ip in perf_callchain_kernel()"

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, Oct 27, 2025 at 01:52:18PM +0000, bot+bpf-ci@kernel.org wrote:
> 
> ```
> commit 26b06def1956497a15eecf22787d5430fad2fed6
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> 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