Re: pg_memory_barrier() doesn't compile, let alone work, for me - Mailing list pgsql-hackers

From Tom Lane
Subject Re: pg_memory_barrier() doesn't compile, let alone work, for me
Date
Msg-id 8421.1374098410@sss.pgh.pa.us
Whole thread Raw
In response to Re: pg_memory_barrier() doesn't compile, let alone work, for me  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
I wrote:
> 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.  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.

After going through s_lock.h another time, I can't help noticing that
a large majority of the non-mainstream architectures make use of the
default version of S_UNLOCK(), which is just

#define S_UNLOCK(lock)        (*((volatile slock_t *) (lock)) = 0)

I assert that if this is a correct implementation, then the platform
does not reorder writes, since correctness requires that any writes to
shared memory variables protected by the lock occur before the lock is
released.

Generally speaking, I'm not seeing any memory-barrier-ish instructions
on the locking side either, meaning there's also no risk of read
reordering.  It's possible that some of these arches do read reordering
except for not hoisting reads before instructions that can be used to
take locks ... but I'll bet that most of them simply don't have weak
memory ordering.

So I'm back to the position that pg_compiler_barrier() is a perfectly
credible default implementation.  More so than an incorrect usage of
spinlocks, anyway.  In particular, I'm going to go fix HPPA that way
so I can get my build working again.

BTW, the only arches for which we seem to have any barrier instructions
in S_UNLOCK are ARM, PPC, Alpha, and MIPS.  Alpha, at least, is probably
dead, and I'm not seeing any MIPS machines in the buildfarm either;
I wouldn't feel bad about desupporting both of those arches.

Also, a comparison to s_lock.h says that the PPC code in barrier.h is a
few bricks shy of a load: it's not honoring USE_PPC_LWSYNC.  And while
I'm bitching, the #ifdef structure in barrier.h is impossible to follow,
not least because none of the #endifs are labeled, contrary to project
style.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Next
From: Josh Berkus
Date:
Subject: Re: Return of "can't paste into psql" issue