On Wed, Jul 17, 2013 at 4:57 PM, Tom Lane <t...@sss.pgh.pa.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
