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

[Song Liu]

> 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. 

> 
>> .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? 

Thanks,
Song