Re: [PATCH 09/20] perf tools: Parse probe points of eBPF programs during preparation

Re: [PATCH 09/20] perf tools: Parse probe points of eBPF programs during preparation

[Arnaldo Carvalho de Melo]

Adding back lkml and Ingo, noticed it wasn't there :-\

Em Fri, Aug 07, 2015 at 01:27:39PM +0800, Wangnan (F) escreveu:
> On 2015/8/6 23:50, Arnaldo Carvalho de Melo wrote:
> >Em Tue, Aug 04, 2015 at 06:28:38AM +0000, Wang Nan escreveu:
> >>This patch parses section name of each program, and creates
> >>corresponding 'struct perf_probe_event' structure.
> >>
> >>parse_perf_probe_command() is used to do the main parsing works.
> >>Parsing result is stored into a global array. This is because
> >>add_perf_probe_events() is non-reentrantable. In following patch,
> >>add_perf_probe_events will be introduced to insert kprobes. It accepts
> >>an array of 'struct perf_probe_event' and do all works in one call.
> >>
> >>Define PERF_BPF_PROBE_GROUP as "perf_bpf_probe", which will be used
> >>as group name of all eBPF probing points.
> >>
> >>This patch utilizes bpf_program__set_private(), bind perf_probe_event
> >>with bpf program by private field.
> >This patch reports errors via pr_err(), which is suboptimal because it
> >assumes that the interface to these facilities use a stdout UI.
> 
> Can I fix it using one patch after all staffs in
> tools/perf/utils/bpf-loader.c
> goes into your tree?

Sure
 
> >I guess we can have it as is now, because the initial use cases is stdio
> >based, but we need to move this to use bpf__strerror() routines like we
> >do elsewhere in tools/perf/, i.e. see:
> >
> >int perf_evlist__strerror_open(struct perf_evlist *evlist __maybe_unused,
> >                                int err, char *buf, size_t size)
> >
> >int perf_evlist__strerror_mmap(struct perf_evlist *evlist, int err,
> >			       char *buf, size_t size)
> >
> >I.e. we have multiple CLASS__strerror_API to map errors we return (that
> >int err) to strings that we store in buf, following the strerror_r()
> >model.
> >
> >I.e. these libraries should not emit errors, at most they can emit log
> >messages (pr_debug), but direct user interaction should be done via UI
> >code (tools/perf/ui/) and tools/perf/builtin-foo.c files.
> >
> >Also the global thing suckz rockz, again, may be acceptable at this
> >point due to friction with how the existing probing routines are
> >structured, but we need to make those functions better, not work around
> >what seems to be limitations they have.
> 
> Also for this one, since it is a fundamental problem in probe related
> code. However, I think global thing can be avoided by moving this patch
> after 'perf probe: Attach trace_probe_event with perf_probe_event',
> config programs at the head of bpf__probe().

Right, I'm preparing a pull req for up to the 'perf test LLVM' patch,
which I think we went thru enough iterations, that way we erode this
patchkit a bit more, as 4 patches are already in tip.git, from
yesterday's pull req I sent to Ingo, yay!

> Thank you.

Thanks!

- Arnaldo