Re: Spinlocks and compiler/memory barriers

On 2014-06-27 22:34:19 -0400, Robert Haas wrote:
> On Fri, Jun 27, 2014 at 2:04 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > On 2014-06-27 13:04:02 -0400, Robert Haas wrote:
> >> On Thu, Jun 26, 2014 at 6:40 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> > Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> >> >> On 2014-06-26 14:13:07 -0700, Tom Lane wrote:
> >> >>> Surely it had better be a read barrier as well?
> >> >
> >> >> I don't immediately see why it has to be read barrier? Hoisting a load
> >> >> from after the release into the locked area of code should be safe?
> >> >
> >> > No doubt, but delaying a read till after the unlocking write would
> >> > certainly not be safe.
> >> >
> >> > AFAICT, README.barrier completely fails to define what we think the
> >> > semantics of pg_read_barrier and pg_write_barrier actually are, so if
> >> > you believe that a write barrier prevents reordering of reads relative to
> >> > writes, you'd better propose some new text for that file. It certainly
> >> > doesn't say that today.
> >>
> >> The relevant text is in barrier.h
> >
> > Note that that definition of a write barrier is *not* sufficient for the
> > release of a lock... As I said elsewhere I think all the barrier
> > definitions, except maybe alpha, luckily seem to be strong enough for
> > that anyway.
> >
> > Do we want to introduce acquire/release barriers? Or do we want to
> > redefine the current barriers to be strong enough for that?
>
> Well, unless we're prepared to dump support for an awful lot of
> platfomrs, trying to support acquire and release barriers on every
> platform we support is a doomed effort.

Hm? Just declare them to be as heavy as we need them? Already several
platforms fall back to more heavyweight operations than necessary?

> The definitions of the
> barriers implemented by barrier.h are the same as the ones that Linux
> has (minus read-barrier-depends)

Linux has smb_load_acquire()/smp_store_release() for locks on all
platforms.

> If we were going
> to use any of those in s_lock.h, it'd have to be pg_memory_barrier(),
> but I don't think making s_lock.h dependent on barrier.h is the way to
> go. I think we should just adjust s_lock.h in a minimal way, using
> inline assembler or tweaking the existing assembler or whatever.

Isn't that just going to be repeating the contents of barrier.h pretty
much again? How are you suggesting we deal with the generic S_UNLOCK
case without having a huge ifdef?
Why do we build an abstraction layer (barrier.h) and then not use it?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services