On Sun, Mar 20, 2016 at 4:10 AM, Alexander Korotkov < a.korot...@postgrespro.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