Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752114AbdHFV0d (ORCPT ); Sun, 6 Aug 2017 17:26:33 -0400 Received: from mail.kdab.com ([176.9.126.58]:46140 "EHLO mail.kdab.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751681AbdHFVZK (ORCPT ); Sun, 6 Aug 2017 17:25:10 -0400 From: Milian Wolff To: acme@kernel.org, Jin Yao , Linux-kernel@vger.kernel.org, Namhyung Kim Cc: linux-perf-users@vger.kernel.org, Milian Wolff , Arnaldo Carvalho de Melo , David Ahern , Peter Zijlstra Subject: [PATCH v2 09/14] perf report: properly handle branch count in match_chain Date: Sun, 6 Aug 2017 23:24:41 +0200 Message-Id: <20170806212446.24925-10-milian.wolff@kdab.com> In-Reply-To: <20170806212446.24925-1-milian.wolff@kdab.com> References: <20170806212446.24925-1-milian.wolff@kdab.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4418 Lines: 158 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. Cc: Arnaldo Carvalho de Melo Cc: David Ahern Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Yao Jin Signed-off-by: Milian Wolff --- tools/perf/util/callchain.c | 115 +++++++++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 56 deletions(-) diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c index 45a39260a821..2c85adc1a3e2 100644 --- a/tools/perf/util/callchain.c +++ b/tools/perf/util/callchain.c @@ -659,77 +659,80 @@ static enum match_result match_chain_strings(const char *left, return ret; } +static enum match_result match_address(u64 left, u64 right) +{ + if (left == right) + return MATCH_EQ; + else if (left < right) + return MATCH_LT; + else + return MATCH_GT; +} + static enum match_result match_chain(struct callchain_cursor_node *node, struct callchain_list *cnode) { - struct symbol *sym = node->sym; - enum match_result match; - u64 left, right; + enum match_result match = MATCH_ERROR; - if (callchain_param.key == CCKEY_SRCLINE) { + switch (callchain_param.key) { + case CCKEY_SRCLINE: match = match_chain_strings(cnode->srcline, node->srcline); - - // if no srcline is available, fallback to symbol name - if (match == MATCH_ERROR && cnode->ms.sym && node->sym) - match = match_chain_strings(cnode->ms.sym->name, - node->sym->name); - if (match != MATCH_ERROR) - return match; - - // otherwise fall-back to IP-based comparison below - } - - if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) { - // compare inlined frames based on their symbol name because - // different inlined frames will have the same symbol start - if (cnode->ms.sym->inlined || node->sym->inlined) - return match_chain_strings(cnode->ms.sym->name, - node->sym->name); - - left = cnode->ms.sym->start; - right = sym->start; - } else { - left = cnode->ip; - right = node->ip; + break; + // else fall-through + case CCKEY_FUNCTION: + if (node->sym && cnode->ms.sym) { + // compare inlined frames based on their symbol name + // 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) + match = match_chain_strings(cnode->ms.sym->name, + node->sym->name); + else + match = match_address(cnode->ms.sym->start, + node->sym->start); + if (match != MATCH_ERROR) + break; + } + // else fall-through + case CCKEY_ADDRESS: + default: + match = match_address(cnode->ip, node->ip); + break; } - if (left == right) { - if (node->branch) { - cnode->branch_count++; + if (match == MATCH_EQ && node->branch) { + cnode->branch_count++; - if (node->branch_from) { - /* - * It's "to" of a branch - */ - cnode->brtype_stat.branch_to = true; + if (node->branch_from) { + /* + * It's "to" of a branch + */ + cnode->brtype_stat.branch_to = true; - if (node->branch_flags.predicted) - cnode->predicted_count++; + if (node->branch_flags.predicted) + cnode->predicted_count++; - if (node->branch_flags.abort) - cnode->abort_count++; + if (node->branch_flags.abort) + cnode->abort_count++; - branch_type_count(&cnode->brtype_stat, - &node->branch_flags, - node->branch_from, - node->ip); - } else { - /* - * It's "from" of a branch - */ - cnode->brtype_stat.branch_to = false; - cnode->cycles_count += - node->branch_flags.cycles; - cnode->iter_count += node->nr_loop_iter; - cnode->samples_count += node->samples; - } + branch_type_count(&cnode->brtype_stat, + &node->branch_flags, + node->branch_from, + node->ip); + } else { + /* + * It's "from" of a branch + */ + cnode->brtype_stat.branch_to = false; + cnode->cycles_count += node->branch_flags.cycles; + cnode->iter_count += node->nr_loop_iter; + cnode->samples_count += node->samples; } - - return MATCH_EQ; } - return left > right ? MATCH_GT : MATCH_LT; + return match; } /* -- 2.13.3