Messages in this thread | | | From | Song Liu <> | Subject | Re: [PATCH v3 3/4] perf-stat: introduce config stat.bpf-counter-events | Date | Mon, 19 Apr 2021 20:26:02 +0000 |
| |
> On Apr 17, 2021, at 9:45 AM, Namhyung Kim <namhyung@kernel.org> wrote: > > Hi Song, > > On Sat, Apr 17, 2021 at 7:13 AM Song Liu <song@kernel.org> wrote: >> >> 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 <song@kernel.org> >> --- >> @@ -535,12 +549,13 @@ static int enable_counters(void) >> struct evsel *evsel; >> int err; >> >> - if (target__has_bpf(&target)) { >> - 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; > > I just realized it doesn't have a disable counterpart.
I guess it is not really necessary for perf-stat? It is probably good to have it though. How about we do it in a follow up patch?
[...]
>> + if (!evsel__bpf_counter_events) >> + return false; >> + >> + ptr = strstr(evsel__bpf_counter_events, name); >> + name_len = strlen(name); >> + >> + /* check name matches a full token in evsel__bpf_counter_events */ >> + match = (ptr != NULL) && >> + ((ptr == evsel__bpf_counter_events) || (*(ptr - 1) == ',')) && >> + ((*(ptr + name_len) == ',') || (*(ptr + name_len) == '\0')); > > I'm not sure we have an event name which is a substring of another. > Maybe it can retry if it fails to match.
We have ref-cycles and cycles. And some raw events may be substring of others?
Thanks, Song
[...]
| |