Messages in this thread Patch in this message | | | From | Namhyung Kim <> | Subject | [PATCH/RFC 02/16] perf top: Fix and cleanup perf_top__record_precise_ip() | Date | Thu, 10 Dec 2015 16:53:21 +0900 |
| |
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 <namhyung@kernel.org> --- 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
| |