Commit bf36eb5c authored by Milian Wolff's avatar Milian Wolff Committed by Arnaldo Carvalho de Melo

perf report: Properly handle branch count in match_chain()

Some of the code paths I introduced before returned too early without
running the code to handle a node's branch count.  By refactoring
match_chain to only have one exit point, this can be remedied.
Signed-off-by: default avatarMilian Wolff <milian.wolff@kdab.com>
Acked-by: default avatarNamhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1707691.qaJ269GSZW@agathebauer
Link: http://lkml.kernel.org/r/20171018185350.14893-2-milian.wolff@kdab.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent aa441895
...@@ -666,49 +666,69 @@ static enum match_result match_chain_strings(const char *left, ...@@ -666,49 +666,69 @@ static enum match_result match_chain_strings(const char *left,
return ret; return ret;
} }
static enum match_result match_chain(struct callchain_cursor_node *node, /*
struct callchain_list *cnode) * We need to always use relative addresses because we're aggregating
* callchains from multiple threads, i.e. different address spaces, so
* comparing absolute addresses make no sense as a symbol in a DSO may end up
* in a different address when used in a different binary or even the same
* binary but with some sort of address randomization technique, thus we need
* to compare just relative addresses. -acme
*/
static enum match_result match_chain_dso_addresses(struct map *left_map, u64 left_ip,
struct map *right_map, u64 right_ip)
{ {
struct symbol *sym = node->sym; struct dso *left_dso = left_map ? left_map->dso : NULL;
u64 left, right; struct dso *right_dso = right_map ? right_map->dso : NULL;
struct dso *left_dso = NULL;
struct dso *right_dso = NULL;
if (callchain_param.key == CCKEY_SRCLINE) { if (left_dso != right_dso)
enum match_result match = match_chain_strings(cnode->srcline, return left_dso < right_dso ? MATCH_LT : MATCH_GT;
node->srcline);
/* if no srcline is available, fallback to symbol name */ if (left_ip != right_ip)
if (match == MATCH_ERROR && cnode->ms.sym && node->sym) return left_ip < right_ip ? MATCH_LT : MATCH_GT;
match = match_chain_strings(cnode->ms.sym->name,
node->sym->name);
if (match != MATCH_ERROR) return MATCH_EQ;
return match; }
/* otherwise fall-back to IP-based comparison below */ static enum match_result match_chain(struct callchain_cursor_node *node,
} struct callchain_list *cnode)
{
enum match_result match = MATCH_ERROR;
if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) { switch (callchain_param.key) {
case CCKEY_SRCLINE:
match = match_chain_strings(cnode->srcline, node->srcline);
if (match != MATCH_ERROR)
break;
/* otherwise fall-back to symbol-based comparison below */
__fallthrough;
case CCKEY_FUNCTION:
if (node->sym && cnode->ms.sym) {
/* /*
* Compare inlined frames based on their symbol name because * Compare inlined frames based on their symbol name
* different inlined frames will have the same symbol start * because different inlined frames will have the same
* symbol start. Otherwise do a faster comparison based
* on the symbol start address.
*/ */
if (cnode->ms.sym->inlined || node->sym->inlined) if (cnode->ms.sym->inlined || node->sym->inlined) {
return match_chain_strings(cnode->ms.sym->name, match = match_chain_strings(cnode->ms.sym->name,
node->sym->name); node->sym->name);
if (match != MATCH_ERROR)
left = cnode->ms.sym->start; break;
right = sym->start;
left_dso = cnode->ms.map->dso;
right_dso = node->map->dso;
} else { } else {
left = cnode->ip; match = match_chain_dso_addresses(cnode->ms.map, cnode->ms.sym->start,
right = node->ip; node->map, node->sym->start);
break;
}
}
/* otherwise fall-back to IP-based comparison below */
__fallthrough;
case CCKEY_ADDRESS:
default:
match = match_chain_dso_addresses(cnode->ms.map, cnode->ip, node->map, node->ip);
break;
} }
if (left == right && left_dso == right_dso) { if (match == MATCH_EQ && node->branch) {
if (node->branch) {
cnode->branch_count++; cnode->branch_count++;
if (node->branch_from) { if (node->branch_from) {
...@@ -732,17 +752,13 @@ static enum match_result match_chain(struct callchain_cursor_node *node, ...@@ -732,17 +752,13 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
* It's "from" of a branch * It's "from" of a branch
*/ */
cnode->brtype_stat.branch_to = false; cnode->brtype_stat.branch_to = false;
cnode->cycles_count += cnode->cycles_count += node->branch_flags.cycles;
node->branch_flags.cycles;
cnode->iter_count += node->nr_loop_iter; cnode->iter_count += node->nr_loop_iter;
cnode->iter_cycles += node->iter_cycles; cnode->iter_cycles += node->iter_cycles;
} }
} }
return MATCH_EQ; return match;
}
return left > right ? MATCH_GT : MATCH_LT;
} }
/* /*
......
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