Message ID | cover.1718908016.git.naveen@kernel.org (mailing list archive) |
---|---|
Headers | show |
Series | powerpc: Add support for ftrace direct and BPF trampolines | expand |
On Fri, Jun 21, 2024 at 12:24:03AM +0530, Naveen N Rao wrote: > This is v3 of the patches posted here: > http://lkml.kernel.org/r/cover.1718008093.git.naveen@kernel.org > > Since v2, I have addressed review comments from Steven and Masahiro > along with a few fixes. Patches 7-11 are new in this series and add > support for ftrace direct and bpf trampolines. > > This series depends on the patch series from Benjamin Gray adding > support for patch_ulong(): > http://lkml.kernel.org/r/20240515024445.236364-1-bgray@linux.ibm.com This patchset, along with its dependent patchset [2], enables support for BPF schedulers introduced by the sched_ext patchset [1] on Power Architecture. Before this patchset, BPF schedulers were failing to load due to the lack of trampoline support. GitHub repo for sched_ext: https://github.com/sched-ext/sched_ext Note: This repo has been archived. Please refer to the description provided on the repo homepage for more details. [1] v6 https://lore.kernel.org/lkml/20240501151312.635565-1-tj@kernel.org [2] http://lkml.kernel.org/r/20240515024445.236364-1-bgray@linux.ibm.com Thank you for the patch! > > > - Naveen > > > Naveen N Rao (11): > powerpc/kprobes: Use ftrace to determine if a probe is at function > entry > powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code > powerpc/module_64: Convert #ifdef to IS_ENABLED() > powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace > kbuild: Add generic hook for architectures to use before the final > vmlinux link > powerpc64/ftrace: Move ftrace sequence out of line > powerpc/ftrace: Add support for DYNAMIC_FTRACE_WITH_CALL_OPS > powerpc/ftrace: Add support for DYNAMIC_FTRACE_WITH_DIRECT_CALLS > samples/ftrace: Add support for ftrace direct samples on powerpc > powerpc64/bpf: Fold bpf_jit_emit_func_call_hlp() into > bpf_jit_emit_func_call_rel() > powerpc64/bpf: Add support for bpf trampolines > > arch/Kconfig | 3 + > arch/powerpc/Kconfig | 9 + > arch/powerpc/Makefile | 8 + > arch/powerpc/include/asm/ftrace.h | 29 +- > arch/powerpc/include/asm/module.h | 5 + > arch/powerpc/include/asm/ppc-opcode.h | 14 + > arch/powerpc/kernel/asm-offsets.c | 11 + > arch/powerpc/kernel/kprobes.c | 18 +- > arch/powerpc/kernel/module_64.c | 67 +- > arch/powerpc/kernel/trace/ftrace.c | 269 +++++++- > arch/powerpc/kernel/trace/ftrace_64_pg.c | 73 +- > arch/powerpc/kernel/trace/ftrace_entry.S | 210 ++++-- > arch/powerpc/kernel/vmlinux.lds.S | 3 +- > arch/powerpc/net/bpf_jit.h | 11 + > arch/powerpc/net/bpf_jit_comp.c | 702 +++++++++++++++++++- > arch/powerpc/net/bpf_jit_comp32.c | 7 +- > arch/powerpc/net/bpf_jit_comp64.c | 68 +- > arch/powerpc/tools/Makefile | 10 + > arch/powerpc/tools/gen-ftrace-pfe-stubs.sh | 49 ++ > samples/ftrace/ftrace-direct-modify.c | 85 ++- > samples/ftrace/ftrace-direct-multi-modify.c | 101 ++- > samples/ftrace/ftrace-direct-multi.c | 79 ++- > samples/ftrace/ftrace-direct-too.c | 83 ++- > samples/ftrace/ftrace-direct.c | 69 +- > scripts/Makefile.vmlinux | 8 + > scripts/link-vmlinux.sh | 11 +- > 26 files changed, 1813 insertions(+), 189 deletions(-) > create mode 100644 arch/powerpc/tools/Makefile > create mode 100755 arch/powerpc/tools/gen-ftrace-pfe-stubs.sh > > > base-commit: e2b06d707dd067509cdc9ceba783c06fa6a551c2 > prerequisite-patch-id: a1d50e589288239d5a8b1c1f354cd4737057c9d3 > prerequisite-patch-id: da4142d56880861bd0ad7ad7087c9e2670a2ee54 > prerequisite-patch-id: 609d292e054b2396b603890522a940fa0bdfb6d8 > prerequisite-patch-id: 6f7213fb77b1260defbf43be0e47bff9c80054cc > prerequisite-patch-id: ad3b71bf071ae4ba1bee5b087e61a2055772a74f > -- > 2.45.2 >
On Fri, Jun 21, 2024 at 12:24:03AM +0530, Naveen N Rao wrote: > This is v3 of the patches posted here: > http://lkml.kernel.org/r/cover.1718008093.git.naveen@kernel.org > > Since v2, I have addressed review comments from Steven and Masahiro > along with a few fixes. Patches 7-11 are new in this series and add > support for ftrace direct and bpf trampolines. > > This series depends on the patch series from Benjamin Gray adding > support for patch_ulong(): > http://lkml.kernel.org/r/20240515024445.236364-1-bgray@linux.ibm.com > > > - Naveen Hello Naveen, I've noticed an issue with `kstack()` in bpftrace [1] when using `kfunc` compared to `kprobe`. Despite trying all three modes specified in the documentation (bpftrace, perf, or raw), the stack isn't unwinding properly with `kfunc`. [1] https://github.com/bpftrace/bpftrace/blob/master/man/adoc/bpftrace.adoc#kstack for mode in modes; do run bpftrace with kfunc disable cpu kill bpftrace run bpftrace with kprobe enable cpu kill bpftrace # ./kprobe_vs_kfunc.sh + bpftrace -e 'kfunc:vmlinux:build_sched_domains {@[kstack(bpftrace), comm, tid]=count();}' Attaching 1 probe... + chcpu -d 2-3 CPU 2 disabled CPU 3 disabled + kill 35214 @[ bpf_prog_cfd8d6c8bb4898ce+972 , cpuhp/2, 33]: 1 @[ bpf_prog_cfd8d6c8bb4898ce+972 , cpuhp/3, 38]: 1 + bpftrace -e 'kprobe:build_sched_domains {@[kstack(bpftrace), comm, tid]=count();}' Attaching 1 probe... + chcpu -e 2-3 CPU 2 enabled CPU 3 enabled + kill 35221 @[ 0x80000007642bdfb4 partition_sched_domains_locked+1304 rebuild_sched_domains_locked+216 cpuset_handle_hotplug+1148 sched_cpu_activate+664 cpuhp_invoke_callback+480 cpuhp_thread_fun+244 smpboot_thread_fn+460 kthread+308 start_kernel_thread+20 , cpuhp/3, 38]: 1 @[ 0x80000007524b34a4 partition_sched_domains_locked+1304 rebuild_sched_domains_locked+216 cpuset_handle_hotplug+1148 sched_cpu_activate+664 cpuhp_invoke_callback+480 cpuhp_thread_fun+244 smpboot_thread_fn+460 kthread+308 start_kernel_thread+20 , cpuhp/2, 33]: 1 + bpftrace -e 'kfunc:vmlinux:build_sched_domains {@[kstack(perf), comm, tid]=count();}' Attaching 1 probe... + chcpu -d 2-3 CPU 2 disabled CPU 3 disabled + kill 35229 @[ c008000003433454 bpf_prog_cfd8d6c8bb4898ce+960 , cpuhp/3, 38]: 1 @[ c008000003433454 bpf_prog_cfd8d6c8bb4898ce+960 , cpuhp/2, 33]: 1 + bpftrace -e 'kprobe:build_sched_domains {@[kstack(perf), comm, tid]=count();}' Attaching 1 probe... + chcpu -e 2-3 CPU 2 enabled CPU 3 enabled + kill 35235 @[ 80000007524b379c 0x80000007524b379c c000000000206268 partition_sched_domains_locked+1304 c0000000002cbf08 rebuild_sched_domains_locked+216 c0000000002cfd5c cpuset_handle_hotplug+1148 c0000000001b3178 sched_cpu_activate+664 c000000000156fc0 cpuhp_invoke_callback+480 c000000000157974 cpuhp_thread_fun+244 c00000000019ddec smpboot_thread_fn+460 c0000000001932c4 kthread+308 c00000000000dd58 start_kernel_thread+20 , cpuhp/2, 33]: 1 @[ 80000007642b9b6c 0x80000007642b9b6c c000000000206268 partition_sched_domains_locked+1304 c0000000002cbf08 rebuild_sched_domains_locked+216 c0000000002cfd5c cpuset_handle_hotplug+1148 c0000000001b3178 sched_cpu_activate+664 c000000000156fc0 cpuhp_invoke_callback+480 c000000000157974 cpuhp_thread_fun+244 c00000000019ddec smpboot_thread_fn+460 c0000000001932c4 kthread+308 c00000000000dd58 start_kernel_thread+20 , cpuhp/3, 38]: 1 + bpftrace -e 'kfunc:vmlinux:build_sched_domains {@[kstack(raw), comm, tid]=count();}' Attaching 1 probe... + chcpu -d 2-3 CPU 2 disabled CPU 3 disabled + kill 35243 @[ c00800000343346c , cpuhp/3, 38]: 1 @[ c00800000343346c , cpuhp/2, 33]: 1 + bpftrace -e 'kprobe:build_sched_domains {@[kstack(raw), comm, tid]=count();}' Attaching 1 probe... + chcpu -e 2-3 CPU 2 enabled CPU 3 enabled + kill 35249 @[ 80000007642befac c000000000206268 c0000000002cbf08 c0000000002cfd5c c0000000001b3178 c000000000156fc0 c000000000157974 c00000000019ddec c0000000001932c4 c00000000000dd58 , cpuhp/3, 38]: 1 @[ 80000007524b425c c000000000206268 c0000000002cbf08 c0000000002cfd5c c0000000001b3178 c000000000156fc0 c000000000157974 c00000000019ddec c0000000001932c4 c00000000000dd58 , cpuhp/2, 33]: 1 > > > Naveen N Rao (11): > powerpc/kprobes: Use ftrace to determine if a probe is at function > entry > powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code > powerpc/module_64: Convert #ifdef to IS_ENABLED() > powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace > kbuild: Add generic hook for architectures to use before the final > vmlinux link > powerpc64/ftrace: Move ftrace sequence out of line > powerpc/ftrace: Add support for DYNAMIC_FTRACE_WITH_CALL_OPS > powerpc/ftrace: Add support for DYNAMIC_FTRACE_WITH_DIRECT_CALLS > samples/ftrace: Add support for ftrace direct samples on powerpc > powerpc64/bpf: Fold bpf_jit_emit_func_call_hlp() into > bpf_jit_emit_func_call_rel() > powerpc64/bpf: Add support for bpf trampolines > > arch/Kconfig | 3 + > arch/powerpc/Kconfig | 9 + > arch/powerpc/Makefile | 8 + > arch/powerpc/include/asm/ftrace.h | 29 +- > arch/powerpc/include/asm/module.h | 5 + > arch/powerpc/include/asm/ppc-opcode.h | 14 + > arch/powerpc/kernel/asm-offsets.c | 11 + > arch/powerpc/kernel/kprobes.c | 18 +- > arch/powerpc/kernel/module_64.c | 67 +- > arch/powerpc/kernel/trace/ftrace.c | 269 +++++++- > arch/powerpc/kernel/trace/ftrace_64_pg.c | 73 +- > arch/powerpc/kernel/trace/ftrace_entry.S | 210 ++++-- > arch/powerpc/kernel/vmlinux.lds.S | 3 +- > arch/powerpc/net/bpf_jit.h | 11 + > arch/powerpc/net/bpf_jit_comp.c | 702 +++++++++++++++++++- > arch/powerpc/net/bpf_jit_comp32.c | 7 +- > arch/powerpc/net/bpf_jit_comp64.c | 68 +- > arch/powerpc/tools/Makefile | 10 + > arch/powerpc/tools/gen-ftrace-pfe-stubs.sh | 49 ++ > samples/ftrace/ftrace-direct-modify.c | 85 ++- > samples/ftrace/ftrace-direct-multi-modify.c | 101 ++- > samples/ftrace/ftrace-direct-multi.c | 79 ++- > samples/ftrace/ftrace-direct-too.c | 83 ++- > samples/ftrace/ftrace-direct.c | 69 +- > scripts/Makefile.vmlinux | 8 + > scripts/link-vmlinux.sh | 11 +- > 26 files changed, 1813 insertions(+), 189 deletions(-) > create mode 100644 arch/powerpc/tools/Makefile > create mode 100755 arch/powerpc/tools/gen-ftrace-pfe-stubs.sh > > > base-commit: e2b06d707dd067509cdc9ceba783c06fa6a551c2 > prerequisite-patch-id: a1d50e589288239d5a8b1c1f354cd4737057c9d3 > prerequisite-patch-id: da4142d56880861bd0ad7ad7087c9e2670a2ee54 > prerequisite-patch-id: 609d292e054b2396b603890522a940fa0bdfb6d8 > prerequisite-patch-id: 6f7213fb77b1260defbf43be0e47bff9c80054cc > prerequisite-patch-id: ad3b71bf071ae4ba1bee5b087e61a2055772a74f > -- > 2.45.2 >
Hi Vishal, Vishal Chourasia wrote: > On Fri, Jun 21, 2024 at 12:24:03AM +0530, Naveen N Rao wrote: >> This is v3 of the patches posted here: >> http://lkml.kernel.org/r/cover.1718008093.git.naveen@kernel.org >> >> Since v2, I have addressed review comments from Steven and Masahiro >> along with a few fixes. Patches 7-11 are new in this series and add >> support for ftrace direct and bpf trampolines. >> >> This series depends on the patch series from Benjamin Gray adding >> support for patch_ulong(): >> http://lkml.kernel.org/r/20240515024445.236364-1-bgray@linux.ibm.com >> >> >> - Naveen > > Hello Naveen, > > I've noticed an issue with `kstack()` in bpftrace [1] when using `kfunc` > compared to `kprobe`. Despite trying all three modes specified in the > documentation (bpftrace, perf, or raw), the stack isn't unwinding > properly with `kfunc`. > > [1] https://github.com/bpftrace/bpftrace/blob/master/man/adoc/bpftrace.adoc#kstack > > > for mode in modes; do > run bpftrace with kfunc > disable cpu > kill bpftrace > run bpftrace with kprobe > enable cpu > kill bpftrace > > # ./kprobe_vs_kfunc.sh > + bpftrace -e 'kfunc:vmlinux:build_sched_domains {@[kstack(bpftrace), comm, tid]=count();}' > Attaching 1 probe... > + chcpu -d 2-3 > CPU 2 disabled > CPU 3 disabled > + kill 35214 > > @[ > bpf_prog_cfd8d6c8bb4898ce+972 > , cpuhp/2, 33]: 1 > @[ > bpf_prog_cfd8d6c8bb4898ce+972 > , cpuhp/3, 38]: 1 Yeah, this is because we don't capture the full register state with bpf trampolines unlike with kprobes. BPF stackmap relies on perf_arch_fetch_caller_regs() to create a dummy pt_regs for use by get_perf_callchain(). We end up with a NULL LR, and bpftrace (and most other userspace tools) stop showing the backtrace when they encounter a NULL entry. I recall fixing some tools to continue to show backtrace inspite of a NULL entry, but I may be mis-remembering. Perhaps we should fix/change how the perf callchain is captured in the kernel. We filter out invalid entries, and capture an additional entry for perf since we can't be sure of our return address. We should revisit this and see if we can align with the usual expectations of a callchain not having a NULL entry. Something like this may help, but this needs more testing especially on the perf side: diff --git a/arch/powerpc/perf/callchain.c b/arch/powerpc/perf/callchain.c index 6b4434dd0ff3..9f67b764da92 100644 --- a/arch/powerpc/perf/callchain.c +++ b/arch/powerpc/perf/callchain.c @@ -83,12 +83,12 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *re * We can't tell which of the first two addresses * we get are valid, but we can filter out the * obviously bogus ones here. We replace them - * with 0 rather than removing them entirely so + * with -1 rather than removing them entirely so * that userspace can tell which is which. */ if ((level == 1 && next_ip == lr) || (level <= 1 && !kernel_text_address(next_ip))) - next_ip = 0; + next_ip = -1; ++level; } - Naveen