Re: Sync Rep v17
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Marti Raudsepp <marti@juffo.org>
Cc: Fujii Masao <masao.fujii@gmail.com>, Simon Riggs <simon@2ndquadrant.com>, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>, pgsql-hackers@postgresql.org, Daniel Farina <daniel@heroku.com>
Date: 2011-02-22T15:41:24Z
Lists: pgsql-hackers
Marti Raudsepp <marti@juffo.org> writes: > On Tue, Feb 22, 2011 at 07:38, Fujii Masao <masao.fujii@gmail.com> wrote: >> + SpinLockAcquire(&WalSndCtl->ctlmutex); >> + result = WalSndCtl->sync_rep_service_available; >> + SpinLockRelease(&WalSndCtl->ctlmutex); >> volatile pointer needs to be used to prevent code rearrangement. > I don't think that's necessary. Spinlock functions already prevent > reordering using __asm__ __volatile__ You're mistaken. We started using that volatile-pointer convention after noting that some compilers would misoptimize the code otherwise. It's not a problem with LWLock-protected stuff because the LWLock calls are actual out-of-line function calls, and the compiler knows it doesn't know what those functions might do. But gcc is a lot more willing to reorder stuff around asm operations, so you can't assume that SpinLockAcquire/SpinLockRelease are equally safe. The way to prevent optimization bugs is to make sure that the fetches/stores protected by a spinlock are done through volatile pointers. regards, tom lane