Hi Carlos,
Carlos O'Donell wrote:
On Sat, Jun 14, 2008 at 11:36 AM, Helge Deller <deller@xxxxxx> wrote:
I'm wondering if this patch might help people who are seeing locking
problems on SMP boxes ?
Helge
diff --git a/include/asm-parisc/system.h b/include/asm-parisc/system.h
index ee80c92..4752684 100644
--- a/include/asm-parisc/system.h
+++ b/include/asm-parisc/system.h
@@ -168,8 +168,9 @@ static inline void set_eiem(unsigned long val)
/* LDCW, the only atomic read-write operation PA-RISC has. *sigh*. */
#define __ldcw(a) ({ \
unsigned __ret; \
- __asm__ __volatile__(__LDCW " 0(%1),%0" \
- : "=r" (__ret) : "r" (a)); \
+ __asm__ __volatile__(__LDCW " 0(%2),%0" \
+ : "=r" (__ret), "=m" (*(a)) \
+ : "r" (a), "m" (*(a)) ); \
__ret; \
})
You don't want to do that, the compiler might hold "=m" (*(a)) in a
temporary memory location.
e.g.
temp_mem = *a;
reg2 = &temp_mem;
... operation ...
*a = temp_mem;
You would be atomic with regards to the store to temp_mem, but not a.
Infact this could always be the case.
I'm now of the opinion that we need a "memory" clobber in the original
expression to prevent this from ever happening.
So, your proposal is (copy-and-pasted in here) the following ?
diff --git a/include/asm-parisc/system.h b/include/asm-parisc/system.h
index ee80c92..daeae39 100644
--- a/include/asm-parisc/system.h
+++ b/include/asm-parisc/system.h
@@ -169,7 +169,7 @@ static inline void set_eiem(unsigned long val)
#define __ldcw(a) ({ \
unsigned __ret; \
__asm__ __volatile__(__LDCW " 0(%1),%0" \
- : "=r" (__ret) : "r" (a)); \
+ : "=r" (__ret) : "r" (a) : "memory" ); \
__ret; \
})
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
