Re: [PATCH v4 3/4] perf-stat: introduce config stat.bpf-counter-events
> On Apr 20, 2021, at 10:31 AM, Jiri Olsa wrote:
>
> On Mon, Apr 19, 2021 at 01:36:48PM -0700, Song Liu wrote:
>
> SNIP
>
>> if (stat_config.initial_delay < 0) {
>> @@ -784,11 +790,11 @@ static int __run_perf_stat(int argc, const char
>> **argv, int run_idx)
>> if (affinity__setup() < 0)
>> return -1;
>>
>> -if (target__has_bpf()) {
>> -evlist__for_each_entry(evsel_list, counter) {
>> -if (bpf_counter__load(counter, ))
>> -return -1;
>> -}
>> +evlist__for_each_entry(evsel_list, counter) {
>> +if (bpf_counter__load(counter, ))
>> +return -1;
>> +if (!evsel__is_bpf(counter))
>> +all_counters_use_bpf = false;
>
> could be done in bpf_counter__load, check below:
>
>> }
>>
>> evlist__for_each_cpu (evsel_list, i, cpu) {
>> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
>> index 5de991ab46af9..33b1888103dfa 100644
>> --- a/tools/perf/util/bpf_counter.c
>> +++ b/tools/perf/util/bpf_counter.c
>> @@ -790,7 +790,8 @@ int bpf_counter__load(struct evsel *evsel, struct target
>> *target)
>> {
>> if (target->bpf_str)
>> evsel->bpf_counter_ops = _program_profiler_ops;
>> -else if (target->use_bpf)
>> +else if (target->use_bpf ||
>> + evsel__match_bpf_counter_events(evsel->name))
>> evsel->bpf_counter_ops = _ops;
>
> with:
> else
> all_counters_use_bpf = false;
>
> I was also thinking of oving it to evlist, but it's sat specific,
> so I think it's good as static.. thanks for changing the implementation
Hmm... then we need to somehow make all_counters_use_bpf visible in
bpf_counter.c, which won't be very clean. Also, since this is stat
specific, I guess it is better to keep it inside builtin-stat.c?
The runtime overhead should be minimal.
Thanks,
Song
Re: [PATCH v4 3/4] perf-stat: introduce config stat.bpf-counter-events
On Mon, Apr 19, 2021 at 01:36:48PM -0700, Song Liu wrote:
SNIP
> if (stat_config.initial_delay < 0) {
> @@ -784,11 +790,11 @@ static int __run_perf_stat(int argc, const char **argv,
> int run_idx)
> if (affinity__setup() < 0)
> return -1;
>
> - if (target__has_bpf()) {
> - evlist__for_each_entry(evsel_list, counter) {
> - if (bpf_counter__load(counter, ))
> - return -1;
> - }
> + evlist__for_each_entry(evsel_list, counter) {
> + if (bpf_counter__load(counter, ))
> + return -1;
> + if (!evsel__is_bpf(counter))
> + all_counters_use_bpf = false;
could be done in bpf_counter__load, check below:
> }
>
> evlist__for_each_cpu (evsel_list, i, cpu) {
> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> index 5de991ab46af9..33b1888103dfa 100644
> --- a/tools/perf/util/bpf_counter.c
> +++ b/tools/perf/util/bpf_counter.c
> @@ -790,7 +790,8 @@ int bpf_counter__load(struct evsel *evsel, struct target
> *target)
> {
> if (target->bpf_str)
> evsel->bpf_counter_ops = _program_profiler_ops;
> - else if (target->use_bpf)
> + else if (target->use_bpf ||
> + evsel__match_bpf_counter_events(evsel->name))
> evsel->bpf_counter_ops = _ops;
with:
else
all_counters_use_bpf = false;
I was also thinking of oving it to evlist, but it's sat specific,
so I think it's good as static.. thanks for changing the implementation
jirka
>
> if (evsel->bpf_counter_ops)
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 6bcb5ef221f8c..63d472b336de2 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -18,6 +18,7 @@
> #include "util/hist.h" /* perf_hist_config */
> #include "util/llvm-utils.h" /* perf_llvm_config */
> #include "util/stat.h" /* perf_stat__set_big_num */
> +#include "util/evsel.h" /* evsel__hw_names, evsel__use_bpf_counters */
> #include "build-id.h"
> #include "debug.h"
> #include "config.h"
> @@ -460,6 +461,9 @@ static int perf_stat_config(const char *var, const char
> *value)
> if (!strcmp(var, "stat.no-csv-summary"))
> perf_stat__set_no_csv_summary(perf_config_bool(var, value));
>
> + if (!strcmp(var, "stat.bpf-counter-events"))
> + evsel__bpf_counter_events = strdup(value);
> +
> /* Add other config variables here. */
> return 0;
> }
SNIP
[PATCH v4 3/4] perf-stat: introduce config stat.bpf-counter-events
Currently, to use BPF to aggregate perf event counters, the user uses
--bpf-counters option. Enable "use bpf by default" events with a config
option, stat.bpf-counter-events. Events with name in the option will use
BPF.
This also enables mixed BPF event and regular event in the same sesssion.
For example:
perf config stat.bpf-counter-events=instructions
perf stat -e instructions,cs
The second command will use BPF for "instructions" but not "cs".
Signed-off-by: Song Liu
---
tools/perf/Documentation/perf-stat.txt | 2 ++
tools/perf/builtin-stat.c | 40 +++---
tools/perf/util/bpf_counter.c | 3 +-
tools/perf/util/config.c | 4 +++
tools/perf/util/evsel.c| 22 ++
tools/perf/util/evsel.h| 8 ++
tools/perf/util/target.h | 5
7 files changed, 61 insertions(+), 23 deletions(-)
diff --git a/tools/perf/Documentation/perf-stat.txt
b/tools/perf/Documentation/perf-stat.txt
index 6ec5960b08c3d..f10e24da23e90 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -97,6 +97,8 @@ report::
Use BPF programs to aggregate readings from perf_events. This
allows multiple perf-stat sessions that are counting the same metric
(cycles,
instructions, etc.) to share hardware counters.
+ To use BPF programs on common events by default, use
+ "perf config stat.bpf-counter-events=".
--bpf-attr-map::
With option "--bpf-counters", different perf-stat sessions share
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 2a2c15cac80a3..157105e792eaf 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -160,6 +160,7 @@ static const char *smi_cost_attrs = {
};
static struct evlist *evsel_list;
+static bool all_counters_use_bpf = true;
static struct target target = {
.uid= UINT_MAX,
@@ -399,6 +400,9 @@ static int read_affinity_counters(struct timespec *rs)
struct affinity affinity;
int i, ncpus, cpu;
+ if (all_counters_use_bpf)
+ return 0;
+
if (affinity__setup() < 0)
return -1;
@@ -413,6 +417,8 @@ static int read_affinity_counters(struct timespec *rs)
evlist__for_each_entry(evsel_list, counter) {
if (evsel__cpu_iter_skip(counter, cpu))
continue;
+ if (evsel__is_bpf(counter))
+ continue;
if (!counter->err) {
counter->err = read_counter_cpu(counter, rs,
counter->cpu_iter - 1);
@@ -429,6 +435,9 @@ static int read_bpf_map_counters(void)
int err;
evlist__for_each_entry(evsel_list, counter) {
+ if (!evsel__is_bpf(counter))
+ continue;
+
err = bpf_counter__read(counter);
if (err)
return err;
@@ -439,14 +448,10 @@ static int read_bpf_map_counters(void)
static void read_counters(struct timespec *rs)
{
struct evsel *counter;
- int err;
if (!stat_config.stop_read_counter) {
- if (target__has_bpf())
- err = read_bpf_map_counters();
- else
- err = read_affinity_counters(rs);
- if (err < 0)
+ if (read_bpf_map_counters() ||
+ read_affinity_counters(rs))
return;
}
@@ -535,12 +540,13 @@ static int enable_counters(void)
struct evsel *evsel;
int err;
- if (target__has_bpf()) {
- evlist__for_each_entry(evsel_list, evsel) {
- err = bpf_counter__enable(evsel);
- if (err)
- return err;
- }
+ evlist__for_each_entry(evsel_list, evsel) {
+ if (!evsel__is_bpf(evsel))
+ continue;
+
+ err = bpf_counter__enable(evsel);
+ if (err)
+ return err;
}
if (stat_config.initial_delay < 0) {
@@ -784,11 +790,11 @@ static int __run_perf_stat(int argc, const char **argv,
int run_idx)
if (affinity__setup() < 0)
return -1;
- if (target__has_bpf()) {
- evlist__for_each_entry(evsel_list, counter) {
- if (bpf_counter__load(counter, ))
- return -1;
- }
+ evlist__for_each_entry(evsel_list, counter) {
+ if (bpf_counter__load(counter, ))
+ return -1;
+ if (!evsel__is_bpf(counter))
+ all_counters_use_bpf = false;
}
evlist__for_each_cpu (evsel_list, i, cpu) {
diff --git
