Re: [PATCH AUTOSEL for 4.14 18/51] perf callchain: Compare symbol name for inlined frames when matching

From: Milian Wolff
Date: Thu Nov 23 2017 - 03:55:51 EST

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