Skip to content

Commit 1b12e25

Browse files
committed
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.
1 parent dcdc8c3 commit 1b12e25

File tree

2 files changed

+20
-22
lines changed

2 files changed

+20
-22
lines changed

storage/innobase/buf/buf0buf.cc

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -281,25 +281,17 @@ the read requests for the whole area.
281281
#ifndef UNIV_INNOCHECKSUM
282282
void page_hash_latch::read_lock_wait()
283283
{
284-
auto l= read_lock_yield();
285284
/* First, try busy spinning for a while. */
286285
for (auto spin= srv_n_spin_wait_rounds; spin--; )
287286
{
288-
if (l & WRITER_PENDING)
289-
ut_delay(srv_spin_wait_delay);
287+
ut_delay(srv_spin_wait_delay);
290288
if (read_trylock())
291289
return;
292-
l= read_lock_yield();
293290
}
294291
/* Fall back to yielding to other threads. */
295-
for (;;)
296-
{
297-
if (l & WRITER_PENDING)
298-
os_thread_yield();
299-
if (read_trylock())
300-
return;
301-
l= read_lock_yield();
302-
}
292+
do
293+
os_thread_yield();
294+
while (!read_trylock());
303295
}
304296

305297
void page_hash_latch::write_lock_wait()

storage/innobase/include/rw_lock.h

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,24 @@ class rw_lock
3636
/** Flag to indicate that write_lock() or write_lock_wait() is pending */
3737
static constexpr uint32_t WRITER_PENDING= WRITER | WRITER_WAITING;
3838

39-
/** Yield a read lock request due to a conflict with a write lock.
40-
@return the lock value */
41-
uint32_t read_lock_yield()
42-
{
43-
uint32_t l= lock.fetch_sub(1, std::memory_order_relaxed);
44-
DBUG_ASSERT(l & ~WRITER_PENDING);
45-
return l;
46-
}
4739
/** Start waiting for an exclusive lock. */
4840
void write_lock_wait_start()
4941
{ lock.fetch_or(WRITER_WAITING, std::memory_order_relaxed); }
42+
/** Try to acquire a shared lock.
43+
@param l the value of the lock word
44+
@return whether the lock was acquired */
45+
bool read_trylock(uint32_t &l)
46+
{
47+
l= UNLOCKED;
48+
while (!lock.compare_exchange_strong(l, l + 1, std::memory_order_acquire,
49+
std::memory_order_relaxed))
50+
{
51+
DBUG_ASSERT(!(WRITER & l) || !(~WRITER_PENDING & l));
52+
if (l & WRITER_PENDING)
53+
return false;
54+
}
55+
return true;
56+
}
5057
/** Wait for an exclusive lock.
5158
@return whether the exclusive lock was acquired */
5259
bool write_lock_poll()
@@ -80,8 +87,7 @@ class rw_lock
8087
}
8188
/** Try to acquire a shared lock.
8289
@return whether the lock was acquired */
83-
bool read_trylock()
84-
{ return !(lock.fetch_add(1, std::memory_order_acquire) & WRITER_PENDING); }
90+
bool read_trylock() { uint32_t l; return read_trylock(l); }
8591
/** Try to acquire an exclusive lock.
8692
@return whether the lock was acquired */
8793
bool write_trylock()

0 commit comments

Comments
 (0)