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

[Jiri Olsa <jolsa@redhat.com>]

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