Re: Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> writes:
> On 02/10/2014 06:41 PM, Andres Freund wrote:
>> Well, it's not actually using any lwlock.c code, it's a special case
>> locking logic, just reusing the datastructures. That said, I am not
>> particularly happy about the amount of code it's duplicating from
>> lwlock.c. Pretty much all of WALInsertSlotReleaseOne and most of
>> WALInsertSlotAcquireOne() is a copied.

> I'm not too happy with the amount of copy-paste myself, but there was
> enough difference to regular lwlocks that I didn't want to bother all
> lwlocks with the xlog-specific stuff either. The WAL insert slots do
> share the LWLock-related PGPROC fields though, and semaphore. I'm all
> ears if you have ideas on that..

I agree that if the behavior is considerably different, we don't really
want to try to make LWLockAcquire/Release cater to both this and their
standard behavior. But why not add some additional functions in lwlock.c
that do what xlog wants? If we're going to have mostly-copy-pasted logic,
it'd at least be better if it was in the same file, and not somewhere
that's not even in the same major subtree.

Also, the reason that LWLock isn't an abstract struct is because we wanted
to be able to embed it in other structs; *not* as license for other
modules to fool with its contents. If we were working in C++ we'd
certainly have made all its fields private.

regards, tom lane