On Tue, Nov 30, 2021 at 10:53 PM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > 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. Oh, and to be clear, I'm not proposing to bypass underlying perf infra. Rather use it directly as an internal API, not through perf_event_open syscall. > > 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(-) > > > > [...]
