| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2891438.FCgzeiCqBH@milian-kdab2>
Date: Thu, 23 Nov 2017 09:55:37 +0100
From: Milian Wolff <milian.wolff@...b.com>
To: alexander.levin@...izon.com
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"stable@...r.kernel.org" <stable@...r.kernel.org>,
David Ahern <dsahern@...il.com>,
Peter Zijlstra <peterz@...radead.org>,
Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>,
Yao Jin <yao.jin@...ux.intel.com>,
Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH AUTOSEL for 4.14 18/51] perf callchain: Compare symbol name for inlined frames when matching
On Wednesday, November 22, 2017 11:25:40 PM CET alexander.levin@...izon.com
wrote:
> From: Milian Wolff <milian.wolff@...b.com>
>
> [ 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@...b.com>
> Reviewed-by: Jiri Olsa <jolsa@...hat.com>
> Reviewed-by: Namhyung Kim <namhyung@...nel.org>
> Cc: David Ahern <dsahern@...il.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
> Cc: Yao Jin <yao.jin@...ux.intel.com>
> Link: http://lkml.kernel.org/r/20171009203310.17362-10-milian.wolff@kdab.com
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> [ 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@...hat.com>
>
> Signed-off-by: Sasha Levin <alexander.levin@...izon.com>
> ---
> 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@...b.com | Senior Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
Download attachment "smime.p7s" of type "application/pkcs7-signature" (3826 bytes)
Powered by blists - more mailing lists