Re: Spinlocks and compiler/memory barriers

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Spinlocks and compiler/memory barriers
Date: 2014-06-30 12:03:40
Message-ID: CA+TgmoaUgHXGcDPhVpPPUUXDLgg06N=_C30kQhcbmmb9Z3s1CQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Jun 28, 2014 at 9:41 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> No, I think it's going to be *much* simpler than that. How about I
>> take a crack at this next week and then either (a) I'll see why it's a
>> bad idea and we can go from there or (b) you can review what I come up
>> with and tell me why it sucks?
>
> Ok. I think that's going in the wrong direction (duplication of
> nontrivial knowledge), but maybe I'm taking a to 'purist' approach
> here. Prove me wrong :)

I'm not sure what you'll think of this, but see attached. I think
newer releases of Sun Studio support that GCC way of doing a barrier,
but I don't know how to test for that, so at the moment that uses the
fallback "put it in a function" approach, along with HPPA non-GCC and
Univel CC. Those things are obscure enough that I don't care about
making them less performance. I think AIX is actually OK as-is; if
_check_lock() is a compiler barrier but _clear_lock() is not, that
would be pretty odd.

>> > 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?
>>
>> Because (1) the abstraction doesn't fit very well unless we do a lot
>> of additional work to build acquire and release barriers for every
>> platform we support and
>
> Meh. Something like the (untested):
> #if !defined(pg_release_barrier) && defined(pg_read_barrier) && defined(pg_write_barrier)
> #define pg_release_barrier() do { pg_read_barrier(); pg_write_barrier();} while (0)
> #elif !defined(pg_release_barrier)
> #define pg_release_barrier() pg_memory_barrier()
> #endif
>
> before the fallback definition of pg_read/write_barrier should suffice?

That doesn't sound like a good idea. For example, on PPC, a read
barrier is lwsync, and so is a write barrier. That would also suck on
any architecture where either a read or write barrier gets implemented
as a full memory barrier, though I guess it might be smart enough to
optimize away most of the cost of the second of two barriers in
immediate succession.

I think if we want to use barrier.h as the foundation for this, we're
going to need to define a new set of things in there that have acquire
and release semantics, or just use full barriers for everything -
which would be wasteful on, e.g., x86. And I don't particularly see
the point in going to a lot of work to invent those new abstractions
everywhere when we can just tweak s_lock.h in place a bit and be done
with it. Making those files interdependent doesn't strike me as a
win.

>> (2) I don't have much confidence that we can
>> depend on the spinlock fallback for barriers without completely
>> breaking obscure platforms, and I'd rather make a more minimal set of
>> changes.
>
> Well, it's the beginning of the cycle. And we're already depending on
> barriers for correctness (and it's not getting less), so I don't really
> see what avoiding barrier usage buys us except harder to find breakage?

If we use barriers to implement any facility other than spinlocks, I
have reasonable confidence that we won't break things more than they
already are broken today, because I think the fallback to
acquire-and-release a spinlock probably works, even though it's likely
terrible for performance. I have significantly less confidence that
trying to implement spinlocks in terms of barriers is going to be
reliable.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
spinlock-barrier-rmh.patch text/x-patch 5.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2014-06-30 12:54:27 Re: IMPORT FOREIGN SCHEMA statement
Previous Message Abhijit Menon-Sen 2014-06-30 11:45:27 Re: [BUGS] BUG #9652: inet types don't support min/max