On Sat, 8 Jul 2006, J.A. Magallón wrote:
#include <stdint.h>
//volatile
uint32_t spinvar = 1;
uint32_t mtx;
void lock(uint32_t* l)
{
*l = 1;
}
void unlock(uint32_t* l)
{
*l = 0;
}
void spin()
{
uint32_t local;
for (;;)
{
lock(&mtx);
local = spinvar;
unlock(&mtx);
if (!local)
break;
}
}
This is _totally_ incorrect. Your "lock" functions are broken, because
they do not introduce syncronization points or locked bus operations. Due
to this huge failure, the compiler and/or processor is free to re-order
your loads and stores, resulting in totally unpredictable runtime
behavior.
without the volatile:
spin:
pushl %ebp
movl spinvar, %eax
movl %esp, %ebp
testl %eax, %eax
je .L7
.L10:
jmp .L10
.L7:
movl $0, mtx
popl %ebp
ret
so the compiler did something like
local = spinvar;
if (local)
for (;;);
(notice the dead lock/unlock inlined code elimination).
...which indicates that your code is wrong.
With the volatile, the code is correct:
spin:
pushl %ebp
movl %esp, %ebp
.p2align 4,,7
.L7:
movl spinvar, %eax
testl %eax, %eax
jne .L7
movl $0, mtx
popl %ebp
ret
Actually, it's not. It's never setting "mtx" to 1, and it's certainly not
doing any sync or locked ops.
So think about all you inlined spinlocks, mutexes and so on.
Yes, you got it wrong, and the current code gets it right. (Linus's patch
of =m to +m, combined with -volatile, is best)
And if you do
void lock(volatile uint32_t* l)
...
void unlock(volatile uint32_t* l)
...
the code is even better:
spin:
pushl %ebp
movl %esp, %ebp
.p2align 4,,7
.L7:
movl $1, mtx <=========
movl spinvar, %eax
movl $0, mtx <=========
testl %eax, %eax
jne .L7
popl %ebp
ret
NO! It's not better. You're still not syncing or locking the bus! If you
refer to the fact that the "movl $1" has magically appeared, that's
because you've just PAPERED OVER THE PROBLEM WITH "volatile", which is
_exactly_ what Linus is telling you NOT TO DO.
So volatile just means 'dont trust this does not change even you don't see
why'.
No.
Thanks,
Chase
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
