linux-kernel - Re: [PATCH -next 0/2] perf stat: a set of small fixes for bperf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZwYSkJqKRmB5NXrx@google.com>
Date: Tue, 8 Oct 2024 22:20:16 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Tengda Wu <wutengda@...weicloud.com>
Cc: Peter Zijlstra <peterz@...radead.org>, song@...nel.org,
	Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>, kan.liang@...ux.intel.com,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
	bpf@...r.kernel.org
Subject: Re: [PATCH -next 0/2] perf stat: a set of small fixes for bperf

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
>