On Wednesday, November 22, 2017 11:25:40 PM CET alexander.levin@xxxxxxxxxxx wrote: > From: Milian Wolff <milian.wolff@xxxxxxxx> > > [ Upstream commit 9856240ad3269f2fdab0b2fa4400ef8aab792061 ] Hello Alexander, this is the first time I encounter AUTOSEL. I just want to check: The patch below depends on others in a whole series that reworks the handling of inline frames. Why is only this one getting selected? I don't even think it can work stand-alone? Thanks > The fake symbols we create for inlined frames will represent different > functions but can use the symbol start address. This leads to issues > when different inline branches all lead to the same function. > > Before: > ~~~~~ > $ perf report -s sym -i perf.inlining.data --inline --stdio -g function > ... > --38.86%--_start > __libc_start_main > main > > --37.57%--std::norm<double> (inlined) > std::_Norm_helper<true>::_S_do_it<double> > (inlined) > > --36.36%--std::abs<double> (inlined) > std::__complex_abs (inlined) > > > --12.24%--std::linear_congruential_engine<unsigned long, 16807ul, 0ul, > 2147483647ul>::operator() (inlined) std::__detail::__mod<unsigned long, > 2147483647ul, 16807ul, 0ul> (inlined) std::__detail::_Mod<unsigned long, > 2147483647ul, 16807ul, 0ul, true, true>::__calc (inlined) ~~~~~ > > Note that this backtrace representation is completely bogus. > Complex abs does not call the linear congruential engine! It > is just a side-effect of a longer inlined stack being appended > to a shorter, different inlined stack, both of which originate > in the same function (main). > > This patch fixes the issue: > > ~~~~~ > $ perf report -s sym -i perf.inlining.data --inline --stdio -g function > ... > --38.86%--_start > __libc_start_main > main > > |--35.59%--std::uniform_real_distribution<double>::op > |erator()<std::linear_congruential_engine<unsigned > |long, 16807ul, 0ul, 2147483647ul> > (inlined) | > | std::uniform_real_distribution<double>::op > | erator()<std::linear_congruential_engine<u > | nsigned long, 16807ul, 0ul, 2147483647ul> > | > (inlined) | > | --34.37%--std::__detail::_Adaptor<std::li > | near_congruential_engine<unsigned long, > | 16807ul, 0ul, 2147483647ul>, > | double>::operator() (inlined) | > | std::generate_canonical<double, > | 53ul, > | std::linear_congruential_engin > | e<unsigned long, 16807ul, 0ul, > | 2147483647ul> > (inlined) | > | --12.24%--std::linear_congruen > | tial_engine<unsigned long, > | 16807ul, 0ul, > | 2147483647ul>::operator() > | (inlined) | > | std::__detail::__mod > | <unsigned long, > | 2147483647ul, > | 16807ul, 0ul> > | (inlined) > | std::__detail::_Mod< > | unsigned long, > | 2147483647ul, > | 16807ul, 0ul, true, > | true>::__calc > | (inlined) > --1.99%--std::norm<double> (inlined) > std::_Norm_helper<true>::_S_do_it<double> > (inlined) std::abs<double> (inlined) > std::__complex_abs (inlined) > ~~~~~ > > Signed-off-by: Milian Wolff <milian.wolff@xxxxxxxx> > Reviewed-by: Jiri Olsa <jolsa@xxxxxxxxxx> > Reviewed-by: Namhyung Kim <namhyung@xxxxxxxxxx> > Cc: David Ahern <dsahern@xxxxxxxxx> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > Cc: Ravi Bangoria <ravi.bangoria@xxxxxxxxxxxxxxxxxx> > Cc: Yao Jin <yao.jin@xxxxxxxxxxxxxxx> > Link: http://lkml.kernel.org/r/20171009203310.17362-10-milian.wolff@xxxxxxxx > Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> > [ Fix up conflict with c1fbc0cf81f1 ("perf callchain: Compare dsos (as well) > for CCKEY_FUNCTION"), remove unneeded hunk ] Signed-off-by: Arnaldo > Carvalho de Melo <acme@xxxxxxxxxx> > > Signed-off-by: Sasha Levin <alexander.levin@xxxxxxxxxxx> > --- > tools/perf/util/callchain.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c > index 6031933d811c..3cc51e852cf0 100644 > --- a/tools/perf/util/callchain.c > +++ b/tools/perf/util/callchain.c > @@ -697,6 +697,14 @@ static enum match_result match_chain(struct > callchain_cursor_node *node, } > > 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; > left_dso = cnode->ms.map->dso; -- Milian Wolff | milian.wolff@xxxxxxxx | Senior Software Engineer KDAB (Deutschland) GmbH&Co KG, a KDAB Group company Tel: +49-30-521325470 KDAB - The Qt Experts
Attachment:
smime.p7s
Description: S/MIME cryptographic signature