Skip to content

Conversation

@kernel-patches-daemon-bpf
Copy link

Pull request for series with
subject: x86/fgraph,bpf: Fix ORC stack unwind from kprobe_multi
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 78980b4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 95ffdbd
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 95ffdbd
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 78980b4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 8016abd
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 1456ebb
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 35538db
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 35538db
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 35538db
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: ae23bc8
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: ba22540
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 08a7491
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

The previous change (Fixes commit) messed up the rsp register value,
which is wrong because it's already adjusted with FRAME_SIZE, we need
the original rsp value.

This change does not affect fprobe current kernel unwind, the !perf_hw_regs
path perf_callchain_kernel:

        if (perf_hw_regs(regs)) {
                if (perf_callchain_store(entry, regs->ip))
                        return;
                unwind_start(&state, current, regs, NULL);
        } else {
                unwind_start(&state, current, NULL, (void *)regs->sp);
        }

which uses pt_regs.sp as first_frame boundary (FRAME_SIZE shift makes
no difference, unwind stil stops at the right frame).

This change fixes the other path when we want to unwind directly from
pt_regs sp/fp/ip state, which is coming in following change.

Fixes: 20a0bc1 ("x86/fgraph,bpf: Fix stack ORC unwind from kprobe_multi return probe")
Signed-off-by: Jiri Olsa <[email protected]>
Mahe reported missing function from stack trace on top of kprobe
multi program. The missing function is the very first one in the
stacktrace, the one that the bpf program is attached to.

  # bpftrace -e 'kprobe:__x64_sys_newuname* { print(kstack)}'
  Attaching 1 probe...

        do_syscall_64+134
        entry_SYSCALL_64_after_hwframe+118

  ('*' is used for kprobe_multi attachment)

The reason is that the previous change (the Fixes commit) fixed
stack unwind for tracepoint, but removed attached function address
from the stack trace on top of kprobe multi programs, which I also
overlooked in the related test (check following patch).

The tracepoint and kprobe_multi have different stack setup, but use
same unwind path. I think it's better to keep the previous change,
which fixed tracepoint unwind and instead change the kprobe multi
unwind as explained below.

The bpf program stack unwind calls perf_callchain_kernel for kernel
portion and it follows two unwind paths based on X86_EFLAGS_FIXED
bit in pt_regs.flags.

When the bit set we unwind from stack represented by pt_regs argument,
otherwise we unwind currently executed stack up to 'first_frame'
boundary.

The 'first_frame' value is taken from regs.rsp value, but ftrace_caller
and ftrace_regs_caller (ftrace trampoline) functions set the regs.rsp
to the previous stack frame, so we skip the attached function entry.

If we switch kprobe_multi unwind to use the X86_EFLAGS_FIXED bit,
we set the start of the unwind to the attached function address.
As another benefit we also cut extra unwind cycles needed to reach
the 'first_frame' boundary.

The speedup can be measured with trigger bench for kprobe_multi
program and stacktrace support.

- trigger bench with stacktrace on current code:

        kprobe-multi   :     0.810 ± 0.001M/s
        kretprobe-multi:     0.808 ± 0.001M/s

- and with the fix:

        kprobe-multi   :     1.264 ± 0.001M/s
        kretprobe-multi:     1.401 ± 0.002M/s

With the fix, the entry probe stacktrace:

  # bpftrace -e 'kprobe:__x64_sys_newuname* { print(kstack)}'
  Attaching 1 probe...

        __x64_sys_newuname+9
        do_syscall_64+134
        entry_SYSCALL_64_after_hwframe+118

The return probe skips the attached function, because it's no longer
on the stack at the point of the unwind and this way is the same how
standard kretprobe works.

  # bpftrace -e 'kretprobe:__x64_sys_newuname* { print(kstack)}'
  Attaching 1 probe...

        do_syscall_64+134
        entry_SYSCALL_64_after_hwframe+118

Fixes: 6d08340 ("Revert "perf/x86: Always store regs->ip in perf_callchain_kernel()"")
Reported-by: Mahe Tardy <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
We now include the attached function in the stack trace,
fixing the test accordingly.

Fixes: c9e208f ("selftests/bpf: Add stacktrace ips test for kprobe_multi/kretprobe_multi")
Signed-off-by: Jiri Olsa <[email protected]>
Adding test that attaches kprobe/kretprobe and verifies the
ORC stacktrace matches expected functions.

The test is only for ORC unwinder to keep it simple.

Signed-off-by: Jiri Olsa <[email protected]>
Adding test that attaches fentry/fexitand verifies the
ORC stacktrace matches expected functions.

The test is only for ORC unwinder to keep it simple.

Signed-off-by: Jiri Olsa <[email protected]>
Adding support to call bpf_get_stackid helper from trigger programs,
so far added for kprobe multi.

Adding the --stacktrace/-g option to enable it.

Signed-off-by: Jiri Olsa <[email protected]>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 08a7491
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1047287
version: 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant