On 2015-09-11 19:33:26 +0300, YUriy Zhuravlev wrote:
> On Friday 11 September 2015 18:14:21 Andres Freund wrote:
> > This way we can leave the for (;;) loop
> > in BufferAlloc() thinking that the buffer is unused (and can't be further
> > pinned because of the held spinlock!)
>
> We lost lock after PinBuffer_Locked in BufferAlloc. Therefore, in essence,
> nothing has changed.
The relevant piece of code is: /* * Need to lock the buffer header too in order to change its tag. */
LockBufHdr(buf);
/* * Somebody could have pinned or re-dirtied the buffer while we were * doing the I/O and making the new
hashtableentry. If so, we can't * recycle this buffer; we must undo everything we've done and start * over
witha new victim buffer. */ oldFlags = buf->flags; if (buf->refcount == 1 && !(oldFlags & BM_DIRTY))
break;
UnlockBufHdr(buf); BufTableDelete(&newTag, newHash); if ((oldFlags & BM_TAG_VALID) && oldPartitionLock
!=newPartitionLock) LWLockRelease(oldPartitionLock); LWLockRelease(newPartitionLock); UnpinBuffer(buf,
true);}
/* * Okay, it's finally safe to rename the buffer. * * Clearing BM_VALID here is necessary, clearing the dirtybits is
just* paranoia. We also reset the usage_count since any recency of use of * the old content is no longer relevant.
(Theusage_count starts out at * 1 so that the buffer can survive one clock-sweep pass.) */buf->tag = newTag;buf->flags
&=~(BM_VALID | BM_DIRTY | BM_JUST_DIRTIED | BM_CHECKPOINT_NEEDED | BM_IO_ERROR | BM_PERMANENT);if (relpersistence ==
RELPERSISTENCE_PERMANENT) buf->flags |= BM_TAG_VALID | BM_PERMANENT;else buf->flags |=
BM_TAG_VALID;buf->usage_count= 1;
UnlockBufHdr(buf);
so unless I'm missing something, no, we haven't lost the lock.
Greetings,
Andres Freund
