| 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
| ||
|
Message-ID: <20150421070108.GE1905@sejong>
Date: Tue, 21 Apr 2015 16:01:08 +0900
From: Namhyung Kim <namhyung@...nel.org>
To: Ingo Molnar <mingo@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...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>,
Taeung Song <treeze.taeung@...il.com>
Subject: Re: [PATCH v2] perf tools: Document --children option in more detail
Hi Ingo,
On Mon, Apr 20, 2015 at 08:04:11PM +0200, Ingo Molnar wrote:
>
> * Namhyung Kim <namhyung@...nel.org> wrote:
>
> > As the --children option changes the output of perf report (and perf
> > top) it sometimes confuses users. Add more words and examples to help
> > understanding of the option's behavior - and how to disable it ;-).
>
> Nice! :-)
Thanks! :)
>
> > +++ b/tools/perf/Documentation/overhead.txt
>
> As some people might stumble upon this the old fashioned way, by
> looking around in 'Documentation/', I'm wondering how this file name
> will tell the reader that this is about call chains? (which isn't the
> default recording mode) Maybe name it more explicitly, like
> callchain-overhead.txt?
OK. But I'd rather name it 'overhead-calculation.txt' to align with
the section name. And 'callchain-overhead' makes me thinking about
the overhead of processing callchains at record or report time.
>
> > @@ -0,0 +1,96 @@
> > +Overhead calculation
> > +--------------------
> > +The overhead can be shown in two columns as 'Children' and 'Self' when
> > +perf collects callchains. The self overhead is simply calculated by
> > +adding all period values of the entry - usually a function (symbol).
> > +This is the value that perf shows traditionally and sum of the all
> > +self overhead should be 100%.
>
> s/sum of the all self overhead should be 100%
> /sum of all the self-overhead values should be 100%
>
> ?
OK
>
> > +
> > +The children overhead is calculated by adding all period values of the
> > +child functions so that it can show the total overhead of the higher
> > +level functions. The children here means that functions called from
> > +another function.
>
> Maybe put it into quotes to specify it more clearly? Also, maybe add a
> bit more info - Something like:
>
> The 'children' overhead is calculated by adding all period values of
> the child functions so that it can show the total overhead of the
> higher level functions even if they don't directly execute much.
> 'Children' here means functions that are called from another
> (parent) function.
>
> ?
>
> > +
> > +It might be confusing that the sum of the all children overhead
>
> s/children
> /'children'
Right!
>
> > +exceeds 100% since each of them is already an accumulation of (self)
> > +overhead of its children. But with this enabled, users can find which
> > +function has the most overhead even if samples are spread over the
> > +children.
> > +
> > +Consider the following example; there’re three functions like below.
> > +
> > +-----------------------
> > +void foo(void) {
> > + /* something */
> > +}
> > +
> > +bar(void) {
>
> s/bar(void) {
> /void bar(void) {
>
> :-)
Oops..
>
> > + /* do something */
> > + foo();
> > +}
> > +
> > +int main(void) {
> > + bar()
> > + return 0;
> > +}
> > +-----------------------
> > +
> > +In this case 'foo' is a child of 'bar', and 'bar' is an immediate
> > +child of 'main' so 'foo' also is a child of 'main'. In other words,
> > +'main' is a parent of 'foo' and 'bar'. and 'bar' is a parent of 'foo'.
>
> s/and 'bar'. and
> /and 'bar', and
OK
>
> > +
> > +Suppose all samples are recorded in the 'foo' and 'bar' only. When
>
> either:
> Suppose all samples are recorded in the 'foo' and 'bar' functions only.
>
> or:
> Suppose all samples are recorded in 'foo' and 'bar' only.
I like the shorter. :)
>
> > +you record with callchain you'll see something like below in the usual
>
> s/callchain/callchains
>
> > +(self-overhead-only) output of the perf report:
>
> s/of the perf report/of perf report
OK
>
> > +
> > +----------------------------------
> > +Overhead Symbol
> > +........ .....................
> > + 60.00% foo
> > + |
> > + --- foo
> > + bar
> > + main
> > + __libc_start_main
> > +
> > + 40.00% bar
> > + |
> > + --- bar
> > + main
> > + __libc_start_main
> > +----------------------------------
> > +
> > +When --children option is enabled, the (self) overhead of children (in
>
> s/When --children option
> /When the --children option
OK
>
> > +this case foo and bar) are added to the parent to calculate the
> > +children overhead. In this case the report could be displayed as:
> > +
> > +-------------------------------------------
> > +Children Self Symbol
> > +........ ........ ....................
> > + 100.00% 0.00% __libc_start_main
> > + |
> > + --- __libc_start_main
> > +
> > + 100.00% 0.00% main
> > + |
> > + --- main
> > + __libc_start_main
> > +
> > + 100.00% 40.00% bar
> > + |
> > + --- bar
> > + main
> > + __libc_start_main
> > +
> > + 60.00% 60.00% foo
> > + |
> > + --- foo
> > + bar
> > + main
> > + __libc_start_main
> > +-------------------------------------------
> > +
> > +Since v3.16 the children overhead is shown by default and the output
> > +is sorted by the values. Children overhead is disabled by specifying
> > +--no-children option on the command line or by adding 'report.children
> > += false' or 'top.children = false' in the perfconfig file.
>
> s/perfconfig/.perfconfig
>
> ?
It seems we can have a system-wide /etc/perfconfig too. How about
'the perf config file' instead?
>
>
> > diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> > index 4879cf638824..bd97e5c3c9b6 100644
> > --- a/tools/perf/Documentation/perf-report.txt
> > +++ b/tools/perf/Documentation/perf-report.txt
> > @@ -193,6 +193,7 @@ OPTIONS
> > Accumulate callchain of children to parent entry so that then can
> > show up in the output. The output will have a new "Children" column
> > and will be sorted on the data. It requires callchains are recorded.
> > + See `overhead calculation' section for more details.
>
> s/See `overhead calculation' section
> /See the `overhead calculation' section
OK
>
> >
> > --max-stack::
> > Set the stack depth limit when parsing the callchain, anything
> > @@ -323,6 +324,9 @@ OPTIONS
> > --header-only::
> > Show only perf.data header (forces --stdio).
> >
> > +
> > +include::overhead.txt[]
> > +
> > SEE ALSO
> > --------
> > linkperf:perf-stat[1], linkperf:perf-annotate[1]
> > diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
> > index 3265b1070518..526d6bebec1f 100644
> > --- a/tools/perf/Documentation/perf-top.txt
> > +++ b/tools/perf/Documentation/perf-top.txt
> > @@ -168,7 +168,7 @@ Default is to monitor all CPUS.
> > Accumulate callchain of children to parent entry so that then can
> > show up in the output. The output will have a new "Children" column
> > and will be sorted on the data. It requires -g/--call-graph option
> > - enabled.
> > + enabled. See `overhead calculation' section for more details.
>
> s/See `overhead calculation' section
> /See the `overhead calculation' section
OK
>
> With the above things fixed:
Will fix all my bad English. :)
>
> Reviewed-by: Ingo Molnar <mingo@...nel.org>
Thanks for taking your time to review this!
Namhyung
--
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/
Powered by blists - more mailing lists