Thomas Lockhart <lockhart@fourpalms.org> writes:
>> Declaring the lock pointer "volatile" seems to prevent this misbehavior.
> Great. That is what it is anyway, right?
The reason I hadn't declared it volatile in the first place was that I
was assuming there'd be sequence points at the spin lock and unlock
calls. The order of operations *while holding the lock* is, and should
be, optimizable. Pushing updates outside of the range where the lock is
held, however, isn't cool.
Now that I think a little more, I am worried about the idea that the
same thing could potentially happen in other modules that access shared
memory and use spinlocks to serialize the access. Perhaps the fix I
applied was wrong, and the correct fix is to change S_LOCK from
#define S_UNLOCK(lock) (*(lock) = 0)
to
#define S_UNLOCK(lock) (*((volatile slock_t *) (lock)) = 0)
Assuming that the compiler does faithfully treat that as a sequence
point, it would solve potential related problems in other modules, not
only LWLock. I note that we've carefully marked all the TAS operations
as using volatile pointers ... but we forgot about S_UNLOCK.
Comments?
regards, tom lane