Re: Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

On 02/17/2014 10:36 PM, Andres Freund wrote:
> On 2014-02-17 22:30:54 +0200, Heikki Linnakangas wrote:
>> This is what I came up with. I like it, I didn't have to contort lwlocks as
>> much as I feared. I added one field to LWLock structure, which is used to
>> store the position of how far a WAL inserter has progressed. The LWLock code
>> calls it just "value", without caring what's stored in it, and it's used by
>> new functions LWLockWait and LWLockWakeup to implement the behavior the WAL
>> insertion slots have, to wake up other processes waiting for the slot
>> without releasing it.
>>
>> This passes regression tests, but I'll have to re-run the performance tests
>> with this. One worry is that if the padded size of the LWLock struct is
>> smaller than cache line, neighboring WAL insertion locks will compete for
>> the cache line. Another worry is that since I added a field to LWLock
>> struct, it might now take 64 bytes on platforms where it used to be 32 bytes
>> before. That wastes some memory.
>
> Why don't you allocate them in a separate tranche, from xlog.c? Then you
> can store them inside whatever bigger object you want, guaranteeing
> exactly the alignment you need. possibly you even can have the extra
> value in the enclosing object?

Good idea. New patch attached, doing that.

I'll try to find time on some multi-CPU hardware to performance test
this against current master, to make sure there's no regression.

- Heikki