Re: [patch] spinlocks: remove 'volatile'

Linus Torvalds
2006-07-07T17:10:04Z

On Fri, 7 Jul 2006, Chuck Ebbert wrote:
>
> > #define __raw_spin_unlock_string \
> > "movb $1,%0" \
> > - :"=m" (lock->slock) : : "memory"
> > + :"+m" (lock->slock) : : "memory"
>
> This really is just an overwrite of whatever is there. OTOH I can't see
> how this change could hurt anything..

Yeah, I don't think any non-buggy sequence can make it matter.

In theory, you could have something like

- create a spinlock already locked
- add the object that contains the spinlock to some global queues
- do some op on the object to finalize it
- unlock the spinlock

and then the "unlock" had better not optimize away the previous
initialization.

HOWEVER, it can't really do that anyway, since anything that made the
spinlock visible to anybody else would have had to serialize the lock
state for _that_, so if the "+m" vs "=m" makes a difference, you'd have
had a serious bug already for other reasons.

I'll leave it with a "+m". I've tested it locally, and looking at the
patch it definitely fixes real bugs in the inline asms, but I still hope
other people will take a look before I commit it, in case there are any
other subtle cases.

(A "+m" should always be safer than a "=m", so the patch should be very
safe, but hey, bugs happen to "obvious" code too)

Linus