Commit be5863b7 authored by Namhyung Kim's avatar Namhyung Kim Committed by Arnaldo Carvalho de Melo

perf top: Fix event group with more than two events

The event group feature links relevant hist entries among events so that
they can be displayed together.  During the link process, each hist
entry in non-leader events is connected to a hist entry in the leader
event.  This is done in order of events specified in the command line so
it assumes that events are linked in the order.

But 'perf top' can break the assumption since it does the link process
multiple times.  For example, a hist entry can be in the third event
only at first so it's linked after the leader.  Some time later, second
event has a hist entry for it and it'll be linked after the entry of the
third event.

This makes the code compilicated to deal with such unordered entries.
This patch simply unlink all the entries after it's printed so that they
can assume the correct order after the repeated link process.  Also it'd
be easy to deal with decaying old entries IMHO.
Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
Reported-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Link: http://lkml.kernel.org/r/20190827231555.121411-2-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent ea4385f8
...@@ -273,6 +273,12 @@ static void perf_top__resort_hists(struct perf_top *t) ...@@ -273,6 +273,12 @@ static void perf_top__resort_hists(struct perf_top *t)
evlist__for_each_entry(evlist, pos) { evlist__for_each_entry(evlist, pos) {
struct hists *hists = evsel__hists(pos); struct hists *hists = evsel__hists(pos);
/*
* unlink existing entries so that they can be linked
* in a correct order in hists__match() below.
*/
hists__unlink(hists);
if (evlist->enabled) { if (evlist->enabled) {
if (t->zero) { if (t->zero) {
hists__delete_entries(hists); hists__delete_entries(hists);
......
...@@ -2439,7 +2439,7 @@ void hists__match(struct hists *leader, struct hists *other) ...@@ -2439,7 +2439,7 @@ void hists__match(struct hists *leader, struct hists *other)
{ {
struct rb_root_cached *root; struct rb_root_cached *root;
struct rb_node *nd; struct rb_node *nd;
struct hist_entry *pos, *pair, *pos_pair, *tmp_pair; struct hist_entry *pos, *pair;
if (symbol_conf.report_hierarchy) { if (symbol_conf.report_hierarchy) {
/* hierarchy report always collapses entries */ /* hierarchy report always collapses entries */
...@@ -2456,25 +2456,9 @@ void hists__match(struct hists *leader, struct hists *other) ...@@ -2456,25 +2456,9 @@ void hists__match(struct hists *leader, struct hists *other)
pos = rb_entry(nd, struct hist_entry, rb_node_in); pos = rb_entry(nd, struct hist_entry, rb_node_in);
pair = hists__find_entry(other, pos); pair = hists__find_entry(other, pos);
if (pair && list_empty(&pair->pairs.node)) { if (pair)
list_for_each_entry_safe(pos_pair, tmp_pair, &pos->pairs.head, pairs.node) {
if (pos_pair->hists == other) {
/*
* XXX maybe decayed entries can appear
* here? but then we would have use
* after free, as decayed entries are
* freed see hists__delete_entry
*/
BUG_ON(!pos_pair->dummy);
list_del_init(&pos_pair->pairs.node);
hist_entry__delete(pos_pair);
break;
}
}
hist_entry__add_pair(pair, pos); hist_entry__add_pair(pair, pos);
} }
}
} }
static int hists__link_hierarchy(struct hists *leader_hists, static int hists__link_hierarchy(struct hists *leader_hists,
...@@ -2558,6 +2542,25 @@ int hists__link(struct hists *leader, struct hists *other) ...@@ -2558,6 +2542,25 @@ int hists__link(struct hists *leader, struct hists *other)
return 0; return 0;
} }
int hists__unlink(struct hists *hists)
{
struct rb_root_cached *root;
struct rb_node *nd;
struct hist_entry *pos;
if (hists__has(hists, need_collapse))
root = &hists->entries_collapsed;
else
root = hists->entries_in;
for (nd = rb_first_cached(root); nd; nd = rb_next(nd)) {
pos = rb_entry(nd, struct hist_entry, rb_node_in);
list_del_init(&pos->pairs.node);
}
return 0;
}
void hist__account_cycles(struct branch_stack *bs, struct addr_location *al, void hist__account_cycles(struct branch_stack *bs, struct addr_location *al,
struct perf_sample *sample, bool nonany_branch_mode) struct perf_sample *sample, bool nonany_branch_mode)
{ {
......
...@@ -217,6 +217,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *he); ...@@ -217,6 +217,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *he);
void hists__match(struct hists *leader, struct hists *other); void hists__match(struct hists *leader, struct hists *other);
int hists__link(struct hists *leader, struct hists *other); int hists__link(struct hists *leader, struct hists *other);
int hists__unlink(struct hists *hists);
struct hists_evsel { struct hists_evsel {
struct evsel evsel; struct evsel evsel;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment