| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150428122919.GI16849@kernel.org>
Date: Tue, 28 Apr 2015 09:29:19 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Jiri Olsa <jolsa@...hat.com>,
LKML <linux-kernel@...r.kernel.org>,
David Ahern <dsahern@...il.com>
Subject: Re: [PATCH 10/10] perf tools: Move TUI-specific fields out of
map_symbol
Em Tue, Apr 28, 2015 at 04:30:29PM +0900, Namhyung Kim escreveu:
> Hi Arnaldo,
>
> On Mon, Apr 27, 2015 at 02:20:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Apr 22, 2015 at 04:18:21PM +0900, Namhyung Kim escreveu:
> >
> > > @@ -139,24 +139,19 @@ static char tree__folded_sign(bool unfolded)
> > > return unfolded ? '-' : '+';
> > > }
> > >
> > > -static char map_symbol__folded(const struct map_symbol *ms)
> > > -{
> > > - return ms->has_children ? tree__folded_sign(ms->unfolded) : ' ';
> > > -}
> > > -
> >
> > So what is wrong with renaming the above function to
> > hist_entry__folded() instead of open coding it multiple times? Applied
> > all the other patches, thanks,
>
> Please see below..
> The struct map_symbol__folded() was called from hist_entry__folded()
> and callchain_list__folded(). I just removed map_symbol__folded()
> since it does not contain the info anymore and open coded it just for
> the two callers. The real callers of these functions are not touched.
You are right, nevermind, ENOCOFFEE or something, applying!
- Arnaldo
> > > static char hist_entry__folded(const struct hist_entry *he)
> > > {
> > > - return map_symbol__folded(&he->ms);
> > > + return he->has_children ? tree__folded_sign(he->unfolded) : ' ';
> > > }
> > >
> > > static char callchain_list__folded(const struct callchain_list *cl)
> > > {
> > > - return map_symbol__folded(&cl->ms);
> > > + return cl->has_children ? tree__folded_sign(cl->unfolded) : ' ';
> > > }
>
> Here.
>
> Thanks,
> Namhyung
>
>
> > >
> > > -static void map_symbol__set_folding(struct map_symbol *ms, bool unfold)
> > > +static void callchain_list__set_folding(struct callchain_list *cl, bool unfold)
> > > {
> > > - ms->unfolded = unfold ? ms->has_children : false;
> > > + cl->unfolded = unfold ? cl->has_children : false;
> > > }
> > >
> > > static int callchain_node__count_rows_rb_tree(struct callchain_node *node)
> > > @@ -192,7 +187,7 @@ static int callchain_node__count_rows(struct callchain_node *node)
> > >
> > > list_for_each_entry(chain, &node->val, list) {
> > > ++n;
> > > - unfolded = chain->ms.unfolded;
> > > + unfolded = chain->unfolded;
> > > }
> > >
> > > if (unfolded)
> > > @@ -214,15 +209,15 @@ static int callchain__count_rows(struct rb_root *chain)
> > > return n;
> > > }
> > >
> > > -static bool map_symbol__toggle_fold(struct map_symbol *ms)
> > > +static bool hist_entry__toggle_fold(struct hist_entry *he)
> > > {
> > > - if (!ms)
> > > + if (!he)
> > > return false;
> > >
> > > - if (!ms->has_children)
> > > + if (!he->has_children)
> > > return false;
> > >
> > > - ms->unfolded = !ms->unfolded;
> > > + he->unfolded = !he->unfolded;
> > > return true;
> > > }
> > >
> > > @@ -238,10 +233,10 @@ static void callchain_node__init_have_children_rb_tree(struct callchain_node *no
> > > list_for_each_entry(chain, &child->val, list) {
> > > if (first) {
> > > first = false;
> > > - chain->ms.has_children = chain->list.next != &child->val ||
> > > + chain->has_children = chain->list.next != &child->val ||
> > > !RB_EMPTY_ROOT(&child->rb_root);
> > > } else
> > > - chain->ms.has_children = chain->list.next == &child->val &&
> > > + chain->has_children = chain->list.next == &child->val &&
> > > !RB_EMPTY_ROOT(&child->rb_root);
> > > }
> > >
> > > @@ -255,11 +250,11 @@ static void callchain_node__init_have_children(struct callchain_node *node,
> > > struct callchain_list *chain;
> > >
> > > chain = list_entry(node->val.next, struct callchain_list, list);
> > > - chain->ms.has_children = has_sibling;
> > > + chain->has_children = has_sibling;
> > >
> > > 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);
> > > + chain->has_children = !RB_EMPTY_ROOT(&node->rb_root);
> > > }
> > >
> > > callchain_node__init_have_children_rb_tree(node);
> > > @@ -279,7 +274,7 @@ static void callchain__init_have_children(struct rb_root *root)
> > > static void hist_entry__init_have_children(struct hist_entry *he)
> > > {
> > > if (!he->init_have_children) {
> > > - he->ms.has_children = !RB_EMPTY_ROOT(&he->sorted_chain);
> > > + he->has_children = !RB_EMPTY_ROOT(&he->sorted_chain);
> > > callchain__init_have_children(&he->sorted_chain);
> > > he->init_have_children = true;
> > > }
> > > @@ -287,14 +282,14 @@ static void hist_entry__init_have_children(struct hist_entry *he)
> > >
> > > static bool hist_browser__toggle_fold(struct hist_browser *browser)
> > > {
> > > - if (map_symbol__toggle_fold(browser->selection)) {
> > > + if (hist_entry__toggle_fold(browser->he_selection)) {
> > > struct hist_entry *he = browser->he_selection;
> > >
> > > hist_entry__init_have_children(he);
> > > browser->b.nr_entries -= he->nr_rows;
> > > browser->nr_callchain_rows -= he->nr_rows;
> > >
> > > - if (he->ms.unfolded)
> > > + if (he->unfolded)
> > > he->nr_rows = callchain__count_rows(&he->sorted_chain);
> > > else
> > > he->nr_rows = 0;
> > > @@ -321,8 +316,8 @@ static int callchain_node__set_folding_rb_tree(struct callchain_node *node, bool
> > >
> > > list_for_each_entry(chain, &child->val, list) {
> > > ++n;
> > > - map_symbol__set_folding(&chain->ms, unfold);
> > > - has_children = chain->ms.has_children;
> > > + callchain_list__set_folding(chain, unfold);
> > > + has_children = chain->has_children;
> > > }
> > >
> > > if (has_children)
> > > @@ -340,8 +335,8 @@ static int callchain_node__set_folding(struct callchain_node *node, bool unfold)
> > >
> > > list_for_each_entry(chain, &node->val, list) {
> > > ++n;
> > > - map_symbol__set_folding(&chain->ms, unfold);
> > > - has_children = chain->ms.has_children;
> > > + callchain_list__set_folding(chain, unfold);
> > > + has_children = chain->has_children;
> > > }
> > >
> > > if (has_children)
> > > @@ -366,9 +361,9 @@ static int callchain__set_folding(struct rb_root *chain, bool unfold)
> > > static void hist_entry__set_folding(struct hist_entry *he, bool unfold)
> > > {
> > > hist_entry__init_have_children(he);
> > > - map_symbol__set_folding(&he->ms, unfold);
> > > + hist_entry__set_folding(he, unfold);
> > >
> > > - if (he->ms.has_children) {
> > > + if (he->has_children) {
> > > int n = callchain__set_folding(&he->sorted_chain, unfold);
> > > he->nr_rows = unfold ? n : 0;
> > > } else
> > > @@ -1019,7 +1014,7 @@ do_offset:
> > > if (offset > 0) {
> > > do {
> > > h = rb_entry(nd, struct hist_entry, rb_node);
> > > - if (h->ms.unfolded) {
> > > + if (h->unfolded) {
> > > u16 remaining = h->nr_rows - h->row_offset;
> > > if (offset > remaining) {
> > > offset -= remaining;
> > > @@ -1040,7 +1035,7 @@ do_offset:
> > > } else if (offset < 0) {
> > > while (1) {
> > > h = rb_entry(nd, struct hist_entry, rb_node);
> > > - if (h->ms.unfolded) {
> > > + if (h->unfolded) {
> > > if (first) {
> > > if (-offset > h->row_offset) {
> > > offset += h->row_offset;
> > > @@ -1077,7 +1072,7 @@ do_offset:
> > > * row_offset at its last entry.
> > > */
> > > h = rb_entry(nd, struct hist_entry, rb_node);
> > > - if (h->ms.unfolded)
> > > + if (h->unfolded)
> > > h->row_offset = h->nr_rows;
> > > break;
> > > }
> > > diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
> > > index 6033a0a212ca..679c2c6d8ade 100644
> > > --- a/tools/perf/util/callchain.h
> > > +++ b/tools/perf/util/callchain.h
> > > @@ -72,6 +72,10 @@ extern struct callchain_param callchain_param;
> > > struct callchain_list {
> > > u64 ip;
> > > struct map_symbol ms;
> > > + struct /* for TUI */ {
> > > + bool unfolded;
> > > + bool has_children;
> > > + };
> > > char *srcline;
> > > struct list_head list;
> > > };
> > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > > index cc22b9158b93..338770679863 100644
> > > --- a/tools/perf/util/hist.c
> > > +++ b/tools/perf/util/hist.c
> > > @@ -1163,7 +1163,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
> > > return;
> > >
> > > /* force fold unfiltered entry for simplicity */
> > > - h->ms.unfolded = false;
> > > + h->unfolded = false;
> > > h->row_offset = 0;
> > > h->nr_rows = 0;
> > >
> > > diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> > > index 4d923e6e0069..e97cd476d336 100644
> > > --- a/tools/perf/util/sort.h
> > > +++ b/tools/perf/util/sort.h
> > > @@ -109,6 +109,8 @@ struct hist_entry {
> > > u16 row_offset;
> > > u16 nr_rows;
> > > bool init_have_children;
> > > + bool unfolded;
> > > + bool has_children;
> > > };
> > > };
> > > char *srcline;
> > > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> > > index 09561500164a..8483a864a915 100644
> > > --- a/tools/perf/util/symbol.h
> > > +++ b/tools/perf/util/symbol.h
> > > @@ -158,8 +158,6 @@ struct ref_reloc_sym {
> > > struct map_symbol {
> > > struct map *map;
> > > struct symbol *sym;
> > > - bool unfolded;
> > > - bool has_children;
> > > };
> > >
> > > struct addr_map_symbol {
> > > --
> > > 2.3.5
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists