Re: Move PinBuffer and UnpinBuffer to atomics

On Sun, Mar 20, 2016 at 4:10 AM, Alexander Korotkov <
a(dot)korotkov(at)postgrespro(dot)ru> wrote:

> Actually, we behave like old code and do such modifications without
> increasing number of atomic operations. We can just calculate new value of
> state (including unset of BM_LOCKED flag) and write it to the buffer. In
> this case we don't require more atomic operations. However, it becomes not
> safe to use atomic decrement in UnpinBuffer(). We can use there loop of
> CAS which wait buffer header to be unlocked like PinBuffer() does. I'm not
> sure if this affects multicore scalability, but I think it worth trying.
>
> So, this idea is implemented in attached patch. Please, try it for both
> regression on lower number of clients and scalability on large number of
> clients.
>
>
Good, seems like we have reduced some instructions, I will test it soon.

> Other changes in this patch:
> 1) PinBuffer() and UnpinBuffer() use exponential backoff in spindealy
> like LockBufHdr() does.
> 2) Previous patch contained revert
> of ac1d7945f866b1928c2554c0f80fd52d7f977772. This was not intentional.
> No, it doesn't contains this revert.
>

Some other comments..

*************** ReadBuffer_common(SMgrRelation smgr, cha
*** 828,837 ****
*/
do
{
! LockBufHdr(bufHdr);
! Assert(bufHdr->flags & BM_VALID);
! bufHdr->flags &= ~BM_VALID;
! UnlockBufHdr(bufHdr);
} while (!StartBufferIO(bufHdr, true));
}
}
--- 826,834 ----
*/
do
{
! uint32 state = LockBufHdr(bufHdr);
! state &= ~(BM_VALID | BM_LOCKED);
! pg_atomic_write_u32(&bufHdr->state, state);
} while (!StartBufferIO(bufHdr, true));

Better Write some comment, about we clearing the BM_LOCKED from stage
directly and need not to call UnlockBufHdr explicitly.
otherwise its confusing.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com