[Linux-ia64] Re: spin_unlock() problem

[Jack Steiner <steiner@sgi.com>]

> 
> Hi,
> 
> I have been tracing a problem with tty->count hitting an unidenfied
> state and I am starting to ponder if our current spin_unlock()
> implementation is sufficient.
> 
> Currently the spin_unlock() implementation looks like this:
> 
> #define spin_unlock(x)		do { barrier(); ((spinlock_t *) x)->lock = 0;} while (0)
> 
> barrier() doesn't guarantee memory ordering, in other words, we are not
> guaranteed that writes have been flushed to physical memory on exit. Now
> Jesse pointed out to me that spin_lock() uses aquire semantics which
> should take care of this, however this is only the case if the other CPU
> grabs a spin lock before reading the variable we wrote while holding the
> lock.
> 
> Consider the following example:
> 
> cpu1()
> {
>    spin_lock(&bleh);
>    *a = foo;
>    *b = bar;
>    spin_unlock(&bleh);
> }
> 
> cpu2()
> {
>    if (*b = bar)
>       boink(*a);
> }
> 
> With our weak memory ordering, b might have been written back to memory
> while a still hasn't made it out. Or am I missing something here?

I think IA64 is ok.

spinlock_t defines "lock" as volatile. On IA64, all references to volatile are required
to use loads/stores with acquire/release semantics. gcc ensures this.

The barrier() statement prevents compiler optimizations. The st.rel [lock]=0
clears the lock. The "st.rel" ensure that all previously issued stores have reached
visibilty in the coherency domain before the lock is cleared.


> 
> The question is, shouldn't our spin_unlock() implementation call wmb()
> instead of barrier()? I noticed Alpha calls mb() in their spin_unlock()
> implementation.
> 
> Cheers,
> Jes
> 


-- 
Thanks

Jack Steiner    (651-683-5302)   (vnet 233-5302)      steiner@sgi.com