From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754316AbbLJHyQ (ORCPT ); Thu, 10 Dec 2015 02:54:16 -0500 Received: from LGEAMRELO12.lge.com ([156.147.23.52]:33594 "EHLO lgeamrelo12.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753736AbbLJHxn (ORCPT ); Thu, 10 Dec 2015 02:53:43 -0500 X-Original-SENDERIP: 156.147.1.121 X-Original-MAILFROM: namhyung@kernel.org X-Original-SENDERIP: 10.177.227.17 X-Original-MAILFROM: namhyung@kernel.org From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , David Ahern , Frederic Weisbecker , Andi Kleen , Stephane Eranian , Adrian Hunter Subject: [PATCH/RFC 02/16] perf top: Fix and cleanup perf_top__record_precise_ip() Date: Thu, 10 Dec 2015 16:53:21 +0900 Message-Id: <1449734015-9148-3-git-send-email-namhyung@kernel.org> X-Mailer: git-send-email 2.6.2 In-Reply-To: <1449734015-9148-1-git-send-email-namhyung@kernel.org> References: <1449734015-9148-1-git-send-email-namhyung@kernel.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At first, it has duplicate ui__has_annotation() and 'sort__has_sym' and 'use_browser' check. In fact, the ui__has_annotation() should be removed as it needs to annotate on --stdio as well. And the top->sym_filter_entry is used only for stdio mode, so make it clear on the condition too. Also the check will be simplifed if it sets sym before the check. And omit check for 'he' since its caller (hist_iter__top_callback) only gets called when iter->he is not NULL. Secondly, it converts the 'ip' argument using map__unmap_ip() before the call and then reverts it using map__map_ip(). This seems meaningless and strange since only one of them checks the 'map'. Finally, access the he->hists->lock only if there's an error. Signed-off-by: Namhyung Kim --- tools/perf/builtin-top.c | 52 ++++++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index 7e2e72e6d9d1..7cd9bb69f5a6 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -175,42 +175,40 @@ static void perf_top__record_precise_ip(struct perf_top *top, int counter, u64 ip) { struct annotation *notes; - struct symbol *sym; + struct symbol *sym = he->ms.sym; int err = 0; - if (he == NULL || he->ms.sym == NULL || - ((top->sym_filter_entry == NULL || - top->sym_filter_entry->ms.sym != he->ms.sym) && use_browser != 1)) + if (sym == NULL || (use_browser == 0 && + (top->sym_filter_entry == NULL || + top->sym_filter_entry->ms.sym != sym))) return; - sym = he->ms.sym; notes = symbol__annotation(sym); if (pthread_mutex_trylock(¬es->lock)) return; - ip = he->ms.map->map_ip(he->ms.map, ip); - - if (ui__has_annotation()) - err = hist_entry__inc_addr_samples(he, counter, ip); + err = hist_entry__inc_addr_samples(he, counter, ip); pthread_mutex_unlock(¬es->lock); - /* - * This function is now called with he->hists->lock held. - * Release it before going to sleep. - */ - pthread_mutex_unlock(&he->hists->lock); + if (unlikely(err)) { + /* + * This function is now called with hists->lock held. + * Release it before going to sleep. + */ + pthread_mutex_unlock(&he->hists->lock); + + if (err == -ERANGE && !he->ms.map->erange_warned) + ui__warn_map_erange(he->ms.map, sym, ip); + else if (err == -ENOMEM) { + pr_err("Not enough memory for annotating '%s' symbol!\n", + sym->name); + sleep(1); + } - if (err == -ERANGE && !he->ms.map->erange_warned) - ui__warn_map_erange(he->ms.map, sym, ip); - else if (err == -ENOMEM) { - pr_err("Not enough memory for annotating '%s' symbol!\n", - sym->name); - sleep(1); + pthread_mutex_lock(&he->hists->lock); } - - pthread_mutex_lock(&he->hists->lock); } static void perf_top__show_details(struct perf_top *top) @@ -687,14 +685,8 @@ static int hist_iter__top_callback(struct hist_entry_iter *iter, struct hist_entry *he = iter->he; struct perf_evsel *evsel = iter->evsel; - if (sort__has_sym && single) { - u64 ip = al->addr; - - if (al->map) - ip = al->map->unmap_ip(al->map, ip); - - perf_top__record_precise_ip(top, he, evsel->idx, ip); - } + if (sort__has_sym && single) + perf_top__record_precise_ip(top, he, evsel->idx, al->addr); hist__account_cycles(iter->sample->branch_stack, al, iter->sample, !(top->record_opts.branch_stack & PERF_SAMPLE_BRANCH_ANY)); -- 2.6.2