Messages in this thread | | | From | Song Liu <> | Subject | Re: [RFC 2/2] perf-stat: enable counting events for BPF programs | Date | Tue, 24 Nov 2020 23:43:40 +0000 |
| |
> On Nov 24, 2020, at 11:51 AM, Jiri Olsa <jolsa@redhat.com> wrote: > > On Wed, Nov 18, 2020 at 08:50:46PM -0800, Song Liu wrote: > > SNIP > >> +static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu, >> + int fd) >> +{ >> + struct bpf_prog_profiler_bpf *skel = evsel->bpf_counter.skel; >> + >> + return bpf_map_update_elem(bpf_map__fd(skel->maps.events), >> + &cpu, &fd, BPF_ANY); >> +} >> + >> +struct bpf_counter_ops bpf_program_profiler_ops = { >> + .load = bpf_program_profiler__load, >> + .enable = bpf_program_profiler__enable, >> + .read = bpf_program_profiler__read, >> + .destroy = bpf_program_profiler__destroy, >> + .install_pe = bpf_program_profiler__install_pe, >> +}; > > hum, what's the point of this ops? you plan some other ops? > we could just define stat callbacks right?
I do have other ideas using BPF program to aggregate perf event counts. This ops provides common interface for different BPF extensions to evsel.
> >> +SEC("fentry/XXX") >> +int BPF_PROG(fentry_XXX) >> +{ >> + u32 key = bpf_get_smp_processor_id(); >> + struct bpf_perf_event_value reading; >> + struct bpf_perf_event_value *ptr; >> + u32 zero = 0; >> + long err; >> + >> + /* look up before reading, to reduce error */ >> + ptr = bpf_map_lookup_elem(&fentry_readings, &zero); >> + if (!ptr) >> + return 0; >> + >> + err = bpf_perf_event_read_value(&events, key, &reading, >> + sizeof(reading)); >> + if (err) >> + return 0; >> + >> + *ptr = reading; >> + return 0; >> +} > > so currently it's one extra bpf program for each event, > but it seems bad for multiple events stat.. could we > just have one program that would read and process all events?
Multiple fentry programs should not be too expensive. Current design extends evsel, so it is a cleaner implementation. We can evaluate the difference of these two designs by comparing this with "bpftool prog profile".
Thanks, Song
| |