From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S964860AbcKWNgj (ORCPT <rfc822;w@1wt.eu>);
        Wed, 23 Nov 2016 08:36:39 -0500
Received: from mail.kernel.org ([198.145.29.136]:41580 "EHLO mail.kernel.org"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S964810AbcKWNgh (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 23 Nov 2016 08:36:37 -0500
Date: Wed, 23 Nov 2016 10:36:24 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>, Jiri Olsa <jolsa@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Stephane Eranian <eranian@google.com>,
        Andi Kleen <andi@firstfloor.org>
Subject: Re: [PATCH v2 7/8] perf sched timehist: Add -V/--cpu-visual option
Message-ID: <20161123133624.GG5390@kernel.org>
References: <20161116060634.28477-1-namhyung@kernel.org>
 <20161116060634.28477-8-namhyung@kernel.org>
 <20161122183326.GD5390@kernel.org>
 <20161123053440.GB19719@sejong>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20161123053440.GB19719@sejong>
X-Url: http://acmel.wordpress.com
User-Agent: Mutt/1.7.1 (2016-10-04)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Em Wed, Nov 23, 2016 at 02:34:40PM +0900, Namhyung Kim escreveu:
> On Tue, Nov 22, 2016 at 03:33:26PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Nov 16, 2016 at 03:06:33PM +0900, Namhyung Kim escreveu:
> > > From: David Ahern <dsahern@gmail.com>
> > > 
> > > The -V option provides a visual aid for sched switches by cpu:
> > > 
> > >   $ perf sched timehist -V
> > >              time    cpu  0123456789abc  task name              b/n time  sch delay   run time
> > >                                          [tid/pid]                (msec)     (msec)     (msec)
> > >   --------------- ------  -------------  --------------------  ---------  ---------  ---------
> > >   ...
> > >    2412598.429696 [0009]           i     <idle>                    0.000      0.000      0.000
> > >    2412598.429767 [0002]    s            perf[7219]                0.000      0.000      0.000
> > >    2412598.429783 [0009]           s     perf[7220]                0.000      0.006      0.087
> > >    2412598.429794 [0010]            i    <idle>                    0.000      0.000      0.000
> > >    2412598.429795 [0009]           s     migration/9[53]           0.000      0.003      0.011
> > >    2412598.430370 [0010]            s    sleep[7220]               0.011      0.000      0.576
> > >    2412598.432584 [0003]     i           <idle>                    0.000      0.000      0.000
> > >   ...
> > 
> > Forgot to add docs, will do.
> 
> The documentation of sched timehist command (including the -V option)
> comes with the next patch (8/8).

I changed that to do it the preferred way, which is to add documentation
on the same changeset that introduces a new command line option.

I have it queued up, just reworking some stuff on a patch I wrote to
then send all this to Ingo, shouldn't take long,

Thanks,

- Arnaldo
 
> Thanks,
> Namhyung
> 
> 
> > 
> > - Arnaldo
> >  
> > > Signed-off-by: David Ahern <dsahern@gmail.com>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > >  tools/perf/builtin-sched.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 42 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > > index 1f8731640809..829468defa07 100644
> > > --- a/tools/perf/builtin-sched.c
> > > +++ b/tools/perf/builtin-sched.c
> > > @@ -201,6 +201,7 @@ struct perf_sched {
> > >  	bool		summary_only;
> > >  	bool		show_callchain;
> > >  	unsigned int	max_stack;
> > > +	bool		show_cpu_visual;
> > >  	bool		show_wakeups;
> > >  	u64		skipped_samples;
> > >  };
> > > @@ -1783,10 +1784,23 @@ static char *timehist_get_commstr(struct thread *thread)
> > >  	return str;
> > >  }
> > >  
> > > -static void timehist_header(void)
> > > +static void timehist_header(struct perf_sched *sched)
> > >  {
> > > +	u32 ncpus = sched->max_cpu + 1;
> > > +	u32 i, j;
> > > +
> > >  	printf("%15s %6s ", "time", "cpu");
> > >  
> > > +	if (sched->show_cpu_visual) {
> > > +		printf(" ");
> > > +		for (i = 0, j = 0; i < ncpus; ++i) {
> > > +			printf("%x", j++);
> > > +			if (j > 15)
> > > +				j = 0;
> > > +		}
> > > +		printf(" ");
> > > +	}
> > > +
> > >  	printf(" %-20s  %9s  %9s  %9s",
> > >  		"task name", "wait time", "sch delay", "run time");
> > >  
> > > @@ -1797,6 +1811,9 @@ static void timehist_header(void)
> > >  	 */
> > >  	printf("%15s %-6s ", "", "");
> > >  
> > > +	if (sched->show_cpu_visual)
> > > +		printf(" %*s ", ncpus, "");
> > > +
> > >  	printf(" %-20s  %9s  %9s  %9s\n", "[tid/pid]", "(msec)", "(msec)", "(msec)");
> > >  
> > >  	/*
> > > @@ -1804,6 +1821,9 @@ static void timehist_header(void)
> > >  	 */
> > >  	printf("%.15s %.6s ", graph_dotted_line, graph_dotted_line);
> > >  
> > > +	if (sched->show_cpu_visual)
> > > +		printf(" %.*s ", ncpus, graph_dotted_line);
> > > +
> > >  	printf(" %.20s  %.9s  %.9s  %.9s",
> > >  		graph_dotted_line, graph_dotted_line, graph_dotted_line,
> > >  		graph_dotted_line);
> > > @@ -1817,11 +1837,28 @@ static void timehist_print_sample(struct perf_sched *sched,
> > >  				  struct thread *thread)
> > >  {
> > >  	struct thread_runtime *tr = thread__priv(thread);
> > > +	u32 max_cpus = sched->max_cpu + 1;
> > >  	char tstr[64];
> > >  
> > >  	timestamp__scnprintf_usec(sample->time, tstr, sizeof(tstr));
> > >  	printf("%15s [%04d] ", tstr, sample->cpu);
> > >  
> > > +	if (sched->show_cpu_visual) {
> > > +		u32 i;
> > > +		char c;
> > > +
> > > +		printf(" ");
> > > +		for (i = 0; i < max_cpus; ++i) {
> > > +			/* flag idle times with 'i'; others are sched events */
> > > +			if (i == sample->cpu)
> > > +				c = (thread->tid == 0) ? 'i' : 's';
> > > +			else
> > > +				c = ' ';
> > > +			printf("%c", c);
> > > +		}
> > > +		printf(" ");
> > > +	}
> > > +
> > >  	printf(" %-*s ", comm_width, timehist_get_commstr(thread));
> > >  
> > >  	print_sched_time(tr->dt_wait, 6);
> > > @@ -2095,6 +2132,8 @@ static void timehist_print_wakeup_event(struct perf_sched *sched,
> > >  
> > >  	timestamp__scnprintf_usec(sample->time, tstr, sizeof(tstr));
> > >  	printf("%15s [%04d] ", tstr, sample->cpu);
> > > +	if (sched->show_cpu_visual)
> > > +		printf(" %*s ", sched->max_cpu + 1, "");
> > >  
> > >  	printf(" %-*s ", comm_width, timehist_get_commstr(thread));
> > >  
> > > @@ -2458,7 +2497,7 @@ static int perf_sched__timehist(struct perf_sched *sched)
> > >  		sched->summary = sched->summary_only;
> > >  
> > >  	if (!sched->summary_only)
> > > -		timehist_header();
> > > +		timehist_header(sched);
> > >  
> > >  	err = perf_session__process_events(session);
> > >  	if (err) {
> > > @@ -2842,6 +2881,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
> > >  	OPT_BOOLEAN('S', "with-summary", &sched.summary,
> > >  		    "Show all syscalls and summary with statistics"),
> > >  	OPT_BOOLEAN('w', "wakeups", &sched.show_wakeups, "Show wakeup events"),
> > > +	OPT_BOOLEAN('V', "cpu-visual", &sched.show_cpu_visual, "Add CPU visual"),
> > >  	OPT_PARENT(sched_options)
> > >  	};
> > >  
> > > -- 
> > > 2.10.1