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

[Song Liu]

> On Apr 8, 2021, at 11:50 AM, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:
> 
> Em Thu, Apr 08, 2021 at 08:24:47PM +0200, Jiri Olsa escreveu:
>> 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?
> 
> I.e. do they use tools/lib/perf/? :-)

Not right now. I do hope we can eventually let them use libperf. But I 
haven't figured out the best path forward. 

> 
> I guess they will use it now for getting that "struct perf_event_attr_map_entry" and
> the map name define.
> 
>>> "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?
> 
> He wants users not to bother if they are using bpf based counters, this will happen
> automagically after they set their ~/.perfconfig with some command line Song provides.
> 
> Then they will be using bpf counters that won't get exclusive access to those
> scarce counters, the tooling they are using will use bpf-counters and all will
> be well.
> 
> Right Song?

Yes, exactly. The config automatically switches ad-hoc perf-stat runs (for debug, 
performance tuning, etc.) to bpf managed counters. 

Thanks,
Song