On 07/04/2019 02:57 AM, Andrii Nakryiko wrote: > On Wed, Jul 3, 2019 at 9:47 AM Andrii Nakryiko > <andrii.nakryiko@xxxxxxxxx> wrote: [...] >> [1] https://lore.kernel.org/bpf/20190621045555.4152743-4-andriin@xxxxxx/T/#m6cfc141e7b57970bc948134bf671a46972b95134 >> >>> with bpf_link with destructor looks good to me, but my feedback from back then was >>> that all the kprobe/uprobe/tracepoint/raw_tracepoint should be split API-wise, so >>> you'll end up with something like the below, that is, 1) a set of functions that >>> only /create/ the bpf_link handle /once/, and 2) a helper that allows /attaching/ >>> progs to one or multiple bpf_links. The set of APIs would look like: >>> >>> struct bpf_link *bpf_link__create_kprobe(bool retprobe, const char *func_name); >>> struct bpf_link *bpf_link__create_uprobe(bool retprobe, pid_t pid, >>> const char *binary_path, >>> size_t func_offset); >>> int bpf_program__attach_to_link(struct bpf_link *link, struct bpf_program *prog); >>> int bpf_link__destroy(struct bpf_link *link); >>> >>> This seems much more natural to me. Right now you sort of do both in one single API. >> >> It felt that way for me as well, until I implemented it and used it in >> selftests. And then it felt unnecessarily verbose without giving any >> benefit. I still have a local patchset with that change, I can post it >> as RFC, if you don't trust my judgement. Please let me know. >> >>> Detangling the bpf_program__attach_{uprobe,kprobe}() would also avoid that you have >>> to redo all the perf_event_open_probe() work over and over in order to get the pfd > > So re-reading this again, I wonder if you meant that with separate > bpf_link (or rather bpf_hook in that case) creation and attachment > operations, one would be able to create single bpf_hook for same > kprobe and then attach multiple BPF programs to that single pfd > representing that specific probe. > > If that's how I should have read it, I agree that it probably would be > possible for some types of hooks, but not for every type of hook. But > furthermore, how often in practice same application attaches many > different BPF programs to the same hook? And it's also hard to imagine > that hook creation (i.e., creating such FD for BPF hook), would ever > be a bottleneck. > > So I still think it's not a strong reason to go with API that's harder > to use for typical use cases just because of hypothetical benefits in > some extreme cases. Was thinking along that lines, yes, as we run over an array of BPF progs, but I just double checked the kernel code again and the relationship of a BPF prog to perf_event is really just 1:1, just that the backing tp_event (trace_event_call) contains the shared array. Given that, all makes sense and there is no point in splitting. Therefore, applied, thanks!
