Re: pg_memory_barrier() doesn't compile, let alone work, for me

On Wed, Jul 17, 2013 at 4:57 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> This would not be hard to fix, I think.
>
> Really? Given that the memory barrier primitives are supposed to be,
> well, primitive, I don't think this is exactly a trivial problem.
> There's no good place to initialize such a variable, and there's even
> less of a place to make sure that fork or exec leaves it in an
> appropriate state in the child process.

Well, I admit that I don't really know how spinlocks work on every
obscure platform out there, but I would have thought we could
initialize this in, say, main() and call it good. For that to be not
OK, we'd have to be running on a non-EXEC_BACKEND platform where a
previously initialized spinlock is no longer in a good state after
fork(). Unless you know of a case where that happens, I'd be inclined
to assume it's a non-problem. If we find a counterexample later, then
I'd insert an architecture-specific hack for that platform only, with
a comment along the lines of /* YBGTBFKM */.

>> Well, pg_memory_barrier() isn't even equivalent to
>> pg_compiler_barrier() on x86, which has among the strongest memory
>> orderings out there, so I think your first idea is a non-starter.
>
> Among the strongest memory orderings compared to what? Since what we're
> discussing here is non-mainstream architectures, I think this claim is
> unfounded. Most of the ones I can think of offhand are old enough to
> not even have multiprocessor support, so that the issue is vacuous.

Compared to other multi-processor architectures. I agree that the
barriers are all reducible to compiler barriers on single-processor
architectures, but I think new ports of PostgreSQL are much more
likely to be to multi-processor systems rather than uniprocessor
systems. There are very, very few multi-processor systems where
pg_memory_barrier() is reducible to pg_compiler_barrier().

>> I'm pretty sure we've got latent memory-ordering risks in our existing
>> code which we just haven't detected and fixed yet. Consider, for
>> example, this exciting code from GetNewTransactionId:
>
>> myproc->subxids.xids[nxids] = xid;
>> mypgxact->nxids = nxids + 1;
>
>> I don't believe that's technically safe even on an architecture like
>> x86, because the compiler could decide to reorder those assignments.
>
> Wrong, because both pointers are marked volatile. If the compiler does
> reorder the stores, it's broken. Admittedly, this doesn't say anything
> about hardware reordering :-(

OK, natch. So it's safe on x86, but not on POWER.

>> My preference would be to fix this in a narrow way, by initializing
>> dummy_spinlock somewhere. But if not, then I think #error is the only
>> safe way to go.
>
> I'm inclined to agree that #error is the only realistic answer in
> general, though we could probably go ahead with equating
> pg_memory_barrier to pg_compiler_barrier on specific architectures we
> know are single-processor-only.

I'd be fine with that.

> Unfortunately, that means we just
> raised the bar for porting efforts significantly. And in particular,
> it means somebody had better go through s_lock.h and make sure we have a
> credible barrier implementation for every single arch+compiler supported
> therein.

I tried, but the evidence shows that I have not entirely succeeded. :-(

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