Re: Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease

On Feb14, 2014, at 16:32 , Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-02-14 10:26:07 -0500, Tom Lane wrote:
>> Florian Pflug <fgp(at)phlo(dot)org> writes:
>>> Another idea for a fix would be to conflate lwWaiting and lwWaitLink into one
>>> field. We could replace "lwWaiting" by "lwWaitLink != NULL" everywhere it's
>>> tested, and set lwWaitLink to some special non-NULL value (say 0x1) when we
>>> enqueue a PGPROC, instead of setting it to NULL and setting lwWaiting to true.
>>
>>> We'd then depend on pointer-sized stores being atomic, which I think we depend
>>> on in other places already.
>>
>> I don't believe that's true; neither that we depend on it now, nor that
>> it would be safe to do so.
>
> Yea, we currently rely on 4 byte stores being atomic (most notably for
> xids), but we don't rely on anything bigger. I am not sure if there are
> architectures with 64bit pointers that aren't accessed atomically when
> aligned? Even if, that's certainly nothing that should be introduced
> when backpatching.

Hm, we could use 4-byte stores instead of 8-byte stores if we turned lwWaitLink
into an index into the proc array instead of a pointer to the PGPROC struct.

We could then use 0xffffffff instead place of NULL to indicate "not waiting",
and PROCARRAY_MAXPROCS to indicate "waiting, but no successor in the queue".

best regards,
Florian Pflug