Skip to content

Commit af3694e

Browse files
olsajiriKernel Patches Daemon
authored andcommitted
Revert "perf/x86: Always store regs->ip in perf_callchain_kernel()"
This reverts commit 83f44ae. Currently we store initial stacktrace entry twice for non-HW ot_regs, which means callers that fail perf_hw_regs(regs) condition in perf_callchain_kernel. It's easy to reproduce this bpftrace: # 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 When perf_callchain_kernel calls unwind_start with first_frame, AFAICS we do not skip regs->ip, but it's added as part of the unwind process. Hence reverting the extra perf_callchain_store for non-hw regs leg. I was not able to bisect this, so I'm not really sure why this was needed in v5.2 and why it's not working anymore, but I could see double entries as far as v5.10. I did the test for both ORC and framepointer unwind with and without the this fix and except for the initial entry the stacktraces are the same. Acked-by: Song Liu <[email protected]> Signed-off-by: Jiri Olsa <[email protected]>
1 parent fe98223 commit af3694e

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

arch/x86/events/core.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,13 +2789,13 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *re
27892789
return;
27902790
}
27912791

2792-
if (perf_callchain_store(entry, regs->ip))
2793-
return;
2794-
2795-
if (perf_hw_regs(regs))
2792+
if (perf_hw_regs(regs)) {
2793+
if (perf_callchain_store(entry, regs->ip))
2794+
return;
27962795
unwind_start(&state, current, regs, NULL);
2797-
else
2796+
} else {
27982797
unwind_start(&state, current, NULL, (void *)regs->sp);
2798+
}
27992799

28002800
for (; !unwind_done(&state); unwind_next_frame(&state)) {
28012801
addr = unwind_get_return_address(&state);

0 commit comments

Comments
 (0)