> On Nov 24, 2020, at 11:51 AM, Jiri Olsa <[email protected]> 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
