Re: [patch] spinlocks: remove 'volatile'

[Linus Torvalds]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/