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
