From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: anole: assorted stability problems |
Date: | 2015-07-07 11:25:24 |
Message-ID: | 20150707112524.GJ30359@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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?
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2015-07-07 11:31:34 | Re: 9.6 First Commitfest Begins |
Previous Message | Andres Freund | 2015-07-07 11:21:33 | Re: Repeated pg_upgrade buildfarm failures on binturon |