mirrors
/
postgres
mirror of https://github.com/postgres/postgres
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove volatile qualifiers from bufmgr.c and freelist.c 

Prior to commit 0709b7ee72, access to
variables within a spinlock-protected critical section had to be done
through a volatile pointer, but that should no longer be necessary.

Review by Andres Freund
This commit is contained in:
Robert Haas 2015-11-16 18:50:06 -05:00
parent 8004953b5a
commit e93b62985f
3 changed files with 59 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
src/backend/storage/buffer/bufmgr.c
View File

@ -92,11 +92,11 @@ int effective_io_concurrency = 0;
int target_prefetch_pages = 0;
/* local state for StartBufferIO and related functions */
static volatile BufferDesc *InProgressBuf = NULL;
static BufferDesc *InProgressBuf = NULL;
static bool IsForInput;
/* local state for LockBufferForCleanup */
static volatile BufferDesc *PinCountWaitBuf = NULL;
static BufferDesc *PinCountWaitBuf = NULL;
/*
* Backend-Private refcount management:
@ -395,24 +395,24 @@ static Buffer ReadBuffer_common(SMgrRelation reln, char relpersistence,
ForkNumber forkNum, BlockNumber blockNum,
ReadBufferMode mode, BufferAccessStrategy strategy,
bool *hit);
static bool PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy);
static void PinBuffer_Locked(volatile BufferDesc *buf);
static void UnpinBuffer(volatile BufferDesc *buf, bool fixOwner);
static bool PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy);
static void PinBuffer_Locked(BufferDesc *buf);
static void UnpinBuffer(BufferDesc *buf, bool fixOwner);
static void BufferSync(int flags);
static int SyncOneBuffer(int buf_id, bool skip_recently_used);
static void WaitIO(volatile BufferDesc *buf);
static bool StartBufferIO(volatile BufferDesc *buf, bool forInput);
static void TerminateBufferIO(volatile BufferDesc *buf, bool clear_dirty,
static void WaitIO(BufferDesc *buf);
static bool StartBufferIO(BufferDesc *buf, bool forInput);
static void TerminateBufferIO(BufferDesc *buf, bool clear_dirty,
int set_flag_bits);
static void shared_buffer_write_error_callback(void *arg);
static void local_buffer_write_error_callback(void *arg);
static volatile BufferDesc *BufferAlloc(SMgrRelation smgr,
static BufferDesc *BufferAlloc(SMgrRelation smgr,
char relpersistence,
ForkNumber forkNum,
BlockNumber blockNum,
BufferAccessStrategy strategy,
bool *foundPtr);
static void FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln);
static void FlushBuffer(BufferDesc *buf, SMgrRelation reln);
static void AtProcExit_Buffers(int code, Datum arg);
static void CheckForBufferLeaks(void);
static int rnode_comparator(const void *p1, const void *p2);
@ -663,7 +663,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
BlockNumber blockNum, ReadBufferMode mode,
BufferAccessStrategy strategy, bool *hit)
{
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
Block bufBlock;
bool found;
bool isExtend;
@ -927,7 +927,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
*
* No locks are held either at entry or exit.
*/
static volatile BufferDesc *
static BufferDesc *
BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
BlockNumber blockNum,
BufferAccessStrategy strategy,
@ -941,7 +941,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
LWLock *oldPartitionLock; /* buffer partition lock for it */
BufFlags oldFlags;
int buf_id;
volatile BufferDesc *buf;
BufferDesc *buf;
bool valid;
/* create a tag so we can lookup the buffer */
@ -1281,7 +1281,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
* to acquire the necessary locks; if so, don't mess it up.
*/
static void
InvalidateBuffer(volatile BufferDesc *buf)
InvalidateBuffer(BufferDesc *buf)
{
BufferTag oldTag;
uint32 oldHash; /* hash value for oldTag */
@ -1380,7 +1380,7 @@ retry:
void
MarkBufferDirty(Buffer buffer)
{
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
if (!BufferIsValid(buffer))
elog(ERROR, "bad buffer ID: %d", buffer);
@ -1436,7 +1436,7 @@ ReleaseAndReadBuffer(Buffer buffer,
BlockNumber blockNum)
{
ForkNumber forkNum = MAIN_FORKNUM;
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
if (BufferIsValid(buffer))
{
@ -1485,7 +1485,7 @@ ReleaseAndReadBuffer(Buffer buffer,
* some callers to avoid an extra spinlock cycle.
*/
static bool
PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy)
PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy)
{
Buffer b = BufferDescriptorGetBuffer(buf);
bool result;
@ -1547,7 +1547,7 @@ PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy)
* its state can change under us.
*/
static void
PinBuffer_Locked(volatile BufferDesc *buf)
PinBuffer_Locked(BufferDesc *buf)
{
Buffer b;
PrivateRefCountEntry *ref;
@ -1578,7 +1578,7 @@ PinBuffer_Locked(volatile BufferDesc *buf)
* Those that don't should pass fixOwner = FALSE.
*/
static void
UnpinBuffer(volatile BufferDesc *buf, bool fixOwner)
UnpinBuffer(BufferDesc *buf, bool fixOwner)
{
PrivateRefCountEntry *ref;
Buffer b = BufferDescriptorGetBuffer(buf);
@ -1672,7 +1672,7 @@ BufferSync(int flags)
num_to_write = 0;
for (buf_id = 0; buf_id < NBuffers; buf_id++)
{
volatile BufferDesc *bufHdr = GetBufferDescriptor(buf_id);
BufferDesc *bufHdr = GetBufferDescriptor(buf_id);
/*
* Header spinlock is enough to examine BM_DIRTY, see comment in
@ -1707,7 +1707,7 @@ BufferSync(int flags)
num_written = 0;
while (num_to_scan-- > 0)
{
volatile BufferDesc *bufHdr = GetBufferDescriptor(buf_id);
BufferDesc *bufHdr = GetBufferDescriptor(buf_id);
/*
* We don't need to acquire the lock here, because we're only looking
@ -2079,7 +2079,7 @@ BgBufferSync(void)
static int
SyncOneBuffer(int buf_id, bool skip_recently_used)
{
volatile BufferDesc *bufHdr = GetBufferDescriptor(buf_id);
BufferDesc *bufHdr = GetBufferDescriptor(buf_id);
int result = 0;
ReservePrivateRefCountEntry();
@ -2252,7 +2252,7 @@ CheckForBufferLeaks(void)
void
PrintBufferLeakWarning(Buffer buffer)
{
volatile BufferDesc *buf;
BufferDesc *buf;
int32 loccount;
char *path;
BackendId backend;
@ -2324,7 +2324,7 @@ BufmgrCommit(void)
BlockNumber
BufferGetBlockNumber(Buffer buffer)
{
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
Assert(BufferIsPinned(buffer));
@ -2346,7 +2346,7 @@ void
BufferGetTag(Buffer buffer, RelFileNode *rnode, ForkNumber *forknum,
BlockNumber *blknum)
{
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
/* Do the same checks as BufferGetBlockNumber. */
Assert(BufferIsPinned(buffer));
@ -2382,7 +2382,7 @@ BufferGetTag(Buffer buffer, RelFileNode *rnode, ForkNumber *forknum,
* as the second parameter. If not, pass NULL.
*/
static void
FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln)
FlushBuffer(BufferDesc *buf, SMgrRelation reln)
{
XLogRecPtr recptr;
ErrorContextCallback errcallback;
@ -2520,7 +2520,7 @@ RelationGetNumberOfBlocksInFork(Relation relation, ForkNumber forkNum)
bool
BufferIsPermanent(Buffer buffer)
{
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
/* Local buffers are used only for temp relations. */
if (BufferIsLocal(buffer))
@ -2550,7 +2550,7 @@ BufferIsPermanent(Buffer buffer)
XLogRecPtr
BufferGetLSNAtomic(Buffer buffer)
{
volatile BufferDesc *bufHdr = GetBufferDescriptor(buffer - 1);
BufferDesc *bufHdr = GetBufferDescriptor(buffer - 1);
char *page = BufferGetPage(buffer);
XLogRecPtr lsn;
@ -2613,7 +2613,7 @@ DropRelFileNodeBuffers(RelFileNodeBackend rnode, ForkNumber forkNum,
for (i = 0; i < NBuffers; i++)
{
volatile BufferDesc *bufHdr = GetBufferDescriptor(i);
BufferDesc *bufHdr = GetBufferDescriptor(i);
/*
* We can make this a tad faster by prechecking the buffer tag before
@ -2703,7 +2703,7 @@ DropRelFileNodesAllBuffers(RelFileNodeBackend *rnodes, int nnodes)
for (i = 0; i < NBuffers; i++)
{
RelFileNode *rnode = NULL;
volatile BufferDesc *bufHdr = GetBufferDescriptor(i);
BufferDesc *bufHdr = GetBufferDescriptor(i);
/*
* As in DropRelFileNodeBuffers, an unlocked precheck should be safe
@ -2767,7 +2767,7 @@ DropDatabaseBuffers(Oid dbid)
for (i = 0; i < NBuffers; i++)
{
volatile BufferDesc *bufHdr = GetBufferDescriptor(i);
BufferDesc *bufHdr = GetBufferDescriptor(i);
/*
* As in DropRelFileNodeBuffers, an unlocked precheck should be safe
@ -2799,7 +2799,7 @@ PrintBufferDescs(void)
for (i = 0; i < NBuffers; ++i)
{
volatile BufferDesc *buf = GetBufferDescriptor(i);
BufferDesc *buf = GetBufferDescriptor(i);
Buffer b = BufferDescriptorGetBuffer(buf);
/* theoretically we should lock the bufhdr here */
@ -2822,7 +2822,7 @@ PrintPinnedBufs(void)
for (i = 0; i < NBuffers; ++i)
{
volatile BufferDesc *buf = GetBufferDescriptor(i);
BufferDesc *buf = GetBufferDescriptor(i);
Buffer b = BufferDescriptorGetBuffer(buf);
if (GetPrivateRefCount(b) > 0)
@ -2863,7 +2863,7 @@ void
FlushRelationBuffers(Relation rel)
{
int i;
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
/* Open rel at the smgr level if not already done */
RelationOpenSmgr(rel);
@ -2955,7 +2955,7 @@ void
FlushDatabaseBuffers(Oid dbid)
{
int i;
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
/* Make sure we can handle the pin inside the loop */
ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
@ -3064,7 +3064,7 @@ IncrBufferRefCount(Buffer buffer)
void
MarkBufferDirtyHint(Buffer buffer, bool buffer_std)
{
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
Page page = BufferGetPage(buffer);
if (!BufferIsValid(buffer))
@ -3198,7 +3198,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std)
void
UnlockBuffers(void)
{
volatile BufferDesc *buf = PinCountWaitBuf;
BufferDesc *buf = PinCountWaitBuf;
if (buf)
{
@ -3224,7 +3224,7 @@ UnlockBuffers(void)
void
LockBuffer(Buffer buffer, int mode)
{
volatile BufferDesc *buf;
BufferDesc *buf;
Assert(BufferIsValid(buffer));
if (BufferIsLocal(buffer))
@ -3250,7 +3250,7 @@ LockBuffer(Buffer buffer, int mode)
bool
ConditionalLockBuffer(Buffer buffer)
{
volatile BufferDesc *buf;
BufferDesc *buf;
Assert(BufferIsValid(buffer));
if (BufferIsLocal(buffer))
@ -3280,7 +3280,7 @@ ConditionalLockBuffer(Buffer buffer)
void
LockBufferForCleanup(Buffer buffer)
{
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
Assert(BufferIsValid(buffer));
Assert(PinCountWaitBuf == NULL);
@ -3392,7 +3392,7 @@ HoldingBufferPinThatDelaysRecovery(void)
bool
ConditionalLockBufferForCleanup(Buffer buffer)
{
volatile BufferDesc *bufHdr;
BufferDesc *bufHdr;
Assert(BufferIsValid(buffer));
@ -3445,7 +3445,7 @@ ConditionalLockBufferForCleanup(Buffer buffer)
* WaitIO -- Block until the IO_IN_PROGRESS flag on 'buf' is cleared.
*/
static void
WaitIO(volatile BufferDesc *buf)
WaitIO(BufferDesc *buf)
{
/*
* Changed to wait until there's no IO - Inoue 01/13/2000
@ -3492,7 +3492,7 @@ WaitIO(volatile BufferDesc *buf)
* FALSE if someone else already did the work.
*/
static bool
StartBufferIO(volatile BufferDesc *buf, bool forInput)
StartBufferIO(BufferDesc *buf, bool forInput)
{
Assert(!InProgressBuf);
@ -3558,8 +3558,7 @@ StartBufferIO(volatile BufferDesc *buf, bool forInput)
* be 0, or BM_VALID if we just finished reading in the page.
*/
static void
TerminateBufferIO(volatile BufferDesc *buf, bool clear_dirty,
int set_flag_bits)
TerminateBufferIO(BufferDesc *buf, bool clear_dirty, int set_flag_bits)
{
Assert(buf == InProgressBuf);
@ -3590,7 +3589,7 @@ TerminateBufferIO(volatile BufferDesc *buf, bool clear_dirty,
void
AbortBufferIO(void)
{
volatile BufferDesc *buf = InProgressBuf;
BufferDesc *buf = InProgressBuf;
if (buf)
{
@ -3643,7 +3642,7 @@ AbortBufferIO(void)
static void
shared_buffer_write_error_callback(void *arg)
{
volatile BufferDesc *bufHdr = (volatile BufferDesc *) arg;
BufferDesc *bufHdr = (BufferDesc *) arg;
/* Buffer is pinned, so we can read the tag without locking the spinlock */
if (bufHdr != NULL)
@ -3662,7 +3661,7 @@ shared_buffer_write_error_callback(void *arg)
static void
local_buffer_write_error_callback(void *arg)
{
volatile BufferDesc *bufHdr = (volatile BufferDesc *) arg;
BufferDesc *bufHdr = (BufferDesc *) arg;
if (bufHdr != NULL)
{

18
src/backend/storage/buffer/freelist.c
View File

@ -98,9 +98,9 @@ typedef struct BufferAccessStrategyData
/* Prototypes for internal functions */
static volatile BufferDesc *GetBufferFromRing(BufferAccessStrategy strategy);
static BufferDesc *GetBufferFromRing(BufferAccessStrategy strategy);
static void AddBufferToRing(BufferAccessStrategy strategy,
volatile BufferDesc *buf);
BufferDesc *buf);
/*
* ClockSweepTick - Helper routine for StrategyGetBuffer()
@ -179,10 +179,10 @@ ClockSweepTick(void)
* To ensure that no one else can pin the buffer before we do, we must
* return the buffer with the buffer header spinlock still held.
*/
volatile BufferDesc *
BufferDesc *
StrategyGetBuffer(BufferAccessStrategy strategy)
{
volatile BufferDesc *buf;
BufferDesc *buf;
int bgwprocno;
int trycounter;
@ -338,7 +338,7 @@ StrategyGetBuffer(BufferAccessStrategy strategy)
* StrategyFreeBuffer: put a buffer on the freelist
*/
void
StrategyFreeBuffer(volatile BufferDesc *buf)
StrategyFreeBuffer(BufferDesc *buf)
{
SpinLockAcquire(&StrategyControl->buffer_strategy_lock);
@ -584,10 +584,10 @@ FreeAccessStrategy(BufferAccessStrategy strategy)
*
* The bufhdr spin lock is held on the returned buffer.
*/
static volatile BufferDesc *
static BufferDesc *
GetBufferFromRing(BufferAccessStrategy strategy)
{
volatile BufferDesc *buf;
BufferDesc *buf;
Buffer bufnum;
/* Advance to next ring slot */
@ -639,7 +639,7 @@ GetBufferFromRing(BufferAccessStrategy strategy)
* is called with the spinlock held, it had better be quite cheap.
*/
static void
AddBufferToRing(BufferAccessStrategy strategy, volatile BufferDesc *buf)
AddBufferToRing(BufferAccessStrategy strategy, BufferDesc *buf)
{
strategy->buffers[strategy->current] = BufferDescriptorGetBuffer(buf);
}
@ -656,7 +656,7 @@ AddBufferToRing(BufferAccessStrategy strategy, volatile BufferDesc *buf)
* if this buffer should be written and re-used.
*/
bool
StrategyRejectBuffer(BufferAccessStrategy strategy, volatile BufferDesc *buf)
StrategyRejectBuffer(BufferAccessStrategy strategy, BufferDesc *buf)
{
/* We only do this in bulkread mode */
if (strategy->btype != BAS_BULKREAD)

11
src/include/storage/buf_internals.h
View File

@ -194,11 +194,6 @@ typedef union BufferDescPadded
/*
* Macros for acquiring/releasing a shared buffer header's spinlock.
* Do not apply these to local buffers!
*
* Note: as a general coding rule, if you are using these then you probably
* need to be using a volatile-qualified pointer to the buffer header, to
* ensure that the compiler doesn't rearrange accesses to the header to
* occur before or after the spinlock is acquired/released.
*/
#define LockBufHdr(bufHdr) SpinLockAcquire(&(bufHdr)->buf_hdr_lock)
#define UnlockBufHdr(bufHdr) SpinLockRelease(&(bufHdr)->buf_hdr_lock)
@ -216,10 +211,10 @@ extern BufferDesc *LocalBufferDescriptors;
*/
/* freelist.c */
extern volatile BufferDesc *StrategyGetBuffer(BufferAccessStrategy strategy);
extern void StrategyFreeBuffer(volatile BufferDesc *buf);
extern BufferDesc *StrategyGetBuffer(BufferAccessStrategy strategy);
extern void StrategyFreeBuffer(BufferDesc *buf);
extern bool StrategyRejectBuffer(BufferAccessStrategy strategy,
volatile BufferDesc *buf);
BufferDesc *buf);
extern int StrategySyncStart(uint32 *complete_passes, uint32 *num_buf_alloc);
extern void StrategyNotifyBgWriter(int bgwprocno);