Re: Spinlocks and compiler/memory barriers

On Tue, Sep 9, 2014 at 5:32 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-09-09 17:30:44 -0400, Robert Haas wrote:
>> On Tue, Sep 9, 2014 at 5:09 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> > On 2014-09-09 13:52:40 -0400, Robert Haas wrote:
>> >> I had forgotten that it needed an update. Thanks for the reminder. Here's v2.
>> >
>> > I've attached a incremental patch fixing minor gripes. Other than that I
>> > think you can go ahead with this once the buildfarm accepts the sparc
>> > fixes (man, those machines are slow. spoonbill apparently takes ~5h for
>> > one run).
>> >
>> > I've done a read through s_lock.h and the only remaining potential issue
>> > that I see is that I have no idea if unixware's tas() is actually a safe
>> > compiler barrier (it is a memory barrier). And I really, really can't
>> > make myself care.
>>
>> * It is the responsibility of these macros to make sure that the compiler
>> * does not re-order accesses to shared memory to precede the actual lock
>> - * acquisition, or follow the lock release. Prior to PostgreSQL 9.5, this
>> - * was the caller's responsibility, which meant that callers had to use
>> + * acquisition, or following the lock release. Prior to PostgreSQL 9.5,
>> + * this was the caller's responsibility, which meant that callers had to use
>>
>> AFAICS my version is right and your version is grammatically
>> incorrect. "re-order to proceed or follow" uses the same verb tense in
>> both branches of the or; "re-order to proceed or following" does not.
>
> Wasn't sure about that one. It read oddly to me, but then I'm not a
> native speaker. And won't read the sentence often ;)
>
>> I agree that if there are problems on UnixWare, we can let anyone who
>> cares about UnixWare submit a patch to fix them.
>
> Ok.

So, that's committed, then. I think we should pick something that uses
spinlocks and is likely to fail spectacularly if we haven't got this
totally right yet, and de-volatilize it. And then watch to see what
turns red in the buildfarm and/or which users start screaming. I'm
inclined to propose lwlock.c as a candidate, since that's very widely
used and a place where we know there's significant contention.

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