linux-kernel - Re: [PATCH 6/9] perf report: Display the branch counter histogram

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM9d7cgE=cDyeOyXrRZt53vKD=FRSqQRMz8=f=bGT-gzm2jjkg@mail.gmail.com>
Date: Tue, 6 Aug 2024 16:29:19 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: "Liang, Kan" <kan.liang@...ux.intel.com>
Cc: acme@...nel.org, irogers@...gle.com, peterz@...radead.org, 
	mingo@...nel.org, linux-kernel@...r.kernel.org, adrian.hunter@...el.com, 
	ak@...ux.intel.com, eranian@...gle.com
Subject: Re: [PATCH 6/9] perf report: Display the branch counter histogram

On Tue, Aug 6, 2024 at 7:40 AM Liang, Kan <kan.liang@...ux.intel.com> wrote:
>
>
>
> On 2024-08-02 8:18 p.m., Namhyung Kim wrote:
> > On Wed, Jul 3, 2024 at 1:03 PM <kan.liang@...ux.intel.com> wrote:
> >>
> >> From: Kan Liang <kan.liang@...ux.intel.com>
> >>
> >> Reusing the existing --total-cycles option to display the branch
> >> counters. Add a new PERF_HPP_REPORT__BLOCK_BRANCH_COUNTER to display
> >> the logged branch counter events. They are shown right after all the
> >> cycle-related annotations.
> >> Extend the struct block_info to store and pass the branch counter
> >> related information.
> >>
> >> The annotation_br_cntr_entry() is to print the histogram of each branch
> >> counter event.
> >> The annotation_br_cntr_abbr_list() prints the branch counter's
> >> abbreviation list. Press 'B' to display the list in the TUI mode.
> >>
> >> $perf record -e "{branch-instructions:ppp,branch-misses}:S" -j any,counter
> >> $perf report  --total-cycles --stdio
> >>
> >>  # To display the perf.data header info, please use --header/--header-only options.
> >>  #
> >>  #
> >>  # Total Lost Samples: 0
> >>  #
> >>  # Samples: 1M of events 'anon group { branch-instructions:ppp, branch-misses }'
> >>  # Event count (approx.): 1610046
> >>  #
> >>  # Branch counter abbr list:
> >>  # branch-instructions:ppp = A
> >>  # branch-misses = B
> >>  # '-' No event occurs
> >>  # '+' Event occurrences may be lost due to branch counter saturated
> >>  #
> >>  # Sampled Cycles%  Sampled Cycles  Avg Cycles%  Avg Cycles          Branch Counter [Program Block Range]
> >>  # ...............  ..............  ...........  ..........  ......................  ..................
> >>  #
> >>            57.55%            2.5M        0.00%           3             |A   |-   |                 ...
> >>            25.27%            1.1M        0.00%           2             |AA  |-   |                 ...
> >>            15.61%          667.2K        0.00%           1             |A   |-   |                 ...
> >>             0.16%            6.9K        0.81%         575             |A   |-   |                 ...
> >>             0.16%            6.8K        1.38%         977             |AA  |-   |                 ...
> >>             0.16%            6.8K        0.04%          28             |AA  |B   |                 ...
> >>             0.15%            6.6K        1.33%         946             |A   |-   |                 ...
> >>             0.11%            4.5K        0.06%          46             |AAA+|-   |                 ...
> >>             0.10%            4.4K        0.88%         624             |A   |-   |                 ...
> >>             0.09%            3.7K        0.74%         524             |AAA+|B   |                 ...
> >
> > I think this format assumes short width and might not work
> > well when it has more events with bigger width.  Maybe
> > A=<n>, B=<n> ?
>
> The purpose of "AAA" is to print a histogram here which can give the end
> user a straightforward image of the distribution. The A=<n> may not be
> that obvious.

I understand your point.  But I think we need to provide an easily
parse-able format at least for CSV output.

>
> I don't think there is a plan to increase the saturation of the counter.
> So 4 bits of width should last for a long time. Other ARCHs don't have
> such a feature either. I think I can the change the code to force the 4
> bits of width now. For more that 3 events, the perf tool can convert it
> to a "+". We may update the perf tool for a more specific histogram
> later, if the saturation is changed. What do you think?

Ok, 4 bits width is probably fine.  How many events can a LBR entry
support?  Maybe that's limited by the number of HW counters but
theoretically 64 / 4 = 16, right?

Thanks,
Namhyung