[bug,report] dead lock (?) occur on ap325 board

Commit Message

Takashi Yoshii Jan. 28, 2009, 9:29 a.m. UTC

Kunihiro's fix looks good for me.
I have accidentally(?) fixed this one just same way as him.

I don't know why the original code does 
> 	__res |= !__ex_flag;
to combine the results (counter and exception flag).
Logical-OR to scaler value is logically:) wrong.
# And using movt explicitly results stupid code generated anyway.

Because this __ex_flag is from "t" of movco, which means
 0: interrupted by int or exp.
 1: done successfully.
, it works as "Low-active" signal, and is not easy to handle
as a C-language conditon.

So, I think 
        int __done, __res;
        ....
        if (unlikely(!done || __res != 0))

is easier to read.
This fixes the deadlock issue Kunihiro reported (by coincident:).

# I'm not sure if this is suitable to be marked as "earlyclobber",
# but I leave it as-is, since nothing bad will occur.
/yoshii

Fix conditon checking expression of __mutex_fastpath_*

Signed-off-by: Takashi YOSHII <[e-mail]>
---
 arch/sh/include/asm/mutex-llsc.h |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

-- 1.6.0.6 
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Paul Mundt Jan. 28, 2009, 10:03 a.m. UTC | #1

On Wed, Jan 28, 2009 at 06:29:13PM +0900, Takashi Yoshii wrote:
> Fix conditon checking expression of __mutex_fastpath_*
> 
> Signed-off-by: Takashi YOSHII <[e-mail]>

Yes, that looks fine. I'll queue this up, thanks.

If Morimoto-san wants to add his Signed-off-by or Acked-by, I'll add that
also.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[e-mail] Jan. 29, 2009, 12:34 a.m. UTC | #2

Morimoto-san, I'm sorry for making mistake for your name.

> Kunihiro's fix looks good for me.
Who I meant was you.
The core part of the patch is just the same as you have posted.
Could your please give your signe-off (or ack) as a reply to my patch?
/yoshii
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Kuninori Morimoto Jan. 29, 2009, 11:54 p.m. UTC | #3

Dear Paul and yoshii-san.

Thank you yoshii-san for your response.

> > Signed-off-by: Takashi YOSHII <[e-mail]>
> 
> Yes, that looks fine. I'll queue this up, thanks.
> 
> If Morimoto-san wants to add his Signed-off-by or Acked-by, I'll add that
> also.

Please add.

>> Kunihiro's fix looks good for me.
> Who I meant was you.

Yes. My name is Kuninori
v(^o^)v         ~~~~~~~~

Best regards
--
Kuninori Morimoto
 
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/sh/include/asm/mutex-llsc.h b/arch/sh/include/asm/mutex-llsc.h
index ee839ee..66f045f 100644
--- a/arch/sh/include/asm/mutex-llsc.h
+++ b/arch/sh/include/asm/mutex-llsc.h
@@ -21,38 +21,36 @@ 
 static inline void
 __mutex_fastpath_lock(atomic_t *count, void (*fail_fn)(atomic_t *))
 {
-	int __ex_flag, __res;
+	int __done, __res;
 
 	__asm__ __volatile__ (
 		"movli.l	@%2, %0	\n"
 		"add		#-1, %0	\n"
 		"movco.l	%0, @%2	\n"
 		"movt		%1	\n"
-		: "=&z" (__res), "=&r" (__ex_flag)
+		: "=&z" (__res), "=&r" (__done)
 		: "r" (&(count)->counter)
 		: "t");
 
-	__res |= !__ex_flag;
-	if (unlikely(__res != 0))
+	if (unlikely(!__done || __res != 0))
 		fail_fn(count);
 }
 
 static inline int
 __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
 {
-	int __ex_flag, __res;
+	int __done, __res;
 
 	__asm__ __volatile__ (
 		"movli.l	@%2, %0	\n"
 		"add		#-1, %0	\n"
 		"movco.l	%0, @%2	\n"
 		"movt		%1	\n"
-		: "=&z" (__res), "=&r" (__ex_flag)
+		: "=&z" (__res), "=&r" (__done)
 		: "r" (&(count)->counter)
 		: "t");
 
-	__res |= !__ex_flag;
-	if (unlikely(__res != 0))
+	if (unlikely(!__done || __res != 0))
 		__res = fail_fn(count);
 
 	return __res;
@@ -61,19 +59,18 @@  __mutex_fastpath_lock_retval(atomic_t *count, int (*fail_fn)(atomic_t *))
 static inline void
 __mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
 {
-	int __ex_flag, __res;
+	int __done, __res;
 
 	__asm__ __volatile__ (
 		"movli.l	@%2, %0	\n\t"
 		"add		#1, %0	\n\t"
 		"movco.l	%0, @%2 \n\t"
 		"movt		%1	\n\t"
-		: "=&z" (__res), "=&r" (__ex_flag)
+		: "=&z" (__res), "=&r" (__done)
 		: "r" (&(count)->counter)
 		: "t");
 
-	__res |= !__ex_flag;
-	if (unlikely(__res <= 0))
+	if (unlikely(!__done || __res <= 0))
 		fail_fn(count);
 }