[PATCH] perf trace: add total time column to summary.

[PATCH] perf trace: add total time column to summary.

[Milian Wolff]

It is cumbersome to manually calculate the total time spent in a
given syscall by multiplying the average value with the number of
calls. Instead, we now do this directly inside perf trace. Note
that this is also done by strace, which even adds a column with
relative numbers - something we could do in the future.

Example:

perf trace -s find /some/folder > /dev/null

 Summary of events:

 find (19976), 700123 events, 100.0%, 0.000 msec

   syscall            calls    total       min       avg       max      stddev
                               (msec)    (msec)    (msec)    (msec)        (%)
   --------------- -------- --------- --------- --------- ---------     ------
   read                   4     0.006     0.001     0.002     0.003     27.42%
   write               8046     9.617     0.001     0.001     0.035      0.56%
   open               34196    40.384     0.001     0.001     0.071      0.30%
   close              68375    57.104     0.001     0.001     0.076      0.25%
   stat                   4     0.004     0.001     0.001     0.001      3.14%
   fstat              34189    27.518     0.001     0.001     0.060      0.34%
   mmap                  13     0.029     0.001     0.002     0.003     10.74%
   mprotect               6     0.018     0.002     0.003     0.005     17.04%
   munmap                 3     0.014     0.003     0.005     0.006     24.87%
   brk                   87     0.490     0.001     0.006     0.016      6.50%
   ioctl                  3     0.004     0.001     0.001     0.003     36.39%
   access                 1     0.004     0.004     0.004     0.004      0.00%
   uname                  1     0.001     0.001     0.001     0.001      0.00%
   getdents           68393   143.600     0.001     0.002     0.187      0.95%
   fchdir             68371    56.980     0.001     0.001     0.111      0.39%
   arch_prctl             1     0.001     0.001     0.001     0.001      0.00%
   openat             34184    41.737     0.001     0.001     0.102      0.41%
   newfstatat         34184    41.180     0.001     0.001     0.064      0.34%

Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
---
 tools/perf/builtin-trace.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index d183d88..98e1d8b 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2586,9 +2586,9 @@ static size_t thread__dump_stats(struct thread_trace *ttrace,
 
 	printed += fprintf(fp, "\n");
 
-	printed += fprintf(fp, "   syscall            calls      min       avg       max      stddev\n");
-	printed += fprintf(fp, "                               (msec)    (msec)    (msec)        (%%)\n");
-	printed += fprintf(fp, "   --------------- -------- --------- --------- ---------     ------\n");
+	printed += fprintf(fp, "   syscall            calls    total       min       avg       max      stddev\n");
+	printed += fprintf(fp, "                               (msec)    (msec)    (msec)    (msec)        (%%)\n");
+	printed += fprintf(fp, "   --------------- -------- --------- --------- --------- ---------     ------\n");
 
 	/* each int_node is a syscall */
 	while (inode) {
@@ -2605,8 +2605,8 @@ static size_t thread__dump_stats(struct thread_trace *ttrace,
 
 			sc = &trace->syscalls.table[inode->i];
 			printed += fprintf(fp, "   %-15s", sc->name);
-			printed += fprintf(fp, " %8" PRIu64 " %9.3f %9.3f",
-					   n, min, avg);
+			printed += fprintf(fp, " %8" PRIu64 " %9.3f %9.3f %9.3f",
+					   n, avg * n, min, avg);
 			printed += fprintf(fp, " %9.3f %9.2f%%\n", max, pct);
 		}
 
-- 
2.5.0

Re: [PATCH] perf trace: add total time column to summary.

[Milian Wolff]

Hey Arnaldo,

On Thursday 06 August 2015 11:24:29 Milian Wolff wrote:
<snip>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Milian Wolff <milian.wolff@kdab.com>

I hope this patch is OK. I have some more questions:

a) Should I always send to linux-kernel and CC linux-perf-users, or is it OK 
to send it only to linux-perf-users? 

b) Should I put you explicitly into the CC, I recon you also read the mailing 
lists?

c) Should I add more people to the CC list? The get_maintainer script also 
lists Ingo Molnar and Peter Zijlstra, but your recent patch set has them not 
in the CC list.

d) My patch has some warnings according to checkpatch, because some lines are 
too long. Is this very frowned upon in the Kernel? It would make the table 
much harder to read when I split it onto multiple lines, but of course I can 
amend that if you want it.

If I messed up anything else, please tell me.

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

Re: [PATCH] perf trace: add total time column to summary.

[Arnaldo Carvalho de Melo]

Em Thu, Aug 06, 2015 at 11:53:42AM +0200, Milian Wolff escreveu:
> On Thursday 06 August 2015 11:24:29 Milian Wolff wrote:
> <snip>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
 
> I hope this patch is OK. I have some more questions:

It is. I did just minor reflowing, consistency changes to the commit log
message, things I do while reading the patch description, added a
Tested-by: me, as I did more than just compile test and cursory review,
actually taking the time to run it and see the results, etc.

Here it is:

https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=237f0f8d473c6bc1952c649267b8b3be5dbaa5d9
 
> a) Should I always send to linux-kernel and CC linux-perf-users, or is it OK 
> to send it only to linux-perf-users? 

So far, the mailing list for development is
linux-kernel@vger.kernel.org, so please keep it on the CC list.

Keeping linux-perf-users@vger.kernel.org may reach people that can't
stand the volume of lkml tho.
 
> b) Should I put you explicitly into the CC, I recon you also read the mailing 
> lists?

Yes, you should, that way it'll go to my main inbox, in addition to the
list folder.

No need to put me, or Ingo or PeterZ on the patch description CC list,
just on the e-mail CC list.

There are people who work on each tool from time to time, so sometimes
its a good idea to do a 'git blame' in the parts of the code you change
to add them to the CC list, both in the e-mail and in the patch
description.

In the e-mail because we want them to straight away receive a
notification that changes are being made in areas they worked on, and in
the CC list in the description message because we want to have that
documented in the source code repository (hey, they were warned!).

My scripts will pick whoever is in the CC list in the e-mail to add them
to the CC list in the commit log, but this only when I process directly
from e-mail.

Preferably those rules are followed by whoever asks me to apply patches,
so that, for frequent contributors we can eventually work on a 'git
pull' way.

I.e. you post patches to the mailing list together with a "git
request-pull" generated cover letter that provides instructions for me
to do a 'git pull' from some publicly accessible git server, preferably
hosted at git.kernel.org, but that, again, for frequent patch submitters
that follow these rules.
 
> c) Should I add more people to the CC list? The get_maintainer script also 
> lists Ingo Molnar and Peter Zijlstra, but your recent patch set has them not 
> in the CC list.

Well, everything will go thru Ingo anyway, having them in the e-mail CC
list may get their attention before I send the patches to Ingo for
pulling, which may help speed up the process, if they have concerns or
suggestions.
 
> d) My patch has some warnings according to checkpatch, because some lines are 
> too long. Is this very frowned upon in the Kernel? It would make the table 

I don't care that much about this one, i.e. long lines. Avoid them when
you can, don't spend too much on it tho.

> much harder to read when I split it onto multiple lines, but of course I can 

Right, it is ok in this case.

> amend that if you want it.
> 
> If I messed up anything else, please tell me.

Nope, take a look at the stylistic stuff I did on the patch comment log,
adopt whatever you think is sensible, thanks for taking the time to ask
these questions and follow the coding style.

- Arnaldo

Re: [PATCH] perf trace: add total time column to summary.

[Wangnan (F)]

On 2015/8/6 21:17, Arnaldo Carvalho de Melo wrote:
> Em Thu, Aug 06, 2015 at 11:53:42AM +0200, Milian Wolff escreveu:
>> On Thursday 06 August 2015 11:24:29 Milian Wolff wrote:
>> <snip>
>>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>>> Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
>   
>> I hope this patch is OK. I have some more questions:
> It is. I did just minor reflowing, consistency changes to the commit log
> message, things I do while reading the patch description, added a
> Tested-by: me, as I did more than just compile test and cursory review,
> actually taking the time to run it and see the results, etc.
>
> Here it is:
>
> https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=237f0f8d473c6bc1952c649267b8b3be5dbaa5d9
>   
>> a) Should I always send to linux-kernel and CC linux-perf-users, or is it OK
>> to send it only to linux-perf-users?
> So far, the mailing list for development is
> linux-kernel@vger.kernel.org, so please keep it on the CC list.
>
> Keeping linux-perf-users@vger.kernel.org may reach people that can't
> stand the volume of lkml tho.
>   
>> b) Should I put you explicitly into the CC, I recon you also read the mailing
>> lists?
> Yes, you should, that way it'll go to my main inbox, in addition to the
> list folder.
>
> No need to put me, or Ingo or PeterZ on the patch description CC list,
> just on the e-mail CC list.
>
> There are people who work on each tool from time to time, so sometimes
> its a good idea to do a 'git blame' in the parts of the code you change
> to add them to the CC list, both in the e-mail and in the patch
> description.
>
> In the e-mail because we want them to straight away receive a
> notification that changes are being made in areas they worked on, and in
> the CC list in the description message because we want to have that
> documented in the source code repository (hey, they were warned!).
>
> My scripts will pick whoever is in the CC list in the e-mail to add them
> to the CC list in the commit log, but this only when I process directly
> from e-mail.
>
> Preferably those rules are followed by whoever asks me to apply patches,
> so that, for frequent contributors we can eventually work on a 'git
> pull' way.
>
> I.e. you post patches to the mailing list together with a "git
> request-pull" generated cover letter that provides instructions for me
> to do a 'git pull' from some publicly accessible git server, preferably
> hosted at git.kernel.org, but that, again, for frequent patch submitters
> that follow these rules.

Thank you to CC-ing me this mail. I have a further question: shall I keep
Cc list in commit log in my git tree commits, and remove then when
I send them to mailing list?

Thank you.

More details about submitting pull requests to upstream was: Re: [PATCH] perf trace: add total time column to summary.

[Arnaldo Carvalho de Melo]

Em Fri, Aug 07, 2015 at 09:31:41AM +0800, Wangnan (F) escreveu:
> On 2015/8/6 21:17, Arnaldo Carvalho de Melo wrote:
> >Em Thu, Aug 06, 2015 at 11:53:42AM +0200, Milian Wolff escreveu:
> >>On Thursday 06 August 2015 11:24:29 Milian Wolff wrote:
> >><snip>
> >>>Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> >>>Signed-off-by: Milian Wolff <milian.wolff@kdab.com>
> >>I hope this patch is OK. I have some more questions:
> >It is. I did just minor reflowing, consistency changes to the commit log
> >message, things I do while reading the patch description, added a
> >Tested-by: me, as I did more than just compile test and cursory review,
> >actually taking the time to run it and see the results, etc.
> >
> >Here it is:
> >
> >https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=237f0f8d473c6bc1952c649267b8b3be5dbaa5d9
> >>a) Should I always send to linux-kernel and CC linux-perf-users, or is it OK
> >>to send it only to linux-perf-users?
> >So far, the mailing list for development is
> >linux-kernel@vger.kernel.org, so please keep it on the CC list.
> >
> >Keeping linux-perf-users@vger.kernel.org may reach people that can't
> >stand the volume of lkml tho.
> >>b) Should I put you explicitly into the CC, I recon you also read the mailing
> >>lists?
> >Yes, you should, that way it'll go to my main inbox, in addition to the
> >list folder.
> >
> >No need to put me, or Ingo or PeterZ on the patch description CC list,
> >just on the e-mail CC list.
> >
> >There are people who work on each tool from time to time, so sometimes
> >its a good idea to do a 'git blame' in the parts of the code you change
> >to add them to the CC list, both in the e-mail and in the patch
> >description.
> >
> >In the e-mail because we want them to straight away receive a
> >notification that changes are being made in areas they worked on, and in
> >the CC list in the description message because we want to have that
> >documented in the source code repository (hey, they were warned!).
> >
> >My scripts will pick whoever is in the CC list in the e-mail to add them
> >to the CC list in the commit log, but this only when I process directly
> >from e-mail.
> >
> >Preferably those rules are followed by whoever asks me to apply patches,
> >so that, for frequent contributors we can eventually work on a 'git
> >pull' way.
> >
> >I.e. you post patches to the mailing list together with a "git
> >request-pull" generated cover letter that provides instructions for me
> >to do a 'git pull' from some publicly accessible git server, preferably
> >hosted at git.kernel.org, but that, again, for frequent patch submitters
> >that follow these rules.
> 
> Thank you to CC-ing me this mail. I have a further question: shall I keep
> Cc list in commit log in my git tree commits, and remove then when
> I send them to mailing list?

Keep the CC list, as it will be used when you do:

  git format-patch -n --cover-letter tip/perf/core..

Then you'll create a signed tag with:

git tag -u 5352AA40 perf-core-for-mingo

Edit 0000-cover-letter, to include the output of:

  git request-pull tip/perf/core git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf-core-for-mingo >> 0000-cover-letter.patch

Replace 'perf-core-for-mingo' with a suitable name, perhaps
'perf-bpf-for-acme', or something to that effect.

You will use that gpg key that you created to get access to the
kernel.org account.

- Arnaldo