This is the mail archive of the libc-alpha@sources.redhat.com mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

locking problem with mips atomicity, etc.


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
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");
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]