Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom, I assume what you are saying is that the access to the spinlocks,
> already marked as volatile, should have prevented any code from
> migrating over those locks. I guess my big question is does any
> volatile access prevent optimization of other variables across that
> volatiles access? I didn't think that was guaranteed.
After eyeballing the C spec some more, I think you might be right.
If that's the correct reading then it is indeed necessary for lwlock.c
to mark the whole lock structure as volatile, not only the spinlock
fields.
However, if that's true then (a) 7.2 has three other modules that are
potentially vulnerable to similar problems; (b) prior releases had
many more places that were potentially vulnerable --- ie, all the
modules that used to use spinlocks directly and now use LWLocks.
If this sort of behavior is allowed, ISTM we should have been seeing
major instability on lots of SMP machines.
Comments? Do we need to put a bunch of "volatile" keywords into
every place that uses raw spinlocks? If so, why wasn't the
previous code equally broken??
I don't think the places that use LWLocks need volatile markers on
their data structures, since the LWLock lock and unlock calls will
be out-of-line subroutine calls. But for spinlocks that can be
inlined, it seems there is a risk.
regards, tom lane