linux-kernel - Re: [RFC][PATCH 7/7] perf/annotate: Add branch stack / basic block information

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]
Date:   Thu, 8 Sep 2016 09:43:53 -0700
From:   Stephane Eranian <eranian@...gle.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andi Kleen <andi@...stfloor.org>, Jiri Olsa <jolsa@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        David Carrillo-Cisneros <davidcc@...gle.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        "Liang, Kan" <kan.liang@...el.com>,
        Anshuman Khandual <khandual@...ux.vnet.ibm.com>
Subject: Re: [RFC][PATCH 7/7] perf/annotate: Add branch stack / basic block information

Hi,

On Thu, Sep 8, 2016 at 9:18 AM, Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> Em Fri, Jul 08, 2016 at 06:36:32PM +0200, Peter Zijlstra escreveu:
> > On Fri, Jul 08, 2016 at 06:27:33PM +0200, Peter Zijlstra wrote:
> >
> > > I've been thinking of filtering all targets and branches that are
> > > smaller than 0.1% in order to avoid this, but so far I've just been
> > > ignoring these things.
> >
> > Like so... seems to 'work'.
>
> So I merged this one with 7/7 and this is the result, screenshot to
> capture the colors:
>
>   http://vger.kernel.org/~acme/perf/annotate_basic_blocks.png
>
> Please let me know if I should go ahead and push with the combined
> patch, that is now at:
>
> https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/annotate_basic_blocks&id=baf41a43fa439ac534d21e41882a7858d5cee1e5
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/annotate_basic_blocks
>
> Is that ok?
>
I like the idea and yes, branch stack can be used for this, but I have
a hard time understanding the colored output.
What is the explanation for the color changes?
How do I interpret the percentages in the comments of the assembly:
-54.50% (p: 42%)
Why not have dedicated columns before the assembly with proper column headers?

As for the command line:

  $ perf record -b my_workload

Will do it right, for both kernel and user by default.

If you want user level only, you can simply do:

  $ perf record -b -e cycles:up my_workload

The branch stack inherit the priv level of the event automatically.

>
> The problem with it is that it is done only for --stdio, I'll check how
> to properly make it UI agnostic...
>
> - Arnaldo
>
>
> > ---
> >  tools/perf/util/annotate.c | 45 ++++++++++++++++++++++++++-------------------
> >  1 file changed, 26 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > index 8eeb151..c78b16f0 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -907,6 +907,7 @@ static void annotate__branch_printf(struct block_range *br, u64 addr)
> >  #if 1
> >       if (br->is_target && br->start == addr) {
> >               struct block_range *branch = br;
> > +             double p;
> >
> >               /*
> >                * Find matching branch to our target.
> > @@ -914,31 +915,37 @@ static void annotate__branch_printf(struct block_range *br, u64 addr)
> >               while (!branch->is_branch)
> >                       branch = block_range__next(branch);
> >
> > -             if (emit_comment) {
> > -                     emit_comment = false;
> > -                     printf("\t#");
> > -             }
> > +             p = 100 *(double)br->entry / branch->coverage;
> >
> > -             /*
> > -              * The percentage of coverage joined at this target in relation
> > -              * to the next branch.
> > -              */
> > -             printf(" +%.2f%%", 100*(double)br->entry / branch->coverage);
> > +             if (p > 0.1) {
> > +                     if (emit_comment) {
> > +                             emit_comment = false;
> > +                             printf("\t#");
> > +                     }
> > +
> > +                     /*
> > +                      * The percentage of coverage joined at this target in relation
> > +                      * to the next branch.
> > +                      */
> > +                     printf(" +%.2f%%", p);
> > +             }
> >       }
> >  #endif
> >       if (br->is_branch && br->end == addr) {
> > +             double p = 100*(double)br->taken / br->coverage;
> >
> > -             if (emit_comment) {
> > -                     emit_comment = false;
> > -                     printf("\t#");
> > -             }
> > +             if (p > 0.1) {
> > +                     if (emit_comment) {
> > +                             emit_comment = false;
> > +                             printf("\t#");
> > +                     }
> >
> > -             /*
> > -              * The percentage of coverage leaving at this branch, and
> > -              * its prediction ratio.
> > -              */
> > -             printf(" -%.2f%% / %.2f%%", 100*(double)br->taken / br->coverage,
> > -                                         100*(double)br->pred  / br->taken);
> > +                     /*
> > +                      * The percentage of coverage leaving at this branch, and
> > +                      * its prediction ratio.
> > +                      */
> > +                     printf(" -%.2f%% (p:%.2f%%)", p, 100*(double)br->pred  / br->taken);
> > +             }
> >       }
> >  }
> >