Re: Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

On 2014-02-10 19:48:47 +0200, Heikki Linnakangas wrote:
> On 02/10/2014 06:41 PM, Andres Freund wrote:
> >On 2014-02-10 11:20:30 -0500, Tom Lane wrote:
> >>I wrote:
> >>>You didn't really explain why you think that ordering is necessary?
> >>
> >>Actually, after grepping to check my memory of what those fields are
> >>being used for, I have a bigger question: WTF is xlog.c doing being
> >>so friendly with the innards of LWLocks? Surely this needs to get
> >>refactored so that most of WakeupWaiters() and friends is in lwlock.c.
> >>Or has all notion of modularity gone out the window while I wasn't
> >>looking?
> >
> >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..

The lwlock scalability stuff has separated out the enqueue/wakeup code,
that probably should work here as well? And that's a fair portion of the
code. I think it should be doable to make that generic enough that the
actual difference of the struct doesn't matter. It'd also reduce
duplication of LWLockAcquire, ConditionalAcquire, OrWait.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services