Hello, On Fri, Mar 26, 2021 at 11:24 AM Yang Jihong <yangjiho...@huawei.com> wrote: > > Hello, > ping :) > > On 2021/3/19 20:35, Yang Jihong wrote: > > In hist__find_annotations function, since different hist_entry 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 <yangjiho...@huawei.com>
Acked-by: Namhyung Kim <namhy...@kernel.org> Thanks, Namhyung > > --- > > > > Changes since v6: > > - Remove separate setup_sorting() for branch mode. > > > > Changes since v5: > > - Add Signed-off-by tag. > > > > Changes since v4: > > - Use the same sort key "dso,symbol" in branch stack mode. > > > > Changes since v3: > > - Modify the first line of comments. > > > > Changes since v2: > > - Remove zfree in hists__find_annotations. > > - Change sort order to avoid duplicate output. > > > > Changes since v1: > > - Change processed flag variable from u8 to bool. > > > > 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 a23ba6bb99b6..795c2ac7fcd1 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); > > } > > } > > } > > @@ -619,14 +612,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); > > > >