locking problems with mips atomicity ?

locking problems with mips atomicity ?

[Harm Verhagen]

Hi folks,

I noticed the following thread "locking problem with mips atomicity" on
the GCC mailing list, and I started to wonder if the linux kernel has
the same problem.

http://gcc.gnu.org/ml/gcc/2004-03/threads.html#01061


It about the problem that gcc can generate loads (using AT) in between
the ll and sc instructions (which is not legal according to the MIPS
spec) This can happen when  incorrect constraints are used with the
inline assembly, and the inline assembly happens to be in an inline
functions.

The code from linux 2.4.26 arch-mips/atomic.h looks _very_ similar to
the code described in the thread that has a BUG.

static __inline__ void atomic_add(int i, atomic_t * v)
{
	unsigned long temp;

	__asm__ __volatile__(
		"1:   ll      %0, %1      # atomic_add\n"
		"     addu    %0, %2                  \n"
		"     sc      %0, %1                  \n"
		"     beqz    %0, 1b                  \n"
		: "=&r" (temp), "=m" (v->counter)
		: "Ir" (i), "m" (v->counter));
}

So I wonder if there is a bug here. 
Can some MIPS guru check ? :)


Please copy me on replies as I'm not subscribed

Kind regards,
Harm Verhagen

Re: locking problems with mips atomicity ?

[Daniel Jacobowitz]

On Wed, Apr 21, 2004 at 12:44:34AM +0200, Harm Verhagen wrote:
> The code from linux 2.4.26 arch-mips/atomic.h looks _very_ similar to
> the code described in the thread that has a BUG.
> 
> static __inline__ void atomic_add(int i, atomic_t * v)
> {
> 	unsigned long temp;
> 
> 	__asm__ __volatile__(
> 		"1:   ll      %0, %1      # atomic_add\n"
> 		"     addu    %0, %2                  \n"
> 		"     sc      %0, %1                  \n"
> 		"     beqz    %0, 1b                  \n"
> 		: "=&r" (temp), "=m" (v->counter)
> 		: "Ir" (i), "m" (v->counter));
> }
> 
> So I wonder if there is a bug here. 
> Can some MIPS guru check ? :)

It won't be a problem in the kernel.  The problem only happens when the
assembler expands a macro to multiple instructions including a load,
and that only happens for position-independent code; the kernel is not
PIC.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: locking problems with mips atomicity ?

[Harm Verhagen]

On Wed, 2004-04-21 at 00:49, Daniel Jacobowitz wrote:
> On Wed, Apr 21, 2004 at 12:44:34AM +0200, Harm Verhagen wrote:
> > The code from linux 2.4.26 arch-mips/atomic.h looks _very_ similar to
> > the code described in the thread that has a BUG.
> > 
> > static __inline__ void atomic_add(int i, atomic_t * v)
> > {
> > 	unsigned long temp;
> > 
> > 	__asm__ __volatile__(
> > 		"1:   ll      %0, %1      # atomic_add\n"
> > 		"     addu    %0, %2                  \n"
> > 		"     sc      %0, %1                  \n"
> > 		"     beqz    %0, 1b                  \n"
> > 		: "=&r" (temp), "=m" (v->counter)
> > 		: "Ir" (i), "m" (v->counter));
> > }
> > 
> > So I wonder if there is a bug here. 
> > Can some MIPS guru check ? :)
> 
> It won't be a problem in the kernel.  The problem only happens when the
> assembler expands a macro to multiple instructions including a load,
> and that only happens for position-independent code; the kernel is not
> PIC.
Sorry for not understanding completely. What makes the gcc code PIC
then? The code looks similar, an inline function with inline assembly.
Could you elaborate ?

Kind regards,
Harm

Re: locking problems with mips atomicity ?

[Daniel Jacobowitz]

On Wed, Apr 21, 2004 at 12:53:56AM +0200, Harm Verhagen wrote:
> On Wed, 2004-04-21 at 00:49, Daniel Jacobowitz wrote:
> > On Wed, Apr 21, 2004 at 12:44:34AM +0200, Harm Verhagen wrote:
> > > The code from linux 2.4.26 arch-mips/atomic.h looks _very_ similar to
> > > the code described in the thread that has a BUG.
> > > 
> > > static __inline__ void atomic_add(int i, atomic_t * v)
> > > {
> > > 	unsigned long temp;
> > > 
> > > 	__asm__ __volatile__(
> > > 		"1:   ll      %0, %1      # atomic_add\n"
> > > 		"     addu    %0, %2                  \n"
> > > 		"     sc      %0, %1                  \n"
> > > 		"     beqz    %0, 1b                  \n"
> > > 		: "=&r" (temp), "=m" (v->counter)
> > > 		: "Ir" (i), "m" (v->counter));
> > > }
> > > 
> > > So I wonder if there is a bug here. 
> > > Can some MIPS guru check ? :)
> > 
> > It won't be a problem in the kernel.  The problem only happens when the
> > assembler expands a macro to multiple instructions including a load,
> > and that only happens for position-independent code; the kernel is not
> > PIC.
> Sorry for not understanding completely. What makes the gcc code PIC
> then? The code looks similar, an inline function with inline assembly.
> Could you elaborate ?

You may want to look for documentation describing the MIPS PIC
conventions.  Good luck; they're bizarre.

The problem is that in userspace "ll $2, symbol_name" may expand to a
load of the address of symbol_name from the GOT (global offset table).

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: locking problems with mips atomicity ?

[Maciej W. Rozycki]

On Tue, 20 Apr 2004, Daniel Jacobowitz wrote:

> > static __inline__ void atomic_add(int i, atomic_t * v)
> > {
> > 	unsigned long temp;
> > 
> > 	__asm__ __volatile__(
> > 		"1:   ll      %0, %1      # atomic_add\n"
> > 		"     addu    %0, %2                  \n"
> > 		"     sc      %0, %1                  \n"
> > 		"     beqz    %0, 1b                  \n"
> > 		: "=&r" (temp), "=m" (v->counter)
> > 		: "Ir" (i), "m" (v->counter));
> > }
> > 
> > So I wonder if there is a bug here. 
> > Can some MIPS guru check ? :)
> 
> It won't be a problem in the kernel.  The problem only happens when the
> assembler expands a macro to multiple instructions including a load,
> and that only happens for position-independent code; the kernel is not
> PIC.

 And if using a function like the above is to be PIC, the solution is to
use the "R" constraint for the ll/sc memory operand to assure it's passed
to gas as a machine-expressible address, although I'm not sure if that's
currently handled by gcc correctly (2.95.x definitely has problems with
it; I'm going to look into it for 3.5.0 soon).

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +