Re: [PATCH v3 3/4] perf-stat: introduce config stat.bpf-counter-events

Jiri Olsa
2021-04-19T14:26:46Z

On Fri, Apr 16, 2021 at 03:13:24PM -0700, Song Liu wrote:

SNIP

> +/*
> + * Returns:
> + * 0 if all events use BPF;
> + * 1 if some events do NOT use BPF;
> + * < 0 on errors;
> + */
> static int read_bpf_map_counters(void)
> {
> + bool has_none_bpf_events = false;
> struct evsel *counter;
> int err;
>
> evlist__for_each_entry(evsel_list, counter) {
> + if (!evsel__is_bpf(counter)) {
> + has_none_bpf_events = true;
> + continue;
> + }
> err = bpf_counter__read(counter);
> if (err)
> return err;
> }
> - return 0;
> + return has_none_bpf_events ? 1 : 0;
> }
>
> static void read_counters(struct timespec *rs)
> @@ -442,9 +455,10 @@ static void read_counters(struct timespec *rs)
> int err;
>
> if (!stat_config.stop_read_counter) {
> - if (target__has_bpf(&target))
> - err = read_bpf_map_counters();
> - else
> + err = read_bpf_map_counters();
> + if (err < 0)
> + return;
> + if (err)
> err = read_affinity_counters(rs);

this part is confusing for me.. I understand we don't want to enter
read_affinity_counters when there's no bpf counter, so we don't set
affinities in vain.. but there must be better way ;-)

> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> index 5de991ab46af9..3189b63714371 100644
> --- a/tools/perf/util/bpf_counter.c
> +++ b/tools/perf/util/bpf_counter.c
> @@ -792,6 +792,8 @@ int bpf_counter__load(struct evsel *evsel, struct target *target)
> evsel->bpf_counter_ops = &bpf_program_profiler_ops;
> else if (target->use_bpf)
> evsel->bpf_counter_ops = &bperf_ops;
> + else if (evsel__match_bpf_counter_events(evsel->name))
> + evsel->bpf_counter_ops = &bperf_ops;

please put this with the target->use_bpf check,
it seems like it's another thing

thanks,
jirka