On Fri, Jun 27, 2014 at 2:04 PM, Andres Freund <andres@2ndquadrant.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@sss.pgh.pa.us> wrote:
>> > Andres Freund <andres@2ndquadrant.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. The definitions of the
barriers implemented by barrier.h are the same as the ones that Linux
has (minus read-barrier-depends), which I think is probably good
evidence that they are generally useful definitions. 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.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
