In connection with the nearby thread about spinlock coding rule violations, I noticed that we have several places that are doing things like this:
SpinLockAcquire(&WalRcv->mutex); ... written_lsn = pg_atomic_read_u64(&WalRcv->writtenUpto); ... SpinLockRelease(&WalRcv->mutex); That's from pg_stat_get_wal_receiver(); there is similar code in freelist.c's ClockSweepTick() and StrategySyncStart(). This seems to me to be very bad code. In the first place, on machines without the appropriate type of atomic operation, atomics.c is going to be using a spinlock to emulate atomicity, which means this code tries to take a spinlock while holding another one. That's not okay, either from the standpoint of performance or error-safety. In the second place, this coding seems to me to indicate serious confusion about which lock is protecting what. In the above example, if writtenUpto is only accessed through atomic operations then it seems like we could just move the pg_atomic_read_u64 out of the spinlock section; or if the spinlock is adequate protection then we could just do a normal fetch. If we actually need both locks then this needs significant re-thinking, IMO. Comments? regards, tom lane