[-next,0/2] perf stat: a set of small fixes for bperf

Message

Tengda Wu Sept. 25, 2024, 1:55 p.m. UTC

Hi,

This is a set of small fixes for bperf (perf-stat --bpf-counters).

It aims to fix the following two issues:
  1) bperf limited the number of events to a maximum of 16, which
     caused failures in some scenarios and lacked friendly prompts. 
  2) bperf failed to correctly handle whether events were supported,
     resulting in the incorrect display when the event count was 0.

The reason for fixing these issues is that bperf is very useful in
some cost-sensitive scenarios, such as top-down analysis scenarios.
Increasing the attr map size can allow these scenarios to collect
more events at the same time, making it possible to gather enough
information to perform complex metric calculations in top-down.

Thanks,
Tengda

Tengda Wu (2):
  perf stat: Increase perf_attr_map entries
  perf stat: Fix incorrect display of bperf when event count is 0

 tools/lib/perf/include/perf/bpf_perf.h |  1 +
 tools/perf/util/bpf_counter.c          | 26 +++++++++++++++++---------
 2 files changed, 18 insertions(+), 9 deletions(-)

Comments

Namhyung Kim Oct. 9, 2024, 5:20 a.m. UTC | #1

Hello,

On Wed, Sep 25, 2024 at 01:55:21PM +0000, Tengda Wu wrote:
> Hi,
> 
> This is a set of small fixes for bperf (perf-stat --bpf-counters).
> 
> It aims to fix the following two issues:
>   1) bperf limited the number of events to a maximum of 16, which
>      caused failures in some scenarios and lacked friendly prompts. 
>   2) bperf failed to correctly handle whether events were supported,
>      resulting in the incorrect display when the event count was 0.
> 
> The reason for fixing these issues is that bperf is very useful in
> some cost-sensitive scenarios, such as top-down analysis scenarios.
> Increasing the attr map size can allow these scenarios to collect
> more events at the same time, making it possible to gather enough
> information to perform complex metric calculations in top-down.

So I tried this patchset and found a couple of issues.

1. Running bperf failed due to perf_event_attr map locking issue.
   It seems that the message is misleading since it didn't to get the
   lock actually.  I think it failed the map is not compatible anymore
   and the error message should say about that.

2. It seems bperf keeps the map pinned in the file system.  I'm not sure
   if we have an option to unpin the map automatically.  I had to find
   the path and delete it manually.

3. Currently bperf doesn't support event groups.  On Intel machines,
   topdown metrics are enabled by default and it makes the perf stat
   failing when --bpf-counters option is used (after the step 2).

    $ sudo ./perf stat -a --bpf-counters true
    bpf managed perf events do not yet support groups.

   Maybe we need to drop the topdown metrics when bperf fails.

Thanks,
Namhyung

> 
> Thanks,
> Tengda
> 
> Tengda Wu (2):
>   perf stat: Increase perf_attr_map entries
>   perf stat: Fix incorrect display of bperf when event count is 0
> 
>  tools/lib/perf/include/perf/bpf_perf.h |  1 +
>  tools/perf/util/bpf_counter.c          | 26 +++++++++++++++++---------
>  2 files changed, 18 insertions(+), 9 deletions(-)
> 
> -- 
> 2.34.1
>