linux-kernel - Re: [PATCH] perf report: distinguish between inliners in the same function

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]

Date:   Tue, 02 May 2017 11:19:16 +0200
From:   Milian Wolff <milian.wolff@...b.com>
To:     "Jin, Yao" <yao.jin@...ux.intel.com>
Cc:     Linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        David Ahern <dsahern@...il.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH] perf report: distinguish between inliners in the same function

On Tuesday, May 2, 2017 4:11:14 AM CEST Jin, Yao wrote:
> SNIP
> 
> > ~~~~~
> > $ perf report --stdio --inline --no-children
> > Failed to open [ext4], continuing without symbols
> > # To display the perf.data header info, please use --header/--header-only
> > options.
> > #
> > #
> > # Total Lost Samples: 0
> > #
> > # Samples: 499  of event 'cycles'
> > # Event count (approx.): 329354953
> > #
> > # Overhead  Command    Shared Object      Symbol
> > # ........  .........  .................  ................................
> > #
> > 
> >      96.70%  ex_random  ex_random          [.] main
> >      
> >              ---main
> >              
> >                 __libc_start_main
> >                 _start
> > 
> > ...
> > ~~~~~
> > 
> > Note how no inlined frames are actually shown, because the first
> > sample in main points to an IP that does not correspond to any
> > inlined frames.
> 
> perf report -g address --inline --stdio
> 
> Did you try with "-g address" option? It's sorted by address.

Yes, that would work just like `-g srcline`. But that is besides the point. If 
I want to aggregate by function, I still want to split different inline 
frames, otherwise the results will be misleading.

> But anyway, I like this patch. It works well in my test.

I fear that the patch is not ready for acceptance though. It fails when the 
sort-order is inverted, i.e. the snippet I included in my patch uses `--no-
children` which works well. But for top-down call-graphs I will need to 
compare the full inlined call stack, otherwise we just get this:

    99.35%     0.00%  a.out    libc-2.25.so       [.] __libc_start_main
            |
            ---__libc_start_main
               |          
               |--72.56%--main
               |          main (inline)
               |          
std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned 
long, 16807ul, 0ul, 2147483647ul> > (inline)
               |          
std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned 
long, 16807ul, 0ul, 2147483647ul> > (inline)
               |          
std::__detail::_Adaptor<std::linear_congruential_engine<unsigned long, 
16807ul, 0ul, 2147483647ul>, double>::operator() (inline)
               |          
                --26.80%--main

The problem here is that all samples in main start with the same inlined 
frame, the "main (inline)" one. Even if that wouldn't be there, we'd still get 
wrong results if we inline a function A which in turns has two other functions 
B and C inlined. we need to split the graph for that situation too, but we 
don't so far...

I'll try t find the time to improve this situation, thanks for the early 
review already Jin!

Cheers


-- 
Milian Wolff | milian.wolff@...b.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts