All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	David Ahern <dsahern@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Jiri Olsa <jolsa@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
	Stephane Eranian <eranian@google.com>,
	Andi Kleen <andi@firstfloor.org>
Subject: Re: [PATCHSET 0/7] perf sched: Introduce timehist command, again (v1)
Date: Tue, 15 Nov 2016 07:42:14 +0100	[thread overview]
Message-ID: <20161115064214.GB27089@gmail.com> (raw)
In-Reply-To: <20161114161243.15403-1-namhyung@kernel.org>


* Namhyung Kim <namhyung@kernel.org> wrote:

> Hello,
> 
> This patchset is a rebased version of David's sched timehist work [1].
> I plan to improve perf sched command more and think that having
> timehist command before the work looks good.  It seems David is busy
> these days, so I'm retrying it by myself.
> 
> This implements only basic feature and a few options.  I just split
> the patch to make it easier to review and did some cosmetic changes.
> More patches will come later.
> 
> The below is from the David's original description:
> 
> ------------------------8<-------------------------
> 'perf sched timehist' provides an analysis of scheduling events.
> 
> Example usage:
>     perf sched record -- sleep 1
>     perf sched timehist


Cool, very nice!

> By default it shows the individual schedule events, including the time between
> sched-in events for the task, the task scheduling delay (time between wakeup
> and actually running) and run time for the task:
> 
>            time cpu  task name[tid/pid]    b/n time sch delay  run time
>   ------------- ---- -------------------- --------- --------- ---------
>    79371.874569 [11] gcc[31949]               0.014     0.000     1.148
>    79371.874591 [10] gcc[31951]               0.000     0.000     0.024
>    79371.874603 [10] migration/10[59]         3.350     0.004     0.011
>    79371.874604 [11] <idle>                   1.148     0.000     0.035
>    79371.874723 [05] <idle>                   0.016     0.000     1.383
>    79371.874746 [05] gcc[31949]               0.153     0.078     0.022
> ...

What does the 'b/n' abbreviation stand for? 'Between'? Could we call the column 
'sch wait' instead, or so?


> Times are in msec.usec.
> 
> If callchains were recorded they are appended to the line with a default stack depth of 5:
> 
>    79371.874569 [11] gcc[31949]                  0.000014  0.000000  0.001148  wait_for_completion_killable do_fork sys_vfork stub_vfork __vfork
>    79371.874591 [10] gcc[31951]                  0.000000  0.000000  0.000024  __cond_resched _cond_resched wait_for_completion stop_one_cpu sched_exec
>    79371.874603 [10] migration/10[59]            0.003350  0.000004  0.000011  smpboot_thread_fn kthread ret_from_fork
>    79371.874604 [11] <idle>                      0.001148  0.000000  0.000035  cpu_startup_entry start_secondary
>    79371.874723 [05] <idle>                      0.000016  0.000000  0.001383  cpu_startup_entry start_secondary
>    79371.874746 [05] gcc[31949]                  0.000153  0.000078  0.000022  do_wait sys_wait4 system_call_fastpath __GI___waitpid

So when I first saw this it was hard for me to disambiguate individual function 
names. Wouldn't this be a bit more readable:

>    79371.874569 [11] gcc[31949]                  0.000014  0.000000  0.001148  wait_for_completion_killable() <- do_fork sys_vfork stub_vfork() <- __vfork()
>    79371.874591 [10] gcc[31951]                  0.000000  0.000000  0.000024  __cond_resched() <- _cond_resched() <- wait_for_completion() <- stop_one_cpu() <- sched_exec()
>    79371.874603 [10] migration/10[59]            0.003350  0.000004  0.000011  smpboot_thread_fn() <- kthread() <- ret_from_fork()
>    79371.874604 [11] <idle>                      0.001148  0.000000  0.000035  cpu_startup_entry() <- start_secondary()
>    79371.874723 [05] <idle>                      0.000016  0.000000  0.001383  cpu_startup_entry() <- start_secondary()
>    79371.874746 [05] gcc[31949]                  0.000153  0.000078  0.000022  do_wait() <- sys_wait4() <- system_call_fastpath() <- __GI___waitpid()

Or:

>    79371.874569 [11] gcc[31949]                  0.000014  0.000000  0.001148  wait_for_completion_killable()		<- do_fork sys_vfork stub_vfork() <- __vfork()
>    79371.874591 [10] gcc[31951]                  0.000000  0.000000  0.000024  __cond_resched()			<- _cond_resched() <- wait_for_completion() <- stop_one_cpu() <- sched_exec()
>    79371.874603 [10] migration/10[59]            0.003350  0.000004  0.000011  smpboot_thread_fn()			<- kthread() <- ret_from_fork()
>    79371.874604 [11] <idle>                      0.001148  0.000000  0.000035  cpu_startup_entry()			<- start_secondary()
>    79371.874723 [05] <idle>                      0.000016  0.000000  0.001383  cpu_startup_entry()			<- start_secondary()
>    79371.874746 [05] gcc[31949]                  0.000153  0.000078  0.000022  do_wait()				<- sys_wait4() <- system_call_fastpath() <- __GI___waitpid()

(i.e. visually separate the first entry - and list the rest.)

Or maybe it could be ASCII color coded so that the different entries are easier to 
separate: for example the functions could be printed in alternating white/grey 
color?

Thanks,

	Ingo

  parent reply	other threads:[~2016-11-15  6:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-14 16:12 [PATCHSET 0/7] perf sched: Introduce timehist command, again (v1) Namhyung Kim
2016-11-14 16:12 ` [PATCH RESEND 1/7] perf symbol: Print symbol offsets conditionally Namhyung Kim
2016-11-14 16:12 ` [PATCH 2/7] perf sched timehist: Introduce timehist command Namhyung Kim
2016-11-14 16:12 ` [PATCH 3/7] perf sched timehist: Add summary options Namhyung Kim
2016-11-14 16:12 ` [PATCH 4/7] perf sched timehist: Add -w/--wakeups option Namhyung Kim
2016-11-14 16:12 ` [PATCH 5/7] perf sched timehist: Add call graph options Namhyung Kim
2016-11-14 16:12 ` [PATCH 6/7] perf sched timehist: Add -V/--cpu-visual option Namhyung Kim
2016-11-14 16:12 ` [PATCH 7/7] perf sched: Add documentation for timehist options Namhyung Kim
2016-11-14 16:26 ` [PATCHSET 0/7] perf sched: Introduce timehist command, again (v1) David Ahern
2016-11-15  6:42 ` Ingo Molnar [this message]
2016-11-15  6:55   ` Namhyung Kim
2016-11-15  7:34     ` Ingo Molnar
2016-11-15 15:14       ` David Ahern
2016-11-15 15:32         ` Namhyung Kim
2016-11-15 18:25           ` David Ahern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161115064214.GB27089@gmail.com \
    --to=mingo@kernel.org \
    --cc=acme@kernel.org \
    --cc=andi@firstfloor.org \
    --cc=dsahern@gmail.com \
    --cc=eranian@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.