On 06/10/2015 11:34 AM, Andres Freund wrote:
If you check the section where the spinlock is held there's nontrivial
code executed. Under contention you'll have the problem that if backend
tries to acquire the the spinlock while another backend holds the lock,
it'll "steal" the cacheline on which the lock and the rest of the buffer
descriptor resides. Which means that operations like buf->refcount--,the
if (), and the &= will now have to wait till the cacheline is
transferred back (as the cacheline will directly be marked as modified
on the attempted locker). All the while other backends will also try to
acquire the pin, causing the same kind of trouble.
Yes, that is the possibility of "cache line stealing while holding the
lock" that I was talking about. It looks like we are in wild agreement
(as Kevin usually puts it).
If this'd instead be written as
ret = pg_atomic_fetch_sub_u32(&buf->state, 1);
if (ret & BM_PIN_COUNT_WAITER)
{
pg_atomic_fetch_sub_u32(&buf->state, BM_PIN_COUNT_WAITER);
/* XXX: deal with race that another backend has set BM_PIN_COUNT_WAITER */
}
There are atomic AND and OR functions too, at least in the GCC built in
parts. We might be able to come up with pg_atomic_...() versions of them
and avoid the race part.
the whole thing looks entirely different. While there's obviously still
cacheline bouncing, every operation is guaranteed to make progress (on
x86 at least, but even elsewhere we'll never block another locker).
Now unlock is the simpler case, but...
While some locks may be avoidable, some may be replaced by atomic
operations, I believe that we will still be left with some of them.
Optimizing spinlocks if we can do it in a generic fashion that does not
hurt other platforms will still give us something.
Regards, Jan
--
Jan Wieck
Senior Software Engineer
http://slony.info
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
