Hi, During the lwlock scalability work I noticed a longstanding issue with the lwlock code. LWLockRelease() and the other mentioned locations do the following to wake up any waiters, without holding the lock's spinlock: /* * Awaken any waiters I removed from the queue. */ while (head != NULL) { LOG_LWDEBUG("LWLockRelease", T_NAME(l), T_ID(l), "release waiter"); proc = head; head = proc->lwWaitLink; proc->lwWaitLink = NULL; proc->lwWaiting = false; PGSemaphoreUnlock(&proc->sem); }
which means they manipulate the lwWaitLink queue without protection. That's done intentionally. The code tries to protect against corruption of the list to do a woken up backend acquiring a lock (this or an independent one) by only continuing when the lwWaiting flag is set to false. Unfortunately there's absolutely no guarantee that a) the assignment to lwWaitLink and lwWaiting are done in that order b) that the stores are done in-order from the POV of other backends. So what we need to do is to acquire a write barrier between the assignments to lwWaitLink and lwWaiting, i.e. proc->lwWaitLink = NULL; pg_write_barrier(); proc->lwWaiting = false; the reader side already uses an implicit barrier by using spinlocks. I've fixed this as part 1 of the lwlock scalability work in [1], but Heikki rightfully suggested that a) this should be backpatched b) done in a separate commit. There is the question what to do about the branches without barriers? I guess a SpinLockAcquire()/Release() would do? Or do we decide it's not important enough to matter, since it's not an issue on x86? [1] http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=commitdiff;h=2de11eb11bb3e3777f6d384de0af9c2f77960637 Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers