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

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, Oct 29, 2025 at 08:17:05AM +0100, Jiri Olsa wrote:
> On Tue, Oct 28, 2025 at 08:39:33PM -0700, Josh Poimboeuf wrote:
> > On Mon, Oct 27, 2025 at 11:38:50PM +0100, Jiri Olsa wrote:
> > > On Mon, Oct 27, 2025 at 01:19:52PM -0700, Josh Poimboeuf wrote:
> > > > On Mon, Oct 27, 2025 at 01:52:18PM +0000, bot+bpf-ci@kernel.org wrote:
> > > > > 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.
> > > > 
> > > > I have the same question.  And note there may be subtle differences
> > > > between the frame pointer and ORC unwinders.  The testcase would need to
> > > > pass for both.
> > > 
> > > as I wrote in the other email that test does not check ips directly,
> > > it just compare stacks taken from bpf_get_stackid and bpf_get_stack
> > > helpers.. so it passes for both orc and frame pointer unwinder
> > 
> > Ok.  So the original fix wasn't actually a fix at all?  It would be good
> > to understand that and mention it in the commit log.  Otherwise it's not
> > clear why it's ok to revert a fix with no real explanation.
> 
> I think it was a fix when it was pushed 6 years ago, but some
> unwind change along that time made it redundant, I'll try to
> find what the change was

hum I can't tell what changed since v5.2 (kernel version when [1] landed)
that reverted the behaviour which the [1] commit was fixing

I did the test for both orc and framepointer unwind with and without the
fix (revert of [1]) and except for the initial entry it does not seem to
change the rest of the unwind ... though I'd expect orc unwind to have
more entries

please check results below

any idea? thanks,
jirka


[1] 83f44ae0f8af perf/x86: Always store regs->ip in perf_callchain_kernel()
[2] ae6a45a08689 x86/unwind/orc: Fall back to using frame pointers for generated code
---
framepointer + fix:

        bpf_prog_2beb79c650d605dd_rawtracepoint_bpf_testmod_test_read_1+324
        bpf_trace_run2+216
        __bpf_trace_bpf_testmod_test_read+13
        bpf_testmod_test_read+1322
        sysfs_kf_bin_read+103
        kernfs_fop_read_iter+243
        vfs_read+549
        ksys_read+115
        __x64_sys_read+29
        x64_sys_call+6112
        do_syscall_64+133
        entry_SYSCALL_64_after_hwframe+118

framepointer withtout fix:

        bpf_prog_2beb79c650d605dd_rawtracepoint_bpf_testmod_test_read_1+324
        bpf_prog_2beb79c650d605dd_rawtracepoint_bpf_testmod_test_read_1+324
        bpf_trace_run2+216
        __bpf_trace_bpf_testmod_test_read+13
        bpf_testmod_test_read+1322
        sysfs_kf_bin_read+103
        kernfs_fop_read_iter+243
        vfs_read+549
        ksys_read+115
        __x64_sys_read+29
        x64_sys_call+6112
        do_syscall_64+133
        entry_SYSCALL_64_after_hwframe+118


orc + fix:

        bpf_prog_2beb79c650d605dd_rawtracepoint_bpf_testmod_test_read_1+324
        bpf_trace_run2+214
        bpf_testmod_test_read+1322
        kernfs_fop_read_iter+228
        vfs_read+550
        ksys_read+112
        do_syscall_64+133
        entry_SYSCALL_64_after_hwframe+118


orc without fix:

        bpf_prog_2beb79c650d605dd_rawtracepoint_bpf_testmod_test_read_1+324
        bpf_prog_2beb79c650d605dd_rawtracepoint_bpf_testmod_test_read_1+324
        bpf_trace_run2+214
        bpf_testmod_test_read+1322
        kernfs_fop_read_iter+228
        vfs_read+550
        ksys_read+112
        do_syscall_64+133
        entry_SYSCALL_64_after_hwframe+118