linux-kernel - Re: [PATCH 4/7] perf diff: Fix to sort by baseline field by default

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: <20141222144513.GB2717@kernel.org>
Date:	Mon, 22 Dec 2014 11:45:13 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Namhyung Kim <namhyung@...nel.org>
Cc:	Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Jiri Olsa <jolsa@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	David Ahern <dsahern@...il.com>,
	Stephane Eranian <eranian@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>
Subject: Re: [PATCH 4/7] perf diff: Fix to sort by baseline field by default

Em Mon, Dec 22, 2014 at 01:44:12PM +0900, Namhyung Kim escreveu:
> The currently perf diff didn't add the baseline and delta (or other
> compute) fields to the sort list so output will be sorted by other
> fields like alphabetical order of DSO or symbol as below example.
> 
> Fix it by adding hpp formats for the fields and provides default
> compare functions.
> 
> Before:
> 
>   $ perf diff
>   # Event 'cycles'
>   #
>   # Baseline    Delta  Shared Object       Symbol
>   # ........  .......  ..................  ...............................
>   #
>                        [bridge]            [k] ip_sabotage_in
>                        [btrfs]             [k] __etree_search.constprop.47
>        0.01%           [btrfs]             [k] btrfs_file_mmap
>        0.01%   -0.01%  [btrfs]             [k] btrfs_getattr
>                        [e1000e]            [k] e1000_watchdog
>        0.00%           [kernel.vmlinux]    [k] PageHuge
>        0.00%           [kernel.vmlinux]    [k] __acct_update_integrals
>        0.00%           [kernel.vmlinux]    [k] __activate_page
>                        [kernel.vmlinux]    [k] __alloc_fd
>        0.02%   +0.02%  [kernel.vmlinux]    [k] __alloc_pages_nodemask
>        ...
> 
> After:
> 
>   # Baseline    Delta  Shared Object       Symbol
>   # ........  .......  ..................  ................................
>   #
>       24.73%   -4.62%  perf                [.] append_chain_children
>        7.96%   -1.29%  perf                [.] dso__find_symbol
>        6.97%   -2.07%  libc-2.20.so        [.] vfprintf
>        4.61%   +0.88%  libc-2.20.so        [.] __fprintf_chk
>        4.41%   +2.43%  perf                [.] sort__comm_cmp
>        4.10%   -0.16%  perf                [.] comm__str
>        4.03%   -0.93%  perf                [.] machine__findnew_thread_time
>        3.82%   +3.09%  perf                [.] __hists__add_entry
>        2.95%   -0.18%  perf                [.] sort__dso_cmp
>        ...

Looks better, here I tried running 'perf record usleep 1'  in quick
succession to then run 'perf diff'  before and after this patch, got:

Before:

[ssdandy ~]$ cat /tmp/before
# Event 'cycles'
#
# Baseline    Delta  Shared Object     Symbol                        
# ........  .......  ................  ..............................
#
            +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
    34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
             +2.45%  [kernel.vmlinux]  [k] local_clock               
     0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
     2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
            +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
    63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
[acme@...andy ~]$

After:

[acme@...andy linux]$ perf diff
# Event 'cycles'
#
# Baseline    Delta  Shared Object     Symbol                        
# ........  .......  ................  ..............................
#
    63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
    34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
     2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
     0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
            +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
             +2.45%  [kernel.vmlinux]  [k] local_clock               
            +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
[acme@...andy linux]$

Which was ok up to the point where symbols that only appeared on the
second run were not sorted by delta, can you fix that?

Please let me know if it is better to apply this one then a followup to
sort the deltas or if a combined patch to achieve both is best.

I.e.:

[acme@...andy linux]$ perf diff
# Event 'cycles'
#
# Baseline    Delta  Shared Object     Symbol                        
# ........  .......  ................  ..............................
#
    63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
    34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
     2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
     0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
            +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
            +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
             +2.45%  [kernel.vmlinux]  [k] local_clock               

[acme@...andy linux]$

Humm, but then wouldn't we be more interested in sorting _everything_ by
delta?

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/