Hi, Finally getting to this.
On 2015-07-15 18:44:03 +0300, Heikki Linnakangas wrote: > Previously, LWLockAcquireWithVar set the variable associated with the lock > atomically with acquiring it. Before the lwlock-scalability changes, that > was straightforward because you held the spinlock anyway, but it's a lot > harder/expensive now. So I changed the way acquiring a lock with a variable > works. There is now a separate flag, LW_FLAG_VAR_SET, which indicates that > the current lock holder has updated the variable. The LWLockAcquireWithVar > function is gone - you now just use LWLockAcquire(), which always clears the > LW_FLAG_VAR_SET flag, and you can call LWLockUpdateVar() after that if you > want to set the variable immediately. > > This passes make check, but I haven't done any testing beyond that. Does > this look sane to you? The prime thing I dislike about the patch is how long it now holds the spinlock in WaitForVar. I don't understand why that's necessary? There's no need to hold a spinlock until after the mustwait = (pg_atomic_read_u32(&lock->state) & LW_VAL_EXCLUSIVE) != 0; unless I miss something? In an earlier email you say: > After the spinlock is released above, but before the LWLockQueueSelf() call, > it's possible that another backend comes in, acquires the lock, changes the > variable's value, and releases the lock again. In 9.4, the spinlock was not > released until the process was queued. But that's not a problem. The updater in that case will have queued a wakeup for all waiters, including WaitForVar()? I'll try to reproduce the problem now. But I do wonder if it's possibly just the missing spinlock during the update. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
