Commit 1b12e251 authored by Marko Mäkelä's avatar Marko Mäkelä

MDEV-24271 rw_lock::read_lock_yield() may cause writer starvation

The greedy fetch_add(1) approach of read_trylock() may cause
starvation of a waiting write lock request. Let us use a
compare-and-swap for the read lock acquisition in order to
guarantee the progress of writers.
parent dcdc8c35
...@@ -281,25 +281,17 @@ the read requests for the whole area. ...@@ -281,25 +281,17 @@ the read requests for the whole area.
#ifndef UNIV_INNOCHECKSUM #ifndef UNIV_INNOCHECKSUM
void page_hash_latch::read_lock_wait() void page_hash_latch::read_lock_wait()
{ {
auto l= read_lock_yield();
/* First, try busy spinning for a while. */ /* First, try busy spinning for a while. */
for (auto spin= srv_n_spin_wait_rounds; spin--; ) for (auto spin= srv_n_spin_wait_rounds; spin--; )
{ {
if (l & WRITER_PENDING)
ut_delay(srv_spin_wait_delay); ut_delay(srv_spin_wait_delay);
if (read_trylock()) if (read_trylock())
return; return;
l= read_lock_yield();
} }
/* Fall back to yielding to other threads. */ /* Fall back to yielding to other threads. */
for (;;) do
{
if (l & WRITER_PENDING)
os_thread_yield(); os_thread_yield();
if (read_trylock()) while (!read_trylock());
return;
l= read_lock_yield();
}
} }
void page_hash_latch::write_lock_wait() void page_hash_latch::write_lock_wait()
......
...@@ -36,17 +36,24 @@ class rw_lock ...@@ -36,17 +36,24 @@ class rw_lock
/** Flag to indicate that write_lock() or write_lock_wait() is pending */ /** Flag to indicate that write_lock() or write_lock_wait() is pending */
static constexpr uint32_t WRITER_PENDING= WRITER | WRITER_WAITING; static constexpr uint32_t WRITER_PENDING= WRITER | WRITER_WAITING;
/** Yield a read lock request due to a conflict with a write lock.
@return the lock value */
uint32_t read_lock_yield()
{
uint32_t l= lock.fetch_sub(1, std::memory_order_relaxed);
DBUG_ASSERT(l & ~WRITER_PENDING);
return l;
}
/** Start waiting for an exclusive lock. */ /** Start waiting for an exclusive lock. */
void write_lock_wait_start() void write_lock_wait_start()
{ lock.fetch_or(WRITER_WAITING, std::memory_order_relaxed); } { lock.fetch_or(WRITER_WAITING, std::memory_order_relaxed); }
/** Try to acquire a shared lock.
@param l the value of the lock word
@return whether the lock was acquired */
bool read_trylock(uint32_t &l)
{
l= UNLOCKED;
while (!lock.compare_exchange_strong(l, l + 1, std::memory_order_acquire,
std::memory_order_relaxed))
{
DBUG_ASSERT(!(WRITER & l) || !(~WRITER_PENDING & l));
if (l & WRITER_PENDING)
return false;
}
return true;
}
/** Wait for an exclusive lock. /** Wait for an exclusive lock.
@return whether the exclusive lock was acquired */ @return whether the exclusive lock was acquired */
bool write_lock_poll() bool write_lock_poll()
...@@ -80,8 +87,7 @@ class rw_lock ...@@ -80,8 +87,7 @@ class rw_lock
} }
/** Try to acquire a shared lock. /** Try to acquire a shared lock.
@return whether the lock was acquired */ @return whether the lock was acquired */
bool read_trylock() bool read_trylock() { uint32_t l; return read_trylock(l); }
{ return !(lock.fetch_add(1, std::memory_order_acquire) & WRITER_PENDING); }
/** Try to acquire an exclusive lock. /** Try to acquire an exclusive lock.
@return whether the lock was acquired */ @return whether the lock was acquired */
bool write_trylock() bool write_trylock()
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment