Обсуждение: pgsql: Fix a few problems in barrier.h.
Fix a few problems in barrier.h. On HPPA, implement pg_memory_barrier() as pg_compiler_barrier(), which should be correct since this arch doesn't do memory access reordering, and is anyway better than the completely-nonfunctional-on-this-arch dummy_spinlock code. (But note this patch only fixes things for gcc, not for builds with HP's compiler.) Also, fix incorrect default definition of pg_memory_barrier as a macro requiring an argument. Also, fix incorrect spelling of "#elif" as "#else if" in icc code path (spotted by pgindent). This doesn't come close to fixing all of the functional and stylistic deficiencies in barrier.h, but at least it un-breaks my personal build. Now that we're actually using barriers in the code, this file is going to need some serious attention. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/89779bf2c8f9aa480e0ceb8883f93e9d65c43a6e Modified Files -------------- src/include/storage/barrier.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
On Wed, Jul 17, 2013 at 6:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Fix a few problems in barrier.h. > > On HPPA, implement pg_memory_barrier() as pg_compiler_barrier(), which > should be correct since this arch doesn't do memory access reordering, > and is anyway better than the completely-nonfunctional-on-this-arch > dummy_spinlock code. (But note this patch only fixes things for gcc, > not for builds with HP's compiler.) According to the comments in the barrier.h (and s_lock.h), HPPA is PA-RISC. According to http://en.wikipedia.org/wiki/Memory_ordering, PA-RISC has weak memory ordering. So either this commit is wrong, or one of those things is wrong. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > On Wed, Jul 17, 2013 at 6:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> On HPPA, implement pg_memory_barrier() as pg_compiler_barrier(), which >> should be correct since this arch doesn't do memory access reordering, >> and is anyway better than the completely-nonfunctional-on-this-arch >> dummy_spinlock code. (But note this patch only fixes things for gcc, >> not for builds with HP's compiler.) > According to http://en.wikipedia.org/wiki/Memory_ordering, PA-RISC has > weak memory ordering. > So either this commit is wrong, or one of those things is wrong. If this commit is wrong, then our HPPA spinlock code has been wrong for circa fifteen years. It's entirely possible that that's true and nobody has noticed (perhaps for lack of any multiprocessor HPPA systems to test on?). Still, that would make it roughly six orders of magnitude less broken than "can't get through bootstrap", which is where we were this morning. As I hope my commit comment made clear, this is the beginning of making barrier.h trustworthy, not the end. regards, tom lane
Robert Haas <robertmhaas@gmail.com> writes: > According to http://en.wikipedia.org/wiki/Memory_ordering, PA-RISC has > weak memory ordering. BTW, so far as I can see, their reference for that claim is the two articles by Paul McKenney; but what McKenney actually says is Although the PA-RISC architecture permits full reordering of loads and stores, actual CPUs run fully ordered. This means the Linux kernel's memory-ordering primitives generate no code; they do, however, use the GCC memory attribute to disable compiler optimizations that would reorder code across the memory barrier. So the committed patch duplicates what the kernel does (unless someone cares to dig into the kernel and show otherwise). regards, tom lane