2009-03-12 22:11:32

by Darren Hart

Subject: [PATCH] futex: remove the pointer math from double_unlock_hb

I mistakenly included the pointer value ordering in the double_unlock_hb
in my previous patch. It's only necessary in the double_lock_hb
function. This patch removes it.

Signed-off-by: Darren Hart <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Rusty Russell <[email protected]>
---

kernel/futex.c | 10 ++--------
1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 9c97f67..2331b73 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -658,14 +658,8 @@ double_lock_hb(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
static inline void
double_unlock_hb(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
{
- if (hb1 <= hb2) {
- spin_unlock(&hb2->lock);
- if (hb1 < hb2)
- spin_unlock(&hb1->lock);
- } else { /* hb1 > hb2 */
- spin_unlock(&hb1->lock);
- spin_unlock(&hb2->lock);
- }
+ spin_unlock(&hb1->lock);
+ spin_unlock(&hb2->lock);
}

/*

2009-03-13 00:20:19

by Darren Hart

Subject: [tip:core/futexes] futex: remove the pointer math from double_unlock_hb

Commit-ID: f061d35150003b7fd5b133d14d66a74500fdaa60
Gitweb: http://git.kernel.org/tip/f061d35150003b7fd5b133d14d66a74500fdaa60
Author: Darren Hart <[email protected]>
AuthorDate: Thu, 12 Mar 2009 15:11:18 -0700
Commit: Ingo Molnar <[email protected]>
CommitDate: Fri, 13 Mar 2009 01:15:46 +0100

futex: remove the pointer math from double_unlock_hb

Impact: simplify code

I mistakenly included the pointer value ordering in the
double_unlock_hb() in my previous patch. It's only necessary
in the double_lock_hb() function. This patch removes it.

Signed-off-by: Darren Hart <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Cc: Rusty Russell <[email protected]>
LKML-Reference: <20090312221118.11146.68610.stgit@Aeon>
Signed-off-by: Ingo Molnar <[email protected]>


---
kernel/futex.c | 10 ++--------
1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 9c97f67..2331b73 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -658,14 +658,8 @@ double_lock_hb(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
static inline void
double_unlock_hb(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
{
- if (hb1 <= hb2) {
- spin_unlock(&hb2->lock);
- if (hb1 < hb2)
- spin_unlock(&hb1->lock);
- } else { /* hb1 > hb2 */
- spin_unlock(&hb1->lock);
- spin_unlock(&hb2->lock);
- }
+ spin_unlock(&hb1->lock);
+ spin_unlock(&hb2->lock);
}

/*

2009-03-13 09:27:01

by Thomas Gleixner

Subject: Re: [PATCH] futex: remove the pointer math from double_unlock_hb

On Thu, 12 Mar 2009, Darren Hart wrote:
> I mistakenly included the pointer value ordering in the double_unlock_hb
> in my previous patch. It's only necessary in the double_lock_hb
> function. This patch removes it.
>
> Signed-off-by: Darren Hart <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Rusty Russell <[email protected]>
> ---
>
> kernel/futex.c | 10 ++--------
> 1 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/futex.c b/kernel/futex.c
> index 9c97f67..2331b73 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -658,14 +658,8 @@ double_lock_hb(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
> static inline void
> double_unlock_hb(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
> {
> - if (hb1 <= hb2) {
> - spin_unlock(&hb2->lock);
> - if (hb1 < hb2)
> - spin_unlock(&hb1->lock);
> - } else { /* hb1 > hb2 */
> - spin_unlock(&hb1->lock);
> - spin_unlock(&hb2->lock);
> - }
> + spin_unlock(&hb1->lock);
> + spin_unlock(&hb2->lock);

This is missing the check for hb1 == hb2. As I said before:

> Can we just put the code into double_unlock_hb() which gets replaced ?
>
> i.e:
>
> spin_unlock(&hb1->lock);
> if (hb1 != hb2)
> spin_unlock(&hb2->lock);

Thanks,

tglx

2009-03-13 09:37:29

by Ingo Molnar

Subject: [tip:core/futexes] futex: remove the pointer math from double_unlock_hb, fix

Commit-ID: 88f502fedba82eff252b6420e8b8328e4ae25c67
Gitweb: http://git.kernel.org/tip/88f502fedba82eff252b6420e8b8328e4ae25c67
Author: Ingo Molnar <[email protected]>
AuthorDate: Fri, 13 Mar 2009 10:32:07 +0100
Commit: Ingo Molnar <[email protected]>
CommitDate: Fri, 13 Mar 2009 10:32:07 +0100

futex: remove the pointer math from double_unlock_hb, fix

Impact: fix double unlock crash

Thomas Gleixner noticed that the simplified double_unlock_hb()
became ... too unsophisticated: in the hb1 == hb2 case it will
do a double unlock.

Reported-by: Thomas Gleixner <[email protected]>
Cc: Darren Hart <[email protected]>
LKML-Reference: <20090312221118.11146.68610.stgit@Aeon>
Signed-off-by: Ingo Molnar <[email protected]>


---
kernel/futex.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 2331b73..6b50a02 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -659,7 +659,8 @@ static inline void
double_unlock_hb(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
{
spin_unlock(&hb1->lock);
- spin_unlock(&hb2->lock);
+ if (hb1 != hb2)
+ spin_unlock(&hb2->lock);
}

/*

2009-03-13 15:04:19

by Darren Hart

Subject: Re: [tip:core/futexes] futex: remove the pointer math from double_unlock_hb, fix

Ingo Molnar wrote:
> Commit-ID: 88f502fedba82eff252b6420e8b8328e4ae25c67
> Gitweb: http://git.kernel.org/tip/88f502fedba82eff252b6420e8b8328e4ae25c67
> Author: Ingo Molnar <[email protected]>
> AuthorDate: Fri, 13 Mar 2009 10:32:07 +0100
> Commit: Ingo Molnar <[email protected]>
> CommitDate: Fri, 13 Mar 2009 10:32:07 +0100
>
> futex: remove the pointer math from double_unlock_hb, fix
>
> Impact: fix double unlock crash
>
> Thomas Gleixner noticed that the simplified double_unlock_hb()
> became ... too unsophisticated: in the hb1 == hb2 case it will
> do a double unlock.

Gah. Thanks for catching it.

>
> Reported-by: Thomas Gleixner <[email protected]>
> Cc: Darren Hart <[email protected]>
> LKML-Reference: <20090312221118.11146.68610.stgit@Aeon>
> Signed-off-by: Ingo Molnar <[email protected]>

Acked-by: Darren Hart <[email protected]>

>
>
> ---
> kernel/futex.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/futex.c b/kernel/futex.c
> index 2331b73..6b50a02 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -659,7 +659,8 @@ static inline void
> double_unlock_hb(struct futex_hash_bucket *hb1, struct futex_hash_bucket *hb2)
> {
> spin_unlock(&hb1->lock);
> - spin_unlock(&hb2->lock);
> + if (hb1 != hb2)
> + spin_unlock(&hb2->lock);
> }
>
> /*


--
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team