2019-11-04 23:28:24

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH] perf tools: Fix time sorting

The final sort might get confused when the comparison
is done over bigger numbers than int like for -s time.

Check following report for longer workloads:
$ perf report -s time -F time,overhead --stdio

Fixing hist_entry__sort to properly return int64_t and
not possible cut int.

Cc: Andi Kleen <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/hist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 679a1d75090c..7b6eaf5e0bda 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1625,7 +1625,7 @@ int hists__collapse_resort(struct hists *hists, struct ui_progress *prog)
return 0;
}

-static int hist_entry__sort(struct hist_entry *a, struct hist_entry *b)
+static int64_t hist_entry__sort(struct hist_entry *a, struct hist_entry *b)
{
struct hists *hists = a->hists;
struct perf_hpp_fmt *fmt;
--
2.21.0


2019-11-05 00:50:46

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Fix time sorting

On Tue, Nov 05, 2019 at 12:27:11AM +0100, Jiri Olsa wrote:
> The final sort might get confused when the comparison
> is done over bigger numbers than int like for -s time.
>
> Check following report for longer workloads:
> $ perf report -s time -F time,overhead --stdio
>
> Fixing hist_entry__sort to properly return int64_t and
> not possible cut int.
>
> Cc: Andi Kleen <[email protected]>
> Link: http://lkml.kernel.org/n/[email protected]
> Signed-off-by: Jiri Olsa <[email protected]>

Looks good.

Reviewed-by: Andi Kleen <[email protected]>

-Andi

2019-11-05 11:51:32

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Fix time sorting

Em Mon, Nov 04, 2019 at 04:48:54PM -0800, Andi Kleen escreveu:
> On Tue, Nov 05, 2019 at 12:27:11AM +0100, Jiri Olsa wrote:
> > The final sort might get confused when the comparison
> > is done over bigger numbers than int like for -s time.
> >
> > Check following report for longer workloads:
> > $ perf report -s time -F time,overhead --stdio
> >
> > Fixing hist_entry__sort to properly return int64_t and
> > not possible cut int.
> >
> > Cc: Andi Kleen <[email protected]>
> > Link: http://lkml.kernel.org/n/[email protected]
> > Signed-off-by: Jiri Olsa <[email protected]>
>
> Looks good.
>
> Reviewed-by: Andi Kleen <[email protected]>

Thanks, applied after adding:

Fixes: 043ca389a318 ("perf tools: Use hpp formats to sort final output")
Cc: [email protected] # v3.16+

- Arnaldo

2019-11-05 12:43:23

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Fix time sorting

On Tue, Nov 05, 2019 at 08:49:41AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 04, 2019 at 04:48:54PM -0800, Andi Kleen escreveu:
> > On Tue, Nov 05, 2019 at 12:27:11AM +0100, Jiri Olsa wrote:
> > > The final sort might get confused when the comparison
> > > is done over bigger numbers than int like for -s time.
> > >
> > > Check following report for longer workloads:
> > > $ perf report -s time -F time,overhead --stdio
> > >
> > > Fixing hist_entry__sort to properly return int64_t and
> > > not possible cut int.
> > >
> > > Cc: Andi Kleen <[email protected]>
> > > Link: http://lkml.kernel.org/n/[email protected]
> > > Signed-off-by: Jiri Olsa <[email protected]>
> >
> > Looks good.
> >
> > Reviewed-by: Andi Kleen <[email protected]>
>
> Thanks, applied after adding:
>
> Fixes: 043ca389a318 ("perf tools: Use hpp formats to sort final output")
> Cc: [email protected] # v3.16+

I was wondering about putting some commit there,
because it was there for long time.. but that
commit you use seems to be old enough.. ;-)

thanks,
jirka

2019-11-05 13:26:20

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf tools: Fix time sorting

Em Tue, Nov 05, 2019 at 01:41:50PM +0100, Jiri Olsa escreveu:
> On Tue, Nov 05, 2019 at 08:49:41AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Nov 04, 2019 at 04:48:54PM -0800, Andi Kleen escreveu:
> > > On Tue, Nov 05, 2019 at 12:27:11AM +0100, Jiri Olsa wrote:
> > > > The final sort might get confused when the comparison
> > > > is done over bigger numbers than int like for -s time.
> > > >
> > > > Check following report for longer workloads:
> > > > $ perf report -s time -F time,overhead --stdio
> > > >
> > > > Fixing hist_entry__sort to properly return int64_t and
> > > > not possible cut int.
> > > >
> > > > Cc: Andi Kleen <[email protected]>
> > > > Link: http://lkml.kernel.org/n/[email protected]
> > > > Signed-off-by: Jiri Olsa <[email protected]>
> > >
> > > Looks good.
> > >
> > > Reviewed-by: Andi Kleen <[email protected]>
> >
> > Thanks, applied after adding:
> >
> > Fixes: 043ca389a318 ("perf tools: Use hpp formats to sort final output")
> > Cc: [email protected] # v3.16+
>
> I was wondering about putting some commit there,
> because it was there for long time.. but that
> commit you use seems to be old enough.. ;-)

Yeah, I think it predates that, but the one I picked should have fixed
that problem, as it was touching these routines, so I thought would be a
good one to stick in a fixes tag :-)

- Arnaldo

> thanks,
> jirka

--

- Arnaldo

Subject: [tip: perf/urgent] perf tools: Fix time sorting

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID: 722ddfde366fd46205456a9c5ff9b3359dc9a75e
Gitweb: https://git.kernel.org/tip/722ddfde366fd46205456a9c5ff9b3359dc9a75e
Author: Jiri Olsa <[email protected]>
AuthorDate: Tue, 05 Nov 2019 00:27:11 +01:00
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitterDate: Tue, 05 Nov 2019 08:49:14 -03:00

perf tools: Fix time sorting

The final sort might get confused when the comparison is done over
bigger numbers than int like for -s time.

Check the following report for longer workloads:

$ perf report -s time -F time,overhead --stdio

Fix hist_entry__sort() to properly return int64_t and not possible cut
int.

Fixes: 043ca389a318 ("perf tools: Use hpp formats to sort final output")
Signed-off-by: Jiri Olsa <[email protected]>
Reviewed-by: Andi Kleen <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Michael Petlan <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: [email protected] # v3.16+
Link: http://lore.kernel.org/lkml/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/hist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 679a1d7..7b6eaf5 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1625,7 +1625,7 @@ int hists__collapse_resort(struct hists *hists, struct ui_progress *prog)
return 0;
}

-static int hist_entry__sort(struct hist_entry *a, struct hist_entry *b)
+static int64_t hist_entry__sort(struct hist_entry *a, struct hist_entry *b)
{
struct hists *hists = a->hists;
struct perf_hpp_fmt *fmt;