Re: Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease - Mailing list pgsql-hackers

From knizhnik
Subject Re: Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease
Date
Msg-id 52FCCACC.3030507@garret.ru
Whole thread Raw
In response to Re: Memory ordering issue in LWLockRelease, WakeupWaiters, WALInsertSlotRelease  (Ants Aasma <ants@cybertec.at>)
List pgsql-hackers
On 02/12/2014 06:10 PM, Ants Aasma wrote:
> On Wed, Feb 12, 2014 at 4:04 PM, knizhnik <knizhnik@garret.ru> wrote:
>> Even if reordering was not done by compiler, it still can happen while
>> execution.
>> There is no warranty that two subsequent assignments will be observed by all
>> CPU cores in the same order.
>
> The x86 memory model (total store order) provides that guarantee in
> this specific case.
>
> Regards,
> Ants Aasma
>

Sorry, I thought that we are talking about general case, not just x86 architecture.
May be I do not understand something in LWlock code, but it seems to me that assigning NULL to proc->lwWaitLink is not
neededat all:
 
while (head != NULL){    LOG_LWDEBUG("LWLockRelease", lockid, "release waiter");    proc = head;    head =
proc->lwWaitLink;>>>       proc->lwWaitLink = NULL;    proc->lwWaiting = false;    PGSemaphoreUnlock(&proc->sem);}
 



This part of L1 list is not traversed by any other processor. So nobody will inspect this field. When awakened process
needsto wait for another lock,
 
it  will just assign NULL to this field itself:
    proc->lwWaiting = 1;    proc->lwWaitMode = mode;>>>        proc->lwWaitLink = NULL;    if (lock->head == NULL)
 lock->head = proc;    else        lock->tail->lwWaitLink = proc;    lock->tail = proc;
 

Without TSO (total store order), such assignment of lwWaitLink in LWLockRlease outside critical section may just
corruptL1 list, in which awakened process is already linked.
 
But I am not sure that elimination of this assignment will be enough to ensure correctness of this code without TSO.




pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: [BUG] Archive recovery failure on 9.3+.
Next
From: Magnus Hagander
Date:
Subject: Re: Prevent pg_basebackup -Fp -D -?