All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf ui/tui: Fix a small callchain display bug
@ 2014-08-13  6:02 Namhyung Kim
  2014-08-13  8:19 ` Ingo Molnar
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Namhyung Kim @ 2014-08-13  6:02 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
	Namhyung Kim, LKML, Jiri Olsa, Andi Kleen, Frederic Weisbecker

The currently when perf TUI report shows callchain, the first level
chains have bogus '+' sign even though only the last one has children.

Since they are on a single line of the chain, toggling intermediate
entries has no effect.  Fix it to show '+' sign at the last entry
only.  Note that non-first level callchain entries don't have this
problem.

Before:

  Children      Self  Command  Shared Object      Symbols
---------------------------------------------------------------------------
-   40.70%     0.00%  swapper  [kernel.kallsyms]  [k] cpuidle_wrap_enter
   + cpuidle_wrap_enter
   + cpuidle_enter_tk
   + cpuidle_idle_call
   + cpu_idle

After:

  Children      Self  Command  Shared Object      Symbols
---------------------------------------------------------------------------
-   40.70%     0.00%  swapper  [kernel.kallsyms]  [k] cpuidle_wrap_enter
     cpuidle_wrap_enter
     cpuidle_enter_tk
     cpuidle_idle_call
   + cpu_idle

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 045c1e16ac59..1818d1275d02 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -228,8 +228,10 @@ static void callchain_node__init_have_children(struct callchain_node *node)
 {
 	struct callchain_list *chain;
 
-	list_for_each_entry(chain, &node->val, list)
+	if (!list_empty(&node->val)) {
+		chain = list_entry(node->val.prev, struct callchain_list, list);
 		chain->ms.has_children = !RB_EMPTY_ROOT(&node->rb_root);
+	}
 
 	callchain_node__init_have_children_rb_tree(node);
 }
-- 
2.0.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf ui/tui: Fix a small callchain display bug
  2014-08-13  6:02 [PATCH] perf ui/tui: Fix a small callchain display bug Namhyung Kim
@ 2014-08-13  8:19 ` Ingo Molnar
  2014-08-13 20:12   ` Arnaldo Carvalho de Melo
  2014-08-13 20:06 ` Arnaldo Carvalho de Melo
  2014-08-14  8:48 ` [tip:perf/core] perf hists browser: " tip-bot for Namhyung Kim
  2 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2014-08-13  8:19 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Namhyung Kim, LKML, Jiri Olsa, Andi Kleen, Frederic Weisbecker


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

> The currently when perf TUI report shows callchain, the first level
> chains have bogus '+' sign even though only the last one has children.
> 
> Since they are on a single line of the chain, toggling intermediate
> entries has no effect.  Fix it to show '+' sign at the last entry
> only.  Note that non-first level callchain entries don't have this
> problem.
> 
> Before:
> 
>   Children      Self  Command  Shared Object      Symbols
> ---------------------------------------------------------------------------
> -   40.70%     0.00%  swapper  [kernel.kallsyms]  [k] cpuidle_wrap_enter
>    + cpuidle_wrap_enter
>    + cpuidle_enter_tk
>    + cpuidle_idle_call
>    + cpu_idle
> 
> After:
> 
>   Children      Self  Command  Shared Object      Symbols
> ---------------------------------------------------------------------------
> -   40.70%     0.00%  swapper  [kernel.kallsyms]  [k] cpuidle_wrap_enter
>      cpuidle_wrap_enter
>      cpuidle_enter_tk
>      cpuidle_idle_call
>    + cpu_idle
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Nice fix!

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf ui/tui: Fix a small callchain display bug
  2014-08-13  6:02 [PATCH] perf ui/tui: Fix a small callchain display bug Namhyung Kim
  2014-08-13  8:19 ` Ingo Molnar
@ 2014-08-13 20:06 ` Arnaldo Carvalho de Melo
  2014-08-14  8:48 ` [tip:perf/core] perf hists browser: " tip-bot for Namhyung Kim
  2 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-08-13 20:06 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim, LKML,
	Jiri Olsa, Andi Kleen, Frederic Weisbecker

Em Wed, Aug 13, 2014 at 03:02:41PM +0900, Namhyung Kim escreveu:
> The currently when perf TUI report shows callchain, the first level
> chains have bogus '+' sign even though only the last one has children.
> 
> Since they are on a single line of the chain, toggling intermediate
> entries has no effect.  Fix it to show '+' sign at the last entry
> only.  Note that non-first level callchain entries don't have this
> problem.
> 
> Before:
> 
>   Children      Self  Command  Shared Object      Symbols
> ---------------------------------------------------------------------------

Please avoid starting a line with --- inside the changeset comment log as this
confuses scripts, that takes that as the end of the comment, like in ...
(see below)

> -   40.70%     0.00%  swapper  [kernel.kallsyms]  [k] cpuidle_wrap_enter
>    + cpuidle_wrap_enter
>    + cpuidle_enter_tk
>    + cpuidle_idle_call
>    + cpu_idle
> 
> After:
> 
>   Children      Self  Command  Shared Object      Symbols
> ---------------------------------------------------------------------------
> -   40.70%     0.00%  swapper  [kernel.kallsyms]  [k] cpuidle_wrap_enter
>      cpuidle_wrap_enter
>      cpuidle_enter_tk
>      cpuidle_idle_call
>    + cpu_idle
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---

... here ^ ^ ^ ^ ^ 

:-)

Thanks for the fix, checking with 'P' after 'E' to expand everything,
before and after, to then do a:

diff -u perf.hist.0 perf.hist.1

- Arnaldo

>  tools/perf/ui/browsers/hists.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 045c1e16ac59..1818d1275d02 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -228,8 +228,10 @@ static void callchain_node__init_have_children(struct callchain_node *node)
>  {
>  	struct callchain_list *chain;
>  
> -	list_for_each_entry(chain, &node->val, list)
> +	if (!list_empty(&node->val)) {
> +		chain = list_entry(node->val.prev, struct callchain_list, list);
>  		chain->ms.has_children = !RB_EMPTY_ROOT(&node->rb_root);
> +	}
>  
>  	callchain_node__init_have_children_rb_tree(node);
>  }
> -- 
> 2.0.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf ui/tui: Fix a small callchain display bug
  2014-08-13  8:19 ` Ingo Molnar
@ 2014-08-13 20:12   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-08-13 20:12 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Namhyung Kim, Peter Zijlstra, Paul Mackerras, Namhyung Kim, LKML,
	Jiri Olsa, Andi Kleen, Frederic Weisbecker

Em Wed, Aug 13, 2014 at 10:19:09AM +0200, Ingo Molnar escreveu:
> * Namhyung Kim <namhyung@kernel.org> wrote:
> > Before:

> >   Children      Self  Command  Shared Object      Symbols
> > ---------------------------------------------------------------------------
> > -   40.70%     0.00%  swapper  [kernel.kallsyms]  [k] cpuidle_wrap_enter
> >    + cpuidle_wrap_enter
> >    + cpuidle_enter_tk
> >    + cpuidle_idle_call
> >    + cpu_idle

> > After:

> >   Children      Self  Command  Shared Object      Symbols
> > ---------------------------------------------------------------------------
> > -   40.70%     0.00%  swapper  [kernel.kallsyms]  [k] cpuidle_wrap_enter
> >      cpuidle_wrap_enter
> >      cpuidle_enter_tk
> >      cpuidle_idle_call
> >    + cpu_idle
> > 
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Nice fix!

Indeed, longstanding problem, kudos to Namhyung for deving into TUI +
callchain land to fix this!

- Arnaldo
 
> Acked-by: Ingo Molnar <mingo@kernel.org>
> 
> Thanks,
> 
> 	Ingo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip:perf/core] perf hists browser: Fix a small callchain display bug
  2014-08-13  6:02 [PATCH] perf ui/tui: Fix a small callchain display bug Namhyung Kim
  2014-08-13  8:19 ` Ingo Molnar
  2014-08-13 20:06 ` Arnaldo Carvalho de Melo
@ 2014-08-14  8:48 ` tip-bot for Namhyung Kim
  2 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Namhyung Kim @ 2014-08-14  8:48 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, andi, a.p.zijlstra,
	namhyung.kim, namhyung, jolsa, fweisbec, tglx

Commit-ID:  82162b5ae3d152fd7d887b36213f5b6785fe1294
Gitweb:     http://git.kernel.org/tip/82162b5ae3d152fd7d887b36213f5b6785fe1294
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Wed, 13 Aug 2014 15:02:41 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 13 Aug 2014 17:12:56 -0300

perf hists browser: Fix a small callchain display bug

The currently when perf TUI report shows callchain, the first level
chains have bogus '+' sign even though only the last one has children.

Since they are on a single line of the chain, toggling intermediate
entries has no effect.  Fix it to show '+' sign at the last entry only.
Note that non-first level callchain entries don't have this problem.

  Before:
  ---------------------------------------------------------------------------
    Children      Self  Command  Shared Object      Symbols
  -   40.70%     0.00%  swapper  [kernel.kallsyms]  [k] cpuidle_wrap_enter
     + cpuidle_wrap_enter
     + cpuidle_enter_tk
     + cpuidle_idle_call
     + cpu_idle

  After:
  ---------------------------------------------------------------------------
    Children      Self  Command  Shared Object      Symbols
  -   40.70%     0.00%  swapper  [kernel.kallsyms]  [k] cpuidle_wrap_enter
       cpuidle_wrap_enter
       cpuidle_enter_tk
       cpuidle_idle_call
     + cpu_idle

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Link: http://lkml.kernel.org/r/1407909761-10822-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 045c1e1..1818d12 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -228,8 +228,10 @@ static void callchain_node__init_have_children(struct callchain_node *node)
 {
 	struct callchain_list *chain;
 
-	list_for_each_entry(chain, &node->val, list)
+	if (!list_empty(&node->val)) {
+		chain = list_entry(node->val.prev, struct callchain_list, list);
 		chain->ms.has_children = !RB_EMPTY_ROOT(&node->rb_root);
+	}
 
 	callchain_node__init_have_children_rb_tree(node);
 }

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-08-14  8:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-13  6:02 [PATCH] perf ui/tui: Fix a small callchain display bug Namhyung Kim
2014-08-13  8:19 ` Ingo Molnar
2014-08-13 20:12   ` Arnaldo Carvalho de Melo
2014-08-13 20:06 ` Arnaldo Carvalho de Melo
2014-08-14  8:48 ` [tip:perf/core] perf hists browser: " tip-bot for Namhyung Kim

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.