linux-kernel - Re: [PATCH 2/4] perf annotate: Align filename:linenr and more correct summary

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: <e24c6f80-c4a8-4a81-8bec-b03cd8661e39@gmail.com>
Date:   Thu, 23 Feb 2017 01:31:33 +0900
From:   Taeung Song <treeze.taeung@...il.com>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jiri Olsa <jolsa@...nel.org>, Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Wang Nan <wangnan0@...wei.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>
Subject: Re: [PATCH 2/4] perf annotate: Align filename:linenr and more correct
 summary



On 02/22/2017 08:22 PM, Namhyung Kim wrote:
> On Wed, Feb 22, 2017 at 7:08 PM, Taeung Song <treeze.taeung@...il.com> wrote:
>> In the stdio interface, currently 'filename:linenr' infos
>> are confusedly printed in the intervals of assembly code.
>> So fix it.
>>
>> The cause was a 0.5% filter of if statement. After fixed,
>> additionally summary of overhead per srcline is more correct.
>>
>> Before:
>>
>>     # perf annotate --stdio -l
>>
>>   Sorted summary for file /home/taeung/workspace/perf-test/test
>>   ----------------------------------------------
>>
>>      36.57 test.c:38
>>      28.72 test.c:37
>>
>>   ...
>>
>>    Percent |      Source code & Disassembly of test ...
>>
>>   ...
>>
>>       0.21 :        400816:       push   %rbp
>>    test.c:26    1.86 :         400817:       mov    %rsp,%rbp
>>       0.21 :        40081a:       mov    %edi,-0x24(%rbp)
>>       0.21 :        40081d:       mov    %rsi,-0x30(%rbp)
>>
>> After:
>>
>>     # perf annotate --stdio -l
>>
>>   Sorted summary for file /home/taeung/workspace/perf-test/test
>>   ----------------------------------------------
>>
>>      37.40 test.c:38
>>      29.34 test.c:37
>>
>>   ...
>>
>>    Percent |      Source code & Disassembly of test ...
>>
>>   ...
>>
>>    test.c:26
>>       0.21 :        400816:       push   %rbp
>>       1.86 :        400817:       mov    %rsp,%rbp
>>       0.21 :        40081a:       mov    %edi,-0x24(%rbp)
>>       0.21 :        40081d:       mov    %rsi,-0x30(%rbp)
>
> I guess it's just a problem of a missing newline..
>

I think the problem is not only from a missing newline but also
from 0.5 filtering if statement.

For example,
If just appending new line, the output is as below

        0.21 :        400816:       push   %rbp
     test.c:26
        1.86 :        400817:       mov    %rsp,%rbp
        0.21 :        40081a:       mov    %edi,-0x24(%rbp)
        0.21 :        40081d:       mov    %rsi,-0x30(%rbp)

The reason of the wrong sorting is that only 400817 is matched with 
test.c:26
And the root cause is a if statement filtering smaller values than 0.5.
The if statement prevent other addresses that are less than 0.5
from matching test.c:26

So I eliminated it.

-               if (percent_max <= 0.5)
-                       goto next;

But 400816, 400817, 40081a and 40081d addresses should be matched
with test.c:26. So I think it is better to show as below

     test.c:26
        0.21 :        400816:       push   %rbp
        1.86 :        400817:       mov    %rsp,%rbp
        0.21 :        40081a:       mov    %edi,-0x24(%rbp)
        0.21 :        40081d:       mov    %rsi,-0x30(%rbp)


And I think it is better to rewrite this commit title and message..
I'll change this patch as v2 to clearly understand problem and solution.


Thanks,
Teaung