* The issue with `perf report -s comm`
@ 2017-09-21 14:51 Alexander Pozdneev
2017-10-04 6:13 ` [RFC] perf callchain: Compare dsos (as well) for CCKEY_FUNCTION Ravi Bangoria
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Pozdneev @ 2017-09-21 14:51 UTC (permalink / raw)
To: linux-perf-users
Hello,
I have the following issue with perf `report -s comm` that seems to be
a bug. This is how the code looks like:
int main() {
double a, b, c;
a = do_things_main(WAIT_TIME * 2); // 40% of time, a.out
b = do_things1(WAIT_TIME); // 20% of time, libdo1.so
c = do_things2(WAIT_TIME * 2); // 20% of time, libdo2.so
return (int)(a + b + c);
}
This is how I run it:
$ LD_LIBRARY_PATH="." perf record -g ./a.out
[ perf record: Woken up 3 times to write data ]
[ perf record: Captured and wrote 0.743 MB perf.data (11911 samples) ]
This is the regular output of `perf report`:
$ perf report | grep -v '#' | head -n 12
39.99% 39.99% a.out a.out [.] main
|
---main
39.94% 39.94% a.out libdo2.so [.] do_things2
|
---do_things2
20.05% 20.05% a.out libdo1.so [.] do_things1
|
---do_things1
It looks exactly as expected. If I sort by `dso`, the output is also OK:
$ perf report -s dso | grep -v '#'
39.99% 39.99% a.out
|
---main
39.94% 39.94% libdo2.so
|
---do_things2
20.05% 20.05% libdo1.so
|
---do_things1
However, If I try to sort by `comm`, the output looks strange:
$ perf report -s comm | grep -v '#'
100.00% 100.00% a.out
|
|--59.99%--do_things1
|
--39.99%--main
Specifically, `do_things2()` is missing in the report.
Here is the full source code of this example:
https://github.com/pozdneev/perf-report-s-comm-bug
Originally, Louis Stuber (IBM Client Center Montpellier) discovered
this behaviour when we have been playing with Flame Graphs
(http://www.brendangregg.com/blog/2016-04-30/linux-perf-folded.html).
This is the list of Linux/perf versions that I've tried:
* Ubuntu 14.04.5 (3.19.0-80-generic), perf version 3.19.8-ckt22
* RHEL 7.3 ppc64le (3.10.0-514.el7.ppc64le), perf version
3.10.0-514.el7.ppc64le.debug
* My colleague confirms that he observes the same behavior with kernel
version 4.13.
Could you please check if the issue remains in the current development
version of Linux perf?
Thanks!
Alexander
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC] perf callchain: Compare dsos (as well) for CCKEY_FUNCTION
2017-09-21 14:51 The issue with `perf report -s comm` Alexander Pozdneev
@ 2017-10-04 6:13 ` Ravi Bangoria
2017-10-04 13:08 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Ravi Bangoria @ 2017-10-04 6:13 UTC (permalink / raw)
To: pozdneyev, acme, linux-kernel
Cc: linux-perf-users, peterz, mingo, alexander.shishkin, yao.jin, ak,
jolsa, kjlx, milian.wolff, zhangmengting, Ravi Bangoria
Two functions from different binaries can have same start
address. Thus, comparing only start address in match_chain()
leads to inconsistent callchains. Fix this by adding a check
for dsos as well.
Ex, https://www.spinics.net/lists/linux-perf-users/msg04067.html
Reported-by: Alexander Pozdneev <pozdneyev@gmail.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
tools/perf/util/callchain.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 510b513..6d5a483 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -678,6 +678,9 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
{
struct symbol *sym = node->sym;
u64 left, right;
+ struct dso *left_dso = NULL;
+ struct dso *right_dso = NULL;
+
if (callchain_param.key == CCKEY_SRCLINE) {
enum match_result match = match_chain_srcline(node, cnode);
@@ -689,12 +692,16 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) {
left = cnode->ms.sym->start;
right = sym->start;
+ if (cnode->ms.map && node->map) {
+ left_dso = cnode->ms.map->dso;
+ right_dso = node->map->dso;
+ }
} else {
left = cnode->ip;
right = node->ip;
}
- if (left == right) {
+ if (left == right && left_dso == right_dso) {
if (node->branch) {
cnode->branch_count++;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC] perf callchain: Compare dsos (as well) for CCKEY_FUNCTION
2017-10-04 6:13 ` [RFC] perf callchain: Compare dsos (as well) for CCKEY_FUNCTION Ravi Bangoria
@ 2017-10-04 13:08 ` Jiri Olsa
2017-10-05 3:50 ` [PATCH] " Ravi Bangoria
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2017-10-04 13:08 UTC (permalink / raw)
To: Ravi Bangoria
Cc: pozdneyev, acme, linux-kernel, linux-perf-users, peterz, mingo,
alexander.shishkin, yao.jin, ak, jolsa, kjlx, milian.wolff,
zhangmengting
On Wed, Oct 04, 2017 at 11:43:08AM +0530, Ravi Bangoria wrote:
> Two functions from different binaries can have same start
> address. Thus, comparing only start address in match_chain()
> leads to inconsistent callchains. Fix this by adding a check
> for dsos as well.
>
> Ex, https://www.spinics.net/lists/linux-perf-users/msg04067.html
>
> Reported-by: Alexander Pozdneev <pozdneyev@gmail.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
> tools/perf/util/callchain.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index 510b513..6d5a483 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -678,6 +678,9 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
> {
> struct symbol *sym = node->sym;
> u64 left, right;
> + struct dso *left_dso = NULL;
> + struct dso *right_dso = NULL;
> +
>
> if (callchain_param.key == CCKEY_SRCLINE) {
> enum match_result match = match_chain_srcline(node, cnode);
> @@ -689,12 +692,16 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
> if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) {
> left = cnode->ms.sym->start;
> right = sym->start;
> + if (cnode->ms.map && node->map) {
> + left_dso = cnode->ms.map->dso;
> + right_dso = node->map->dso;
makes sense.. but why not to get those maps separately?
if (cnode->ms.map)
left_dso = cnode->ms.map->dso;
if (node->map) {
right_dso = node->map->dso;
I'd think that if one is missing, it's most likely different
map/dso and you want to fail the == check
jirka
> + }
> } else {
> left = cnode->ip;
> right = node->ip;
> }
>
> - if (left == right) {
> + if (left == right && left_dso == right_dso) {
> if (node->branch) {
> cnode->branch_count++;
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] perf callchain: Compare dsos (as well) for CCKEY_FUNCTION
2017-10-04 13:08 ` Jiri Olsa
@ 2017-10-05 3:50 ` Ravi Bangoria
2017-10-05 4:13 ` Namhyung Kim
0 siblings, 1 reply; 8+ messages in thread
From: Ravi Bangoria @ 2017-10-05 3:50 UTC (permalink / raw)
To: pozdneyev, acme, linux-kernel, jolsa
Cc: linux-perf-users, peterz, mingo, alexander.shishkin, yao.jin, ak,
kjlx, milian.wolff, zhangmengting, Ravi Bangoria
Two functions from different binaries can have same start
address. Thus, comparing only start address in match_chain()
leads to inconsistent callchains. Fix this by adding a check
for dsos as well.
Ex, https://www.spinics.net/lists/linux-perf-users/msg04067.html
Reported-by: Alexander Pozdneev <pozdneyev@gmail.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
tools/perf/util/callchain.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index be09d77..6d7f645 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -685,6 +685,9 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
{
struct symbol *sym = node->sym;
u64 left, right;
+ struct dso *left_dso = NULL;
+ struct dso *right_dso = NULL;
+
if (callchain_param.key == CCKEY_SRCLINE) {
enum match_result match = match_chain_srcline(node, cnode);
@@ -696,12 +699,16 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) {
left = cnode->ms.sym->start;
right = sym->start;
+ if (cnode->ms.map)
+ left_dso = cnode->ms.map->dso;
+ if (node->map)
+ right_dso = node->map->dso;
} else {
left = cnode->ip;
right = node->ip;
}
- if (left == right) {
+ if (left == right && left_dso == right_dso) {
if (node->branch) {
cnode->branch_count++;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] perf callchain: Compare dsos (as well) for CCKEY_FUNCTION
2017-10-05 3:50 ` [PATCH] " Ravi Bangoria
@ 2017-10-05 4:13 ` Namhyung Kim
2017-10-05 9:12 ` [PATCH v2] " Ravi Bangoria
0 siblings, 1 reply; 8+ messages in thread
From: Namhyung Kim @ 2017-10-05 4:13 UTC (permalink / raw)
To: Ravi Bangoria
Cc: pozdneyev, acme, linux-kernel, jolsa, linux-perf-users, peterz,
mingo, alexander.shishkin, yao.jin, ak, kjlx, milian.wolff,
zhangmengting, kernel-team
Hi,
On Thu, Oct 05, 2017 at 09:20:21AM +0530, Ravi Bangoria wrote:
> Two functions from different binaries can have same start
> address. Thus, comparing only start address in match_chain()
> leads to inconsistent callchains. Fix this by adding a check
> for dsos as well.
>
> Ex, https://www.spinics.net/lists/linux-perf-users/msg04067.html
>
> Reported-by: Alexander Pozdneev <pozdneyev@gmail.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
> tools/perf/util/callchain.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index be09d77..6d7f645 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -685,6 +685,9 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
> {
> struct symbol *sym = node->sym;
> u64 left, right;
> + struct dso *left_dso = NULL;
> + struct dso *right_dso = NULL;
> +
>
> if (callchain_param.key == CCKEY_SRCLINE) {
> enum match_result match = match_chain_srcline(node, cnode);
> @@ -696,12 +699,16 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
> if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) {
> left = cnode->ms.sym->start;
> right = sym->start;
> + if (cnode->ms.map)
> + left_dso = cnode->ms.map->dso;
> + if (node->map)
> + right_dso = node->map->dso;
AFAIK having a symbol guarantees having a map too. So it could simply
be:
left_dso = cnode->ms.map->dso;
right_dso = node->map->dso;
Thanks,
Namhyung
> } else {
> left = cnode->ip;
> right = node->ip;
> }
>
> - if (left == right) {
> + if (left == right && left_dso == right_dso) {
> if (node->branch) {
> cnode->branch_count++;
>
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] perf callchain: Compare dsos (as well) for CCKEY_FUNCTION
2017-10-05 4:13 ` Namhyung Kim
@ 2017-10-05 9:12 ` Ravi Bangoria
2017-10-05 9:26 ` Jiri Olsa
2017-10-05 18:12 ` [tip:perf/urgent] " tip-bot for Ravi Bangoria
0 siblings, 2 replies; 8+ messages in thread
From: Ravi Bangoria @ 2017-10-05 9:12 UTC (permalink / raw)
To: pozdneyev, acme, linux-kernel, jolsa, namhyung
Cc: linux-perf-users, peterz, mingo, alexander.shishkin, yao.jin, ak,
kjlx, milian.wolff, zhangmengting, Ravi Bangoria
Two functions from different binaries can have same start
address. Thus, comparing only start address in match_chain()
leads to inconsistent callchains. Fix this by adding a check
for dsos as well.
Ex, https://www.spinics.net/lists/linux-perf-users/msg04067.html
Reported-by: Alexander Pozdneev <pozdneyev@gmail.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
Changes in v2:
- Remove unnecessary checks for 'map'
tools/perf/util/callchain.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index be09d77..a971caf 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -685,6 +685,8 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
{
struct symbol *sym = node->sym;
u64 left, right;
+ struct dso *left_dso = NULL;
+ struct dso *right_dso = NULL;
if (callchain_param.key == CCKEY_SRCLINE) {
enum match_result match = match_chain_srcline(node, cnode);
@@ -696,12 +698,14 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) {
left = cnode->ms.sym->start;
right = sym->start;
+ left_dso = cnode->ms.map->dso;
+ right_dso = node->map->dso;
} else {
left = cnode->ip;
right = node->ip;
}
- if (left == right) {
+ if (left == right && left_dso == right_dso) {
if (node->branch) {
cnode->branch_count++;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] perf callchain: Compare dsos (as well) for CCKEY_FUNCTION
2017-10-05 9:12 ` [PATCH v2] " Ravi Bangoria
@ 2017-10-05 9:26 ` Jiri Olsa
2017-10-05 18:12 ` [tip:perf/urgent] " tip-bot for Ravi Bangoria
1 sibling, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2017-10-05 9:26 UTC (permalink / raw)
To: Ravi Bangoria
Cc: pozdneyev, acme, linux-kernel, jolsa, namhyung, linux-perf-users,
peterz, mingo, alexander.shishkin, yao.jin, ak, kjlx,
milian.wolff, zhangmengting
On Thu, Oct 05, 2017 at 02:42:34PM +0530, Ravi Bangoria wrote:
> Two functions from different binaries can have same start
> address. Thus, comparing only start address in match_chain()
> leads to inconsistent callchains. Fix this by adding a check
> for dsos as well.
>
> Ex, https://www.spinics.net/lists/linux-perf-users/msg04067.html
>
> Reported-by: Alexander Pozdneev <pozdneyev@gmail.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
> Changes in v2:
> - Remove unnecessary checks for 'map'
Acked-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
>
> tools/perf/util/callchain.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index be09d77..a971caf 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -685,6 +685,8 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
> {
> struct symbol *sym = node->sym;
> u64 left, right;
> + struct dso *left_dso = NULL;
> + struct dso *right_dso = NULL;
>
> if (callchain_param.key == CCKEY_SRCLINE) {
> enum match_result match = match_chain_srcline(node, cnode);
> @@ -696,12 +698,14 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
> if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) {
> left = cnode->ms.sym->start;
> right = sym->start;
> + left_dso = cnode->ms.map->dso;
> + right_dso = node->map->dso;
> } else {
> left = cnode->ip;
> right = node->ip;
> }
>
> - if (left == right) {
> + if (left == right && left_dso == right_dso) {
> if (node->branch) {
> cnode->branch_count++;
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:perf/urgent] perf callchain: Compare dsos (as well) for CCKEY_FUNCTION
2017-10-05 9:12 ` [PATCH v2] " Ravi Bangoria
2017-10-05 9:26 ` Jiri Olsa
@ 2017-10-05 18:12 ` tip-bot for Ravi Bangoria
1 sibling, 0 replies; 8+ messages in thread
From: tip-bot for Ravi Bangoria @ 2017-10-05 18:12 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, yao.jin, namhyung, tglx, alexander.shishkin, acme,
pozdneyev, ak, hpa, ravi.bangoria, peterz, milian.wolff, jolsa,
kjlx, linux-kernel
Commit-ID: c1fbc0cf81f1c464f5fda322c1104d4bb1da6711
Gitweb: https://git.kernel.org/tip/c1fbc0cf81f1c464f5fda322c1104d4bb1da6711
Author: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
AuthorDate: Thu, 5 Oct 2017 14:42:34 +0530
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 5 Oct 2017 10:52:54 -0300
perf callchain: Compare dsos (as well) for CCKEY_FUNCTION
Two functions from different binaries can have same start address. Thus,
comparing only start address in match_chain() leads to inconsistent
callchains. Fix this by adding a check for dsos as well.
Ex, https://www.spinics.net/lists/linux-perf-users/msg04067.html
Reported-by: Alexander Pozdneev <pozdneyev@gmail.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Krister Johansen <kjlx@templeofstupid.com>
Cc: Milian Wolff <milian.wolff@kdab.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Yao Jin <yao.jin@linux.intel.com>
Cc: zhangmengting@huawei.com
Link: http://lkml.kernel.org/r/20171005091234.5874-1-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/callchain.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index be09d77..a971caf 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -685,6 +685,8 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
{
struct symbol *sym = node->sym;
u64 left, right;
+ struct dso *left_dso = NULL;
+ struct dso *right_dso = NULL;
if (callchain_param.key == CCKEY_SRCLINE) {
enum match_result match = match_chain_srcline(node, cnode);
@@ -696,12 +698,14 @@ static enum match_result match_chain(struct callchain_cursor_node *node,
if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) {
left = cnode->ms.sym->start;
right = sym->start;
+ left_dso = cnode->ms.map->dso;
+ right_dso = node->map->dso;
} else {
left = cnode->ip;
right = node->ip;
}
- if (left == right) {
+ if (left == right && left_dso == right_dso) {
if (node->branch) {
cnode->branch_count++;
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-05 18:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-21 14:51 The issue with `perf report -s comm` Alexander Pozdneev
2017-10-04 6:13 ` [RFC] perf callchain: Compare dsos (as well) for CCKEY_FUNCTION Ravi Bangoria
2017-10-04 13:08 ` Jiri Olsa
2017-10-05 3:50 ` [PATCH] " Ravi Bangoria
2017-10-05 4:13 ` Namhyung Kim
2017-10-05 9:12 ` [PATCH v2] " Ravi Bangoria
2017-10-05 9:26 ` Jiri Olsa
2017-10-05 18:12 ` [tip:perf/urgent] " tip-bot for Ravi Bangoria
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.