On 2013-10-01 14:31:11 +0300, Ants Aasma wrote:
> >> The correct way to think of this is
> >> that StartupXLOG() does a bunch of state modifications and then
> >> advertises the fact that it's done by setting
> >> xlogctl->SharedRecoveryInProgress = false; The state modifications
> >> should better be visible to anyone seeing that last write, so you need
> >> one write barrier between the state modifications and setting the
> >> flag.
> >
> > SpinLockAcquire() should provide that.
>
> Yes. It's notable that in this case it's a matter of correctness that
> the global state modifications do *not* share the critical section
> with the flag update. Otherwise the flag update may become visible
> before the state updates.
I think we're currently essentially assuming that not only
SpinLockAcquire() is a barrier but also that SpinLockRelease() is
one... - which is actually far less likely to be true.
> >> So we need a read barrier somewhere *after* reading the flag in
> >> RecoveryInProgress() and reading the shared memory structures, and in
> >> theory a full barrier if we are going to be writing data. In practice
> >> x86 is covered thanks to it's memory model, Power is covered thanks to
> >> the control dependency and ARM would need a read barrier, but I don't
> >> think any real ARM CPU does speculative stores as that would be
> >> insane.
> >
> > Does there necessarily have to be a "visible" control dependency?
>
> Unfortunately no
That's what I thought :(
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
