On Wed, Nov 24, 2021 at 12:41 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote: > > Adding support to create multiple probes within single perf event. > This way we can associate single bpf program with multiple kprobes, > because bpf program gets associated with the perf event. > > The perf_event_attr is not extended, current fields for kprobe > attachment are used for multi attachment. I'm a bit concerned with complicating perf_event_attr further to support this multi-attach. For BPF, at least, we now have bpf_perf_link and corresponding BPF_LINK_CREATE command in bpf() syscall which allows much simpler and cleaner API to do this. Libbpf will actually pick bpf_link-based attachment if kernel supports it. I think we should better do bpf_link-based approach from the get go. Another thing I'd like you to keep in mind and think about is BPF cookie. Currently kprobe/uprobe/tracepoint allow to associate arbitrary user-provided u64 value which will be accessible from BPF program with bpf_get_attach_cookie(). With multi-attach kprobes this because extremely crucial feature to support, otherwise it's both expensive, inconvenient and complicated to be able to distinguish between different instances of the same multi-attach kprobe invocation. So with that, what would be the interface to specify these BPF cookies for this multi-attach kprobe, if we are going through perf_event_attr. Probably picking yet another unused field and union-izing it with a pointer. It will work, but makes the interface even more overloaded. While for LINK_CREATE we can just add another pointer to a u64[] with the same size as number of kfunc names and offsets. But other than that, I'm super happy that you are working on these complicated multi-attach capabilities! It would be great to benchmark one-by-one attachment vs multi-attach to the same set of kprobes once you arrive at the final implementation. > > For current kprobe atachment we use either: > > kprobe_func (in config1) + probe_offset (in config2) > > to define kprobe by function name with offset, or: > > kprobe_addr (in config2) > > to define kprobe with direct address value. > > For multi probe attach the same fields point to array of values > with the same semantic. Each probe is defined as set of values > with the same array index (idx) as: > > kprobe_func[idx] + probe_offset[idx] > > to define kprobe by function name with offset, or: > > kprobe_addr[idx] > > to define kprobe with direct address value. > > The number of probes is passed in probe_cnt value, which shares > the union with wakeup_events/wakeup_watermark values which are > not used for kprobes. > > Since [1] it's possible to stack multiple probes events under > one head event. Using the same code to allow that for probes > defined under perf kprobe interface. > > [1] https://lore.kernel.org/lkml/156095682948.28024.14190188071338900568.stgit@devnote2/ > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx> > --- > include/uapi/linux/perf_event.h | 1 + > kernel/trace/trace_event_perf.c | 106 ++++++++++++++++++++++++++++---- > kernel/trace/trace_kprobe.c | 47 ++++++++++++-- > kernel/trace/trace_probe.c | 2 +- > kernel/trace/trace_probe.h | 3 +- > 5 files changed, 138 insertions(+), 21 deletions(-) > [...]