Re: [patch] spinlocks: remove 'volatile'

J.A. Magallón
2006-07-07T22:05:43Z

On Fri, 7 Jul 2006 17:22:31 -0400, "linux-os \(Dick Johnson\)" <linux-os@analogic.com> wrote:

>
> On Fri, 7 Jul 2006, Linus Torvalds wrote:
>
> >
> >
> > On Fri, 7 Jul 2006, linux-os (Dick Johnson) wrote:
> >>
> >> Now Linus declares that instead of declaring an object volatile
> >> so that it is actually accessed every time it is referenced, he wants
> >> to use a GNU-ism with assembly that tells the compiler to re-read
> >> __every__ variable existing im memory, instead of just one. Go figure!
> >
> > Actually, it's not just me.
> >
> > Read things like the Intel CPU documentation.
> >
> > IT IS ACTIVELY WRONG to busy-loop on a variable. It will make the CPU
> > potentially over-heat, causing degreaded performance, and you're simply
> > not supposed to do it.
>
> This is a bait and switch argument. The code was displayed to show
> the compiler output, not an example of good coding practice.
>

volatile means what it means, is usefull and is right. If it is used
in kernel for other things apart from what it was designed for it is
kernel or programmer responsibility. It does not mention nothing about
locking.

A more real example:

#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;
}
}

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).

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

So think about all you inlined spinlocks, mutexes and so on.

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

So volatile just means 'dont trust this does not change even you don't see
why'.

--
J.A. Magallon <jamagallon()ono!com> \ Software is like sex:
\ It's better when it's free
Mandriva Linux release 2007.0 (Cooker) for i586
Linux 2.6.17-jam01 (gcc 4.1.1 20060518 (prerelease)) #2 SMP PREEMPT Wed