Re: [PATCH v2 10/10] perf tools: Move TUI-specific fields out of map_symbol

[Namhyung Kim <namhyung@kernel.org>]

On Tue, May 05, 2015 at 11:26:26AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 05, 2015 at 11:22:31AM -0300, Arnaldo Carvalho de Melo escreveu:
> > We will need a v3, this fixes the 'E'xpand segfault, but not the first
> > problem reported, again, this time step by step:
>  
> > Samples: 1K of event 'cycles', Event count (approx.): 1597853394
> >   Children      Self  Command  Shared Object               Symbol
> > -   99.86%    99.86%  swapper  [kernel.vmlinux]            [k] cpu_startup_entry
> >    + cpu_startup_entry
> > +   90.17%     0.00%  swapper  [kernel.vmlinux]            [k] start_secondary
> > ---------------------------------------
> > See the "+ cpu_startup_entry"? If I go there and press enter, I would expect to
> > see its callers, but what happens is:
>  
> > Samples: 1K of event 'cycles', Event count (approx.): 1597853394
> >   Children      Self  Command  Shared Object               Symbol
> > +   99.86%    99.86%  swapper  [kernel.vmlinux]            [k] cpu_startup_entry
> > +   90.17%     0.00%  swapper  [kernel.vmlinux]            [k] start_secondary
> > ---------------------------------------
>  
> > It collapses the hist_entry instead of expanding it further.
> 
> I just repeated the bisect, because I was unsure if last time I did it
> using the 'E' as the good/bad determiner, the
> collapse-when-it-should-expand bug described above is indeed also
> introduced by this changeset.

Okay, this is the fix.

I need to check whether the browser->selection is came from hist entry
or callchain list.  Will send v3 soon.

Thanks,
Namhyung


diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 7a70493e938c..f981cb8f0158 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -221,6 +221,18 @@ static bool hist_entry__toggle_fold(struct hist_entry *he)
 	return true;
 }
 
+static bool callchain_list__toggle_fold(struct callchain_list *cl)
+{
+	if (!cl)
+		return false;
+
+	if (!cl->has_children)
+		return false;
+
+	cl->unfolded = !cl->unfolded;
+	return true;
+}
+
 static void callchain_node__init_have_children_rb_tree(struct callchain_node *node)
 {
 	struct rb_node *nd = rb_first(&node->rb_root);
@@ -282,9 +294,17 @@ static void hist_entry__init_have_children(struct hist_entry *he)
 
 static bool hist_browser__toggle_fold(struct hist_browser *browser)
 {
-	if (hist_entry__toggle_fold(browser->he_selection)) {
-		struct hist_entry *he = browser->he_selection;
+	struct hist_entry *he = browser->he_selection;
+	struct map_symbol *ms = browser->selection;
+	struct callchain_list *cl = container_of(ms, struct callchain_list, ms);
+	bool has_children;
+
+	if (ms == &he->ms)
+		has_children = hist_entry__toggle_fold(he);
+	else
+		has_children = callchain_list__toggle_fold(cl);
 
+	if (has_children) {
 		hist_entry__init_have_children(he);
 		browser->b.nr_entries -= he->nr_rows;
 		browser->nr_callchain_rows -= he->nr_rows;