Re: anole: assorted stability problems

On Mon, Jun 29, 2015 at 10:53 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2015-06-29 22:45:49 -0400, Robert Haas wrote:
>> On Mon, Jun 29, 2015 at 10:32 PM, Andres Freund <andres@anarazel.de> wrote:
>> > On 2015-06-29 22:11:33 -0400, Robert Haas wrote:
>> >> On Mon, Jun 29, 2015 at 6:11 AM, Andres Freund <andres@anarazel.de> wrote:
>> >> > On 2015-06-29 00:42:53 -0400, Tom Lane wrote:
>> >> >> #define S_UNLOCK(lock)        \
>> >> >>       do { _Asm_sched_fence(); (*(lock)) = 0; } while (0)
>> >> >
>> >> > Robert, how did you choose that? Isn't _Asm_sched_fence just a compiler
>> >> > barrier? Shouldn't this be a _Asm_mf()?
>> >>
>> >> The point of the commit was to make spinlocks act as compiler barriers
>> >> as well as CPU barriers.  So I was just looking to add a compiler
>> >> barrier to what was already there.
>> >
>> > You removed a volatile at the same time, and volatile on IA64 has
>> > acquire/release semantics.
>>
>> Can you explain what you mean by volatile having acquire/release
>> semantics?  I don't see how volatile can create a CPU barrier, but I'm
>> guessing that it somehow can and that you're about to enlighten me.
>
> It's a IA64 pecularity. I think it started with intel's compiler, but
> since then at least msvc and gcc copied it. In essence volatile reads
> implicitly have acquire semantics, and volatile writes release. That's
> relatively cheap on IA64 because they have 'opcode tags' marking normal
> moves etc. as having release or acquire semantics (mov.rel etc.).
>
> We even have a comment about it that scratches the surface a bit:
> /*
>  * 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.
>  */

Ah.  So my bad, then, removing the volatile.  :-(

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