locking problem with mips atomicity, etc.
Michael Eager
eager@mvista.com
Mon Mar 15 23:58:00 GMT 2004
The MIPS architecture spec says that there should not be a load,
store or prefetch between issuing a LL instruction and a following SC
instruction. (MIPS32 Arch, Vol 2, rev 0.95, SC instruction description, p 186)
The macros in sysdeps/mips/atomicity.h and elsewhere (two other places
in glibc and one in gcc) can generate a load between the LL and SC.
It appears that on most MIPS processors this either works as expected,
or at worst, only fails on the first time through the load is executed.
On at least one processor, the NEC vr4133, the SC fails constently.
Here's a little piece of code, extracted from mcount.c, which shows the
problem:
mcount.c:
#include "atomicity.h"
struct gmonparam {
long int state;
};
extern struct gmonparam _gmonparam ;
static void __attribute__ ((__used__)) __mcount ()
{
register struct gmonparam *p;
p = &_gmonparam;
if (! compare_and_swap (&p->state, 0, 1))
return;
p->state = 0;
}
The code generated for compare_and_swap is (lwc0 == LL, swc0 == SC):
14: 8f850000 lw a1,0(gp)
18: 00000000 nop
1c: c0a50000 lwc0 $5,0(a1)
20: 14a30007 bne a1,v1,40 <__mcount+0x40>
24: 00002021 move a0,zero
28: 00402021 move a0,v0
2c: 8f810000 lw at,0(gp) <<<<<< BAD
30: 00000000 nop
34: e0240000 swc0 $4,0(at)
38: 1080fff6 beqz a0,14 <__mcount+0x14>
3c: 00000000 nop
40: 10800004 beqz a0,54 <__mcount+0x54>
The attached patch loads the address of the location being
modified into a reg before the LL, and references it explicitly,
so that the following is generated instead:
14: 8f850000 lw a1,0(gp)
18: c0a60000 lwc0 $6,0(a1)
1c: 14c30005 bne a2,v1,34 <__mcount+0x34>
20: 00002021 move a0,zero
24: 00402021 move a0,v0
28: e0a40000 swc0 $4,0(a1)
2c: 1080fff9 beqz a0,14 <__mcount+0x14>
30: 00000000 nop
34: 10800004 beqz a0,48 <__mcount+0x48>
--
Michael Eager eager@mvista.com 408-328-8426
MontaVista Software, Inc. 1237 E. Arques Ave., Sunnyvale, CA 94085
-------------- next part --------------
2004-03-15 Michael Eager <eager@mvista.com>
* linuxthreads/sysdeps/mips/pt-machine.h: move load before LL,
instead of btw LL and SC instr.
* sysdeps/mips/atomicity.h: same
* sysdeps/unix/sysv/linux/mips/sys/tas.h: same
Index: linuxthreads/sysdeps/mips/pt-machine.h
===================================================================
RCS file: /cvs/glibc/libc/linuxthreads/sysdeps/mips/pt-machine.h,v
retrieving revision 1.16
diff -u -r1.16 pt-machine.h
--- linuxthreads/sysdeps/mips/pt-machine.h 31 Jul 2003 19:15:43 -0000 1.16
+++ linuxthreads/sysdeps/mips/pt-machine.h 15 Mar 2004 22:06:17 -0000
@@ -55,7 +55,7 @@
PT_EI int
__compare_and_swap (long int *p, long int oldval, long int newval)
{
- long int ret, temp;
+ long int ret, temp, temp2;
__asm__ __volatile__
("/* Inline compare & swap */\n"
@@ -64,24 +64,25 @@
#if _MIPS_SIM == _MIPS_SIM_ABI32
".set mips2\n\t"
#endif
+ "la %3,%5\n\t"
#if defined _ABI64 && _MIPS_SIM == _ABI64
- "lld %1,%5\n\t"
+ "lld %1,0(%3)\n\t"
#else
- "ll %1,%5\n\t"
+ "ll %1,0(%3)\n\t"
#endif
"move %0,$0\n\t"
"bne %1,%3,2f\n\t"
- "move %0,%4\n\t"
+ "move %0,%5\n\t"
#if defined _ABI64 && _MIPS_SIM == _ABI64
- "scd %0,%2\n\t"
+ "scd %0,0(%3)\n\t"
#else
- "sc %0,%2\n\t"
+ "sc %0,0(%3)\n\t"
#endif
".set pop\n\t"
"beqz %0,1b\n"
"2:\n\t"
"/* End compare & swap */"
- : "=&r" (ret), "=&r" (temp), "=m" (*p)
+ : "=&r" (ret), "=&r" (temp), "=m" (*p), "=r" (temp2)
: "r" (oldval), "r" (newval), "m" (*p)
: "memory");
Index: sysdeps/mips/atomicity.h
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/mips/atomicity.h,v
retrieving revision 1.7
diff -u -r1.7 atomicity.h
--- sysdeps/mips/atomicity.h 14 Mar 2003 05:30:31 -0000 1.7
+++ sysdeps/mips/atomicity.h 15 Mar 2004 22:06:18 -0000
@@ -26,7 +26,7 @@
__attribute__ ((unused))
exchange_and_add (volatile uint32_t *mem, int val)
{
- int result, tmp;
+ int result, tmp, temp;
__asm__ __volatile__
("/* Inline exchange & add */\n"
@@ -35,13 +35,14 @@
#if _MIPS_SIM == _MIPS_SIM_ABI32
".set mips2\n\t"
#endif
- "ll %0,%3\n\t"
- "addu %1,%4,%0\n\t"
- "sc %1,%2\n\t"
+ "la %3,%2\n\t"
+ "ll %0,0(%3)\n\t"
+ "addu %1,%5,%0\n\t"
+ "sc %1,0(%3)\n\t"
".set pop\n\t"
"beqz %1,1b\n\t"
"/* End exchange & add */"
- : "=&r"(result), "=&r"(tmp), "=m"(*mem)
+ : "=&r"(result), "=&r"(tmp), "=m"(*mem), "=r" (temp)
: "m" (*mem), "r"(val)
: "memory");
@@ -52,7 +53,7 @@
__attribute__ ((unused))
atomic_add (volatile uint32_t *mem, int val)
{
- int result;
+ int result, temp;
__asm__ __volatile__
("/* Inline atomic add */\n"
@@ -61,13 +62,14 @@
#if _MIPS_SIM == _MIPS_SIM_ABI32
".set mips2\n\t"
#endif
- "ll %0,%2\n\t"
- "addu %0,%3,%0\n\t"
- "sc %0,%1\n\t"
+ "la %2,%1\n\t"
+ "ll %0,0(%2)\n\t"
+ "addu %0,%4,%0\n\t"
+ "sc %0,0(%2)\n\t"
".set pop\n\t"
"beqz %0,1b\n\t"
"/* End atomic add */"
- : "=&r"(result), "=m"(*mem)
+ : "=&r"(result), "=m"(*mem), "=r" (temp)
: "m" (*mem), "r"(val)
: "memory");
}
@@ -76,7 +78,7 @@
__attribute__ ((unused))
compare_and_swap (volatile long int *p, long int oldval, long int newval)
{
- long int ret, temp;
+ long int ret, temp, temp2;
__asm__ __volatile__
("/* Inline compare & swap */\n"
@@ -85,24 +87,25 @@
#if _MIPS_SIM == _MIPS_SIM_ABI32
".set mips2\n\t"
#endif
+ "la %3,%2\n\t"
#if defined _ABI64 && _MIPS_SIM == _ABI64
- "lld %1,%5\n\t"
+ "lld %1,0(%3)\n\t"
#else
- "ll %1,%5\n\t"
+ "ll %1,0(%3)\n\t"
#endif
"move %0,$0\n\t"
- "bne %1,%3,2f\n\t"
- "move %0,%4\n\t"
+ "bne %1,%4,2f\n\t"
+ "move %0,%5\n\t"
#if defined _ABI64 && _MIPS_SIM == _ABI64
- "scd %0,%2\n\t"
+ "scd %0,0(%3)\n\t"
#else
- "sc %0,%2\n\t"
+ "sc %0,0(%3)\n\t"
#endif
".set pop\n\t"
"beqz %0,1b\n"
"2:\n\t"
"/* End compare & swap */"
- : "=&r" (ret), "=&r" (temp), "=m" (*p)
+ : "=&r" (ret), "=&r" (temp), "=m" (*p), "=r" (temp2)
: "r" (oldval), "r" (newval), "m" (*p)
: "memory");
Index: sysdeps/unix/sysv/linux/mips/sys/tas.h
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/unix/sysv/linux/mips/sys/tas.h,v
retrieving revision 1.10
diff -u -r1.10 tas.h
--- sysdeps/unix/sysv/linux/mips/sys/tas.h 14 Mar 2003 07:03:36 -0000 1.10
+++ sysdeps/unix/sysv/linux/mips/sys/tas.h 15 Mar 2004 22:06:18 -0000
@@ -35,7 +35,7 @@
_EXTERN_INLINE int
_test_and_set (int *p, int v) __THROW
{
- int r, t;
+ int r, t, temp;
__asm__ __volatile__
("/* Inline test and set */\n"
@@ -44,15 +44,16 @@
#if _MIPS_SIM == _MIPS_SIM_ABI32
".set mips2\n\t"
#endif
- "ll %0,%3\n\t"
- "move %1,%4\n\t"
- "beq %0,%4,2f\n\t"
- "sc %1,%2\n\t"
+ "la %3,%2\n\t"
+ "ll %0,0(%3)\n\t"
+ "move %1,%5\n\t"
+ "beq %0,%5,2f\n\t"
+ "sc %1,0(%3)\n\t"
".set pop\n\t"
"beqz %1,1b\n"
"2:\n\t"
"/* End test and set */"
- : "=&r" (r), "=&r" (t), "=m" (*p)
+ : "=&r" (r), "=&r" (t), "=m" (*p), "=r" (temp)
: "m" (*p), "r" (v)
: "memory");
More information about the Libc-alpha
mailing list