Re: anole: assorted stability problems

On Tue, Jul 7, 2015 at 7:25 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2015-06-30 11:35:56 +0200, Andres Freund wrote:
>> On 2015-06-29 22:58:05 -0400, Tom Lane wrote:
>> > So personally, I would be inclined to put back the volatile qualifier,
>> > independently of any fooling around with _Asm_double_magic_xyzzy
>> > calls.
>>
>> I'm not sure. I think the reliance on an explicit memory barrier is a
>> lot more robust and easy to understand than some barely documented odd
>> behaviour around volatile. On the other hand the old way worked for a
>> long while.
>>
>> I'm inclined to just do both on platforms as odd as IA6. But it'd like
>> to let anole run with the current set a bit longer - if it doesn't work
>> we have more problems than just S_UNLOCK(). It seems EDB has increased
>> the run rate for now, so it shouldn't take too long:
>> http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=anole&br=HEAD
>
> So, it's starting to look good. Not exactly allowing for a lot of
> confidence yet, but still:
> http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=anole&br=HEAD
>
> I'm inclined to simply revise the comments now, and *not* reintroduce
> the volatile. The assumptions documented in:
>
> /*
> * Intel Itanium, gcc or Intel's compiler.
> *
> * Itanium has weak memory ordering, but we rely on the compiler to enforce
> * strict ordering of accesses to volatile data. In particular, while the
> * xchg instruction implicitly acts as a memory barrier with 'acquire'
> * semantics, we do not have an explicit memory fence instruction in the
> * S_UNLOCK macro. We use a regular assignment to clear the spinlock, and
> * trust that the compiler marks the generated store instruction with the
> * ".rel" opcode.
> *
> * Testing shows that assumption to hold on gcc, although I could not find
> * any explicit statement on that in the gcc manual. In Intel's compiler,
> * the -m[no-]serialize-volatile option controls that, and testing shows that
> * it is enabled by default.
> */
>
> don't sound exactly bullet proof to me. I also personally find explicit
> barriers easier to understand in the light of all the other spinlock
> implementations.
>
> Comments?

I'm fine with that plan.

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