* [PATCH] perf tools: Fix "Command" sort_entry's cmp and collapse function
@ 2015-05-15 15:54 Jiri Olsa
2015-05-18 1:38 ` Namhyung Kim
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Olsa @ 2015-05-15 15:54 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Paul Mackerras, David Ahern, Namhyung Kim,
Ingo Molnar, Jan Stancek
Currently the se_cmp and se_collapse use pointer comparison,
which is ok for for testing equality of strings. It's not ok
as comparing function for rbtree insertion, because it gives
different results based on current pointer values.
We saw test 32 (hists cumulation test) failing based on different
environment setup. Having all sort functions straightened fix the
test for us.
Reported-by: Jan Stancek <jstancek@redhat.com>
Link: http://lkml.kernel.org/n/tip-tklp6y27bseqjibcwn0py9up@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/sort.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 4593f36ecc4c..09d4696fd9a1 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -89,14 +89,14 @@ static int64_t
sort__comm_cmp(struct hist_entry *left, struct hist_entry *right)
{
/* Compare the addr that should be unique among comm */
- return comm__str(right->comm) - comm__str(left->comm);
+ return strcmp(comm__str(right->comm), comm__str(left->comm));
}
static int64_t
sort__comm_collapse(struct hist_entry *left, struct hist_entry *right)
{
/* Compare the addr that should be unique among comm */
- return comm__str(right->comm) - comm__str(left->comm);
+ return strcmp(comm__str(right->comm), comm__str(left->comm));
}
static int64_t
--
1.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] perf tools: Fix "Command" sort_entry's cmp and collapse function
2015-05-15 15:54 [PATCH] perf tools: Fix "Command" sort_entry's cmp and collapse function Jiri Olsa
@ 2015-05-18 1:38 ` Namhyung Kim
2015-05-18 14:39 ` Jiri Olsa
0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2015-05-18 1:38 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Paul Mackerras,
David Ahern, Ingo Molnar, Jan Stancek, Frederic Weisbecker
Hi Jiri,
CC-ing Frederic as he wrote the comm change.
On Fri, May 15, 2015 at 05:54:28PM +0200, Jiri Olsa wrote:
> Currently the se_cmp and se_collapse use pointer comparison,
> which is ok for for testing equality of strings. It's not ok
> as comparing function for rbtree insertion, because it gives
> different results based on current pointer values.
>
> We saw test 32 (hists cumulation test) failing based on different
> environment setup. Having all sort functions straightened fix the
> test for us.
Can you elaborate it?
AFAIK comm string is shared among threads so pointer comparison and
'strcmp == 0' should have same result..
Thanks,
Namhyung
>
> Reported-by: Jan Stancek <jstancek@redhat.com>
> Link: http://lkml.kernel.org/n/tip-tklp6y27bseqjibcwn0py9up@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/util/sort.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 4593f36ecc4c..09d4696fd9a1 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -89,14 +89,14 @@ static int64_t
> sort__comm_cmp(struct hist_entry *left, struct hist_entry *right)
> {
> /* Compare the addr that should be unique among comm */
> - return comm__str(right->comm) - comm__str(left->comm);
> + return strcmp(comm__str(right->comm), comm__str(left->comm));
> }
>
> static int64_t
> sort__comm_collapse(struct hist_entry *left, struct hist_entry *right)
> {
> /* Compare the addr that should be unique among comm */
> - return comm__str(right->comm) - comm__str(left->comm);
> + return strcmp(comm__str(right->comm), comm__str(left->comm));
> }
>
> static int64_t
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] perf tools: Fix "Command" sort_entry's cmp and collapse function
2015-05-18 1:38 ` Namhyung Kim
@ 2015-05-18 14:39 ` Jiri Olsa
0 siblings, 0 replies; 3+ messages in thread
From: Jiri Olsa @ 2015-05-18 14:39 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
Paul Mackerras, David Ahern, Ingo Molnar, Jan Stancek,
Frederic Weisbecker
On Mon, May 18, 2015 at 10:38:50AM +0900, Namhyung Kim wrote:
> Hi Jiri,
>
> CC-ing Frederic as he wrote the comm change.
>
> On Fri, May 15, 2015 at 05:54:28PM +0200, Jiri Olsa wrote:
> > Currently the se_cmp and se_collapse use pointer comparison,
> > which is ok for for testing equality of strings. It's not ok
> > as comparing function for rbtree insertion, because it gives
> > different results based on current pointer values.
> >
> > We saw test 32 (hists cumulation test) failing based on different
> > environment setup. Having all sort functions straightened fix the
> > test for us.
>
> Can you elaborate it?
>
> AFAIK comm string is shared among threads so pointer comparison and
> 'strcmp == 0' should have same result..
well pointers (A, B) substraction will get either negative (A < B) or
possitive number (A > B). Now if your environment happens to have those
pointers switched you'll get different result for same strings.
Following python script forced the 'perf test 32' to switch those pointers
and screwed the Command sorting.. it's not very reliable on upstream for
some reason.. but very reproducible on RHEL7
---
#!/bin/python
import os
env={}
env['PATH'] = "/tmp:"*500
os.execve("./perf", ["perf", "test", "32", "-v"], env)
---
jirka
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-05-18 14:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-15 15:54 [PATCH] perf tools: Fix "Command" sort_entry's cmp and collapse function Jiri Olsa
2015-05-18 1:38 ` Namhyung Kim
2015-05-18 14:39 ` Jiri Olsa
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.