On Mon, Apr 24, 2023 at 9:04 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> hi,
> this patchset is adding support to attach multiple uprobes and usdt probes
> through new uprobe_multi link.
>
> The current uprobe is attached through the perf event and attaching many
> uprobes takes a lot of time because of that.
>
> The main reason is that we need to install perf event for each probed function
> and profile shows perf event installation (perf_install_in_context) as culprit.
>
> The new uprobe_multi link just creates raw uprobes and attaches the bpf
> program to them without perf event being involved.
>
> In addition to being faster we also save file descriptors. For the current
> uprobe attach we use extra perf event fd for each probed function. The new
> link just need one fd that covers all the functions we are attaching to.
All of the above are good reasons and thanks for tackling multi-uprobe!
>
> By dropping perf we lose the ability to attach uprobe to specific pid.
> We can workaround that by having pid check directly in the bpf program,
> but we might need to check for another solution if that will turn out
> to be a problem.
>
I think this is a big deal, because it makes multi-uprobe not a
drop-in replacement for normal uprobes even for typical scenarios. It
might be why you couldn't do transparent use of uprobe.multi in USDT?
But I'm not sure why this is a problem? How does perf handle this?
Does it do runtime filtering or something more efficient that prevents
uprobe to be triggered for other PIDs in the first place? If it's the
former, then why can't we do the same simple check ourselves if pid
filter is specified?
I also see that uprobe_consumer has filter callback, not sure if it's
a better solution just for pid filtering, but might be another way to
do this?
Another aspect I wanted to discuss (and I don't know the right answer)
was whether we need to support separate binary path for each offset?
It would simplify (and trim down memory usage significantly) a bunch
of internals if we knew we are dealing with single inode for each
multi-uprobe link. I'm trying to think if it would be limiting in
practice to have to create link per each binary, and so far it seems
like usually user-space code will do symbol resolution per ELF file
anyways, so doesn't seem limiting to have single path + multiple
offsets/cookies within that file. For USDTs use case even ref_ctr is
probably the same, but I'd keep it 1:1 with offset and cookie anyways.
For uniformity and generality.
WDYT?
>
> Attaching current bpftrace to 1000 uprobes:
>
> # BPFTRACE_MAX_PROBES=100000 perf stat -e cycles,instructions \
> ./bpftrace -e 'uprobe:./uprobe_multi:uprobe_multi_func_* { }, i:ms:1 { exit(); }'
> ...
>
> 126,666,390,509 cycles
> 29,973,207,307 instructions # 0.24 insn per cycle
>
> 85.284833554 seconds time elapsed
>
>
> Same bpftrace setup with uprobe_multi support:
>
> # perf stat -e cycles,instructions \
> ./bpftrace -e 'uprobe:./uprobe_multi:uprobe_multi_func_* { }, i:ms:1 { exit(); }'
> ...
>
> 6,818,470,649 cycles
> 13,275,510,122 instructions # 1.95 insn per cycle
>
> 1.943269451 seconds time elapsed
>
>
> I'm sending this as RFC because of:
> - I added/exported some new elf_* helper functions in libbpf,
> and I'm not sure that's the best/right way of doing this
didn't get to that yet, sounds suspicious :)
> - I'm not completely sure about the usdt integration in bpf_program__attach_usdt,
> I was trying to detect uprobe_multi kernel support first, but ended up with
> just new field for struct bpf_usdt_opts
haven't gotten to this yet as well, but it has to be auto-detectable,
not an option (at least I don't see why it wouldn't be, but let me get
to the patch)
> - I plan to add more tests for usdt probes defined with refctr
>
>
> Also available at:
> https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> uprobe_multi
>
> There are PRs for tetragon [1] and bpftrace [2] support.
>
> thanks,
> jirka
>
>
> [1] https://github.com/cilium/tetragon/pull/936
> [2] https://github.com/olsajiri/bpftrace/tree/uprobe_multi
>
> Cc: Viktor Malik <viktor.malik@xxxxxxxxx>
> Cc: Daniel Xu <dxu@xxxxxxxxx>
> ---
> Jiri Olsa (20):
> bpf: Add multi uprobe link
> bpf: Add cookies support for uprobe_multi link
> bpf: Add bpf_get_func_ip helper support for uprobe link
> libbpf: Update uapi bpf.h tools header
> libbpf: Add uprobe_multi attach type and link names
> libbpf: Factor elf_for_each_symbol function
> libbpf: Add elf_find_multi_func_offset function
> libbpf: Add elf_find_patern_func_offset function
> libbpf: Add bpf_link_create support for multi uprobes
> libbpf: Add bpf_program__attach_uprobe_multi_opts function
> libbpf: Add support for uprobe.multi/uprobe.multi program sections
> libbpf: Add uprobe multi link support to bpf_program__attach_usdt
> selftests/bpf: Add uprobe_multi skel test
> selftests/bpf: Add uprobe_multi api test
> selftests/bpf: Add uprobe_multi link test
> selftests/bpf: Add uprobe_multi test program
> selftests/bpf: Add uprobe_multi bench test
> selftests/bpf: Add usdt_multi test program
> selftests/bpf: Add usdt_multi bench test
> selftests/bpf: Add uprobe_multi cookie test
>
> include/linux/trace_events.h | 6 +
> include/uapi/linux/bpf.h | 15 +++
> kernel/bpf/syscall.c | 18 ++-
> kernel/trace/bpf_trace.c | 310 +++++++++++++++++++++++++++++++++++++++++++-
> tools/include/uapi/linux/bpf.h | 15 +++
> tools/lib/bpf/bpf.c | 10 ++
> tools/lib/bpf/bpf.h | 10 +-
> tools/lib/bpf/libbpf.c | 653 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
> tools/lib/bpf/libbpf.h | 43 ++++++
> tools/lib/bpf/libbpf.map | 3 +
> tools/lib/bpf/libbpf_internal.h | 2 +-
> tools/lib/bpf/usdt.c | 127 +++++++++++++-----
> tools/testing/selftests/bpf/Makefile | 10 ++
> tools/testing/selftests/bpf/prog_tests/bpf_cookie.c | 78 +++++++++++
> tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c | 286 ++++++++++++++++++++++++++++++++++++++++
> tools/testing/selftests/bpf/progs/uprobe_multi.c | 72 +++++++++++
> tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c | 16 +++
> tools/testing/selftests/bpf/uprobe_multi.c | 53 ++++++++
> tools/testing/selftests/bpf/usdt_multi.c | 23 ++++
> 19 files changed, 1634 insertions(+), 116 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi.c
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c
> create mode 100644 tools/testing/selftests/bpf/uprobe_multi.c
> create mode 100644 tools/testing/selftests/bpf/usdt_multi.c
