Re: Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

On 2014-02-14 13:52:45 +0100, Florian Pflug wrote:
> > I agree we should do that, but imo not in the backbranches. Anything
> > more than than the minimal fix in that code should be avoided in the
> > stable branches, this stuff is friggin performance sensitive, and the
> > spinlock already is a *major* contention point in many workloads.
>
> No argument there. But unless I missed something, there actually is a bug
> there, and using volatile won't fix it. A barrier would, but what about the
> back branches that don't have barrier.h?

Yea, but I don't see a better alternative. Realistically the likelihood
of a problem without the compiler reordering stuff is miniscule on any
relevant platform that's supported by the !barrier.h branches. Newer
ARMs are the only realistic suspect, and the support for in older
releases wasn't so good...

> The former
> leaves me with a bit of an uneasy feeling, and the latter quite certainly has
> a larger performance impact than moving the PGPROC updates within the section
> protected by the spinlock and using an array to remember the backends to wakeup.

I am not so sure, it adds a host of new cacheline references in a piece
of code that's already heavily affected by pipeline stalls, that can
influence performance. I am not saying it's super likely, just more than
I want to do for a theoretical problem in the back branches.

Greetings,

Andres Freund

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