From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2994061AbbEEQHu (ORCPT ); Tue, 5 May 2015 12:07:50 -0400 Received: from mail-pa0-f44.google.com ([209.85.220.44]:35277 "EHLO mail-pa0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2993216AbbEEOlq (ORCPT ); Tue, 5 May 2015 10:41:46 -0400 Date: Tue, 5 May 2015 23:40:29 +0900 From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , David Ahern Subject: Re: [PATCH v2 10/10] perf tools: Move TUI-specific fields out of map_symbol Message-ID: <20150505144029.GB1402@danjae.kornet> References: <20150504155116.GE10475@kernel.org> <1430788690-13772-1-git-send-email-namhyung@kernel.org> <20150505142231.GL10475@kernel.org> <20150505142626.GN10475@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150505142626.GN10475@kernel.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.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;