2011-02-22 17:49:12

by Marcin Slusarz

[permalink] [raw]
Subject: [PATCH] perf lock: fix sorting by wait_min

if lock was uncontended, wait_time_min == ULLONG_MAX, so we need to handle
this case differently to show high wait times first

Signed-off-by: Marcin Slusarz <[email protected]>
---
tools/perf/builtin-lock.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 0305a40..812ad26 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -202,9 +202,20 @@ static struct thread_stat *thread_stat_findnew_first(u32 tid)
SINGLE_KEY(nr_acquired)
SINGLE_KEY(nr_contended)
SINGLE_KEY(wait_time_total)
-SINGLE_KEY(wait_time_min)
SINGLE_KEY(wait_time_max)

+static int lock_stat_key_wait_time_min(struct lock_stat *one,
+ struct lock_stat *two)
+{
+ u64 s1 = one->wait_time_min;
+ u64 s2 = two->wait_time_min;
+ if (s1 == ULLONG_MAX)
+ s1 = 0;
+ if (s2 == ULLONG_MAX)
+ s2 = 0;
+ return s1 > s2;
+}
+
struct lock_key {
/*
* name: the value for specify by user
--
1.7.4.rc3


2011-03-14 16:57:13

by Marcin Slusarz

[permalink] [raw]
Subject: Re: [PATCH] perf lock: fix sorting by wait_min

On Tue, Feb 22, 2011 at 06:47:15PM +0100, Marcin Slusarz wrote:
> if lock was uncontended, wait_time_min == ULLONG_MAX, so we need to handle
> this case differently to show high wait times first
>
> Signed-off-by: Marcin Slusarz <[email protected]>
> ---
> tools/perf/builtin-lock.c | 13 ++++++++++++-
> 1 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> index 0305a40..812ad26 100644
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c
> @@ -202,9 +202,20 @@ static struct thread_stat *thread_stat_findnew_first(u32 tid)
> SINGLE_KEY(nr_acquired)
> SINGLE_KEY(nr_contended)
> SINGLE_KEY(wait_time_total)
> -SINGLE_KEY(wait_time_min)
> SINGLE_KEY(wait_time_max)
>
> +static int lock_stat_key_wait_time_min(struct lock_stat *one,
> + struct lock_stat *two)
> +{
> + u64 s1 = one->wait_time_min;
> + u64 s2 = two->wait_time_min;
> + if (s1 == ULLONG_MAX)
> + s1 = 0;
> + if (s2 == ULLONG_MAX)
> + s2 = 0;
> + return s1 > s2;
> +}
> +
> struct lock_key {
> /*
> * name: the value for specify by user
> --

Anything wrong with this patch?
I can't find it in linux-next.

Marcin

2011-03-14 19:23:20

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf lock: fix sorting by wait_min

Em Mon, Mar 14, 2011 at 05:54:58PM +0100, Marcin Slusarz escreveu:
> On Tue, Feb 22, 2011 at 06:47:15PM +0100, Marcin Slusarz wrote:
> > if lock was uncontended, wait_time_min == ULLONG_MAX, so we need to handle
> > this case differently to show high wait times first
> >
> > Signed-off-by: Marcin Slusarz <[email protected]>
> > ---
> > tools/perf/builtin-lock.c | 13 ++++++++++++-
> > 1 files changed, 12 insertions(+), 1 deletions(-)
> >
> > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> > index 0305a40..812ad26 100644
> > --- a/tools/perf/builtin-lock.c
> > +++ b/tools/perf/builtin-lock.c
> > @@ -202,9 +202,20 @@ static struct thread_stat *thread_stat_findnew_first(u32 tid)
> > SINGLE_KEY(nr_acquired)
> > SINGLE_KEY(nr_contended)
> > SINGLE_KEY(wait_time_total)
> > -SINGLE_KEY(wait_time_min)
> > SINGLE_KEY(wait_time_max)
> >
> > +static int lock_stat_key_wait_time_min(struct lock_stat *one,
> > + struct lock_stat *two)
> > +{
> > + u64 s1 = one->wait_time_min;
> > + u64 s2 = two->wait_time_min;
> > + if (s1 == ULLONG_MAX)
> > + s1 = 0;
> > + if (s2 == ULLONG_MAX)
> > + s2 = 0;
> > + return s1 > s2;
> > +}
> > +
> > struct lock_key {
> > /*
> > * name: the value for specify by user
> > --
>
> Anything wrong with this patch?
> I can't find it in linux-next.

Hitoshi-san, can I have your acked-by for this one?

Thanks,

- Arnaldo

2011-03-15 08:09:00

by Hitoshi Mitake

[permalink] [raw]
Subject: Re: [PATCH] perf lock: fix sorting by wait_min

On 2011年03月15日 04:23, Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 14, 2011 at 05:54:58PM +0100, Marcin Slusarz escreveu:
>> On Tue, Feb 22, 2011 at 06:47:15PM +0100, Marcin Slusarz wrote:
>>> if lock was uncontended, wait_time_min == ULLONG_MAX, so we need to handle
>>> this case differently to show high wait times first
>>>
>>> Signed-off-by: Marcin Slusarz<[email protected]>
>>> ---
>>> tools/perf/builtin-lock.c | 13 ++++++++++++-
>>> 1 files changed, 12 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
>>> index 0305a40..812ad26 100644
>>> --- a/tools/perf/builtin-lock.c
>>> +++ b/tools/perf/builtin-lock.c
>>> @@ -202,9 +202,20 @@ static struct thread_stat *thread_stat_findnew_first(u32 tid)
>>> SINGLE_KEY(nr_acquired)
>>> SINGLE_KEY(nr_contended)
>>> SINGLE_KEY(wait_time_total)
>>> -SINGLE_KEY(wait_time_min)
>>> SINGLE_KEY(wait_time_max)
>>>
>>> +static int lock_stat_key_wait_time_min(struct lock_stat *one,
>>> + struct lock_stat *two)
>>> +{
>>> + u64 s1 = one->wait_time_min;
>>> + u64 s2 = two->wait_time_min;
>>> + if (s1 == ULLONG_MAX)
>>> + s1 = 0;
>>> + if (s2 == ULLONG_MAX)
>>> + s2 = 0;
>>> + return s1> s2;
>>> +}
>>> +
>>> struct lock_key {
>>> /*
>>> * name: the value for specify by user
>>> --
>>
>> Anything wrong with this patch?
>> I can't find it in linux-next.
>
> Hitoshi-san, can I have your acked-by for this one?
>
> Thanks,
>
> - Arnaldo
>

Hi Arnaldo and Marcin,

Sure, ignoring the case of wait_time_min == ULLONG_MAX is my mistake.
Thanks a lot!

2011-03-17 11:04:45

by Marcin Slusarz

[permalink] [raw]
Subject: [tip:perf/urgent] perf lock: Fix sorting by wait_min

Commit-ID: 9df03abeda3d928ecdedc0f427336931eac0a477
Gitweb: http://git.kernel.org/tip/9df03abeda3d928ecdedc0f427336931eac0a477
Author: Marcin Slusarz <[email protected]>
AuthorDate: Tue, 22 Feb 2011 18:47:15 +0100
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 16 Mar 2011 09:05:58 -0300

perf lock: Fix sorting by wait_min

If lock was uncontended, wait_time_min == ULLONG_MAX, so we need to
handle this case differently to show high wait times first

Acked-by: Hitoshi Mitake <[email protected]>
Cc: Hitoshi Mitake <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Marcin Slusarz <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-lock.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 2e93f99..7a2a79d 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -202,9 +202,20 @@ static struct thread_stat *thread_stat_findnew_first(u32 tid)
SINGLE_KEY(nr_acquired)
SINGLE_KEY(nr_contended)
SINGLE_KEY(wait_time_total)
-SINGLE_KEY(wait_time_min)
SINGLE_KEY(wait_time_max)

+static int lock_stat_key_wait_time_min(struct lock_stat *one,
+ struct lock_stat *two)
+{
+ u64 s1 = one->wait_time_min;
+ u64 s2 = two->wait_time_min;
+ if (s1 == ULLONG_MAX)
+ s1 = 0;
+ if (s2 == ULLONG_MAX)
+ s2 = 0;
+ return s1 > s2;
+}
+
struct lock_key {
/*
* name: the value for specify by user