From 5676dba708bbb1fc94a9d3b2e9c114db9e4c6699 Mon Sep 17 00:00:00 2001 From: Yang Jihong Date: Fri, 19 Mar 2021 20:35:27 +0800 Subject: [PATCH] perf annotate: Fix sample events lost in stdio mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In hist__find_annotations(), since different 'struct hist_entry' entries may point to same symbol, we free notes->src to signal already processed this symbol in stdio mode; when annotate, entry will skipped if notes->src is NULL to avoid repeated output. However, there is a problem, for example, run the following command: # perf record -e branch-misses -e branch-instructions -a sleep 1 perf.data file contains different types of sample event. If the same IP sample event exists in branch-misses and branch-instructions, this event uses the same symbol. When annotate branch-misses events, notes->src corresponding to this event is set to null, as a result, when annotate branch-instructions events, this event is skipped and no annotate is output. Solution of this patch is to remove zfree in hists__find_annotations and change sort order to "dso,symbol" to avoid duplicate output when different processes correspond to the same symbol. Signed-off-by: Yang Jihong Acked-by: Namhyung Kim Cc: Alexander Shishkin Cc: Gustavo A. R. Silva Cc: Jin Yao Cc: Jiri Olsa Cc: Mark Rutland Cc: Martin Liška Cc: Peter Zijlstra Cc: zhangjinhao2@huawei.com Link: http://lore.kernel.org/lkml/20210319123527.173883-1-yangjihong1@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-annotate.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 021d974c978e5..524e6f0dff22e 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -374,13 +374,6 @@ static void hists__find_annotations(struct hists *hists, } else { hist_entry__tty_annotate(he, evsel, ann); nd = rb_next(nd); - /* - * Since we have a hist_entry per IP for the same - * symbol, free he->ms.sym->src to signal we already - * processed this symbol. - */ - zfree(¬es->src->cycles_hist); - zfree(¬es->src); } } } @@ -623,14 +616,22 @@ int cmd_annotate(int argc, const char **argv) setup_browser(true); - if ((use_browser == 1 || annotate.use_stdio2) && annotate.has_br_stack) { + /* + * Events of different processes may correspond to the same + * symbol, we do not care about the processes in annotate, + * set sort order to avoid repeated output. + */ + sort_order = "dso,symbol"; + + /* + * Set SORT_MODE__BRANCH so that annotate display IPC/Cycle + * if branch info is in perf data in TUI mode. + */ + if ((use_browser == 1 || annotate.use_stdio2) && annotate.has_br_stack) sort__mode = SORT_MODE__BRANCH; - if (setup_sorting(annotate.session->evlist) < 0) - usage_with_options(annotate_usage, options); - } else { - if (setup_sorting(NULL) < 0) - usage_with_options(annotate_usage, options); - } + + if (setup_sorting(NULL) < 0) + usage_with_options(annotate_usage, options); ret = __cmd_annotate(&annotate);