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

Reply via email to