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

From: Jiri Olsa
Date: Thu Apr 08 2021 - 14:24:56 EST


On Thu, Apr 08, 2021 at 06:08:20PM +0000, Song Liu wrote:
>
>
> > On Apr 8, 2021, at 10:45 AM, Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> >
> > On Thu, Apr 08, 2021 at 05:28:10PM +0000, Song Liu wrote:
> >>
> >>
> >>> On Apr 8, 2021, at 10:20 AM, Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> >>>
> >>> On Thu, Apr 08, 2021 at 04:39:33PM +0000, Song Liu wrote:
> >>>>
> >>>>
> >>>>> On Apr 8, 2021, at 4:47 AM, Jiri Olsa <jolsa@xxxxxxxxxx> 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.
> >>>> For systems with BPF-managed perf events running in the background,
> >>>
> >>> hum, I'm not sure I understand what that means.. you mean there
> >>> are tools that run perf stat so you don't want to change them?
> >>
> >> We have tools that do perf_event_open(). I will change them to use
> >> BPF managed perf events for "cycles" and "instructions". Since these
> >> tools are running 24/7, perf-stat on the system should use BPF managed
> >> "cycles" and "instructions" by default.
> >
> > well if you are already changing the tools why not change them to add
> > modifier.. but I don't mind adding that .perfconfig stuff if you need
> > that
>
> The tools I mentioned here don't use perf-stat, they just use
> perf_event_open() and read the perf events fds. We want a config to make

just curious, how those tools use perf_event_open?

> "cycles" to use BPF by default, so that when the user (not these tools)
> runs perf-stat, it will share PMCs with those events by default.

I'm sorry but I still don't see the usecase.. if you need to change both tools,
you can change them to use bpf-managed event, why bother with the list?

> >
> >>
> >>>
> >>>> .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?
> >>>
> >>> if there's reason for that then sure.. but let's not limit that just
> >>> on HARDWARE events only.. there are RAW events with the same demand
> >>> for this feature.. why don't we let user define any event for this?
> >>
> >> I haven't found a good way to config RAW events. I guess RAW events
> >> could use 'b' modifier?
> > any event uing the pmu notation like cpu/instructions/
>
> Can we do something like "perf config stat.bpf-counter-events=cpu/*" means
> all "cpu/xx" events use BPF by default?

I think there's misundestanding, all I'm saying is that IIUC you check
events stat.bpf-counter-events to be HARDWARE type, which I don't think
is necessary and we can allow any event in there

jirka