Em Thu, Apr 08, 2021 at 04:39:33PM +0000, Song Liu escreveu:
> > On Apr 8, 2021, at 4:47 AM, Jiri Olsa <jo...@redhat.com> wrote:
> > On Tue, Apr 06, 2021 at 05:36:01PM -0700, Song Liu 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. This is limited to hardware events in
> >> evsel__hw_names.
> >>
> >> 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
> >>
> >
> > so if we are mixing events now, how about uing modifier for bpf counters,
> > instead of configuring .perfconfig list we could use:
> >
> > perf stat -e instructions:b,cs
> >
> > thoughts?
> >
> > the change below adds 'b' modifier and sets 'evsel::bpf_counter',
> > feel free to use it
>
> I think we will need both 'b' modifier and .perfconfig configuration.
Agreed, maximum flexibility.
> For systems with BPF-managed perf events running in the background,
> .perfconfig makes sure perf-stat sessions will share PMCs with these
> background monitoring tools. 'b' modifier, on the other hand, is useful
> when the user knows there is opportunity to share the PMCs.
>
> Does this make sense?
I think so.
- Arnaldo
> Thanks,
> Song
>
> >
> > jirka
> >
> >
> > ---
> > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> > index ca52581f1b17..c55e4e58d1dc 100644
> > --- a/tools/perf/util/evsel.h
> > +++ b/tools/perf/util/evsel.h
> > @@ -82,6 +82,7 @@ struct evsel {
> > bool auto_merge_stats;
> > bool collect_stat;
> > bool weak_group;
> > + bool bpf_counter;
> > int bpf_fd;
> > struct bpf_object *bpf_obj;
> > };
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index 9ecb45bea948..b5850f1ea90b 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -1801,6 +1801,7 @@ struct event_modifier {
> > int pinned;
> > int weak;
> > int exclusive;
> > + int bpf_counter;
> > };
> >
> > static int get_event_modifier(struct event_modifier *mod, char *str,
> > @@ -1821,6 +1822,7 @@ static int get_event_modifier(struct event_modifier
> > *mod, char *str,
> > int exclude = eu | ek | eh;
> > int exclude_GH = evsel ? evsel->exclude_GH : 0;
> > int weak = 0;
> > + int bpf_counter = 0;
> >
> > memset(mod, 0, sizeof(*mod));
> >
> > @@ -1864,6 +1866,8 @@ static int get_event_modifier(struct event_modifier
> > *mod, char *str,
> > exclusive = 1;
> > } else if (*str == 'W') {
> > weak = 1;
> > + } else if (*str == 'b') {
> > + bpf_counter = 1;
> > } else
> > break;
> >
> > @@ -1895,6 +1899,7 @@ static int get_event_modifier(struct event_modifier
> > *mod, char *str,
> > mod->sample_read = sample_read;
> > mod->pinned = pinned;
> > mod->weak = weak;
> > + mod->bpf_counter = bpf_counter;
> > mod->exclusive = exclusive;
> >
> > return 0;
> > @@ -1909,7 +1914,7 @@ static int check_modifier(char *str)
> > char *p = str;
> >
> > /* The sizeof includes 0 byte as well. */
> > - if (strlen(str) > (sizeof("ukhGHpppPSDIWe") - 1))
> > + if (strlen(str) > (sizeof("ukhGHpppPSDIWeb") - 1))
> > return -1;
> >
> > while (*p) {
> > @@ -1950,6 +1955,7 @@ int parse_events__modifier_event(struct list_head
> > *list, char *str, bool add)
> > evsel->sample_read = mod.sample_read;
> > evsel->precise_max = mod.precise_max;
> > evsel->weak_group = mod.weak;
> > + evsel->bpf_counter = mod.bpf_counter;
> >
> > if (evsel__is_group_leader(evsel)) {
> > evsel->core.attr.pinned = mod.pinned;
> > diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> > index 0b36285a9435..fb8646cc3e83 100644
> > --- a/tools/perf/util/parse-events.l
> > +++ b/tools/perf/util/parse-events.l
> > @@ -210,7 +210,7 @@ name_tag
> > [\'][a-zA-Z_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\']
> > name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
> > drv_cfg_term [a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
> > /* If you add a modifier you need to update check_modifier() */
> > -modifier_event [ukhpPGHSDIWe]+
> > +modifier_event [ukhpPGHSDIWeb]+
> > modifier_bp [rwx]{1,3}
> >
> > %%
> >
>
--
- Arnaldo
