Commit 2bd661ca authored by Marko Mäkelä's avatar Marko Mäkelä

MDEV-34178: Simplify the U lock

The U lock mode of the sux_lock that was introduced in
commit 03ca6495 (MDEV-24142)
is unnecessarily complex.

Internally, sux_lock comprises two parts, each with their own wait queue
inside the operating system kernel: a mutex and a rw-lock.

We can map the operations as follows:

x_lock(): (X,X)
u_lock(): (X,_)
s_lock(): (_,S)

The Update lock mode, which is mutually exclusive with itself and with
X (exclusive) locks but not with shared (S) locks, was unnecessarily
acquiring a shared lock on the second component. The mutual exclusion
is guaranteed by the first component.

We might simplify the #ifdef SUX_LOCK_GENERIC case further by omitting
srw_mutex_impl::lock, because it is kind-of duplicating the mutex
that we will use for having a wait queue. However, the predicate
buf_page_t::can_relocate() would depend on the predicate
is_locked_or_waiting(), which is not available for pthread_mutex_t.

Reviewed by: Debarun Banerjee
Tested by: Matthias Leich
parent 83d3ed49
...@@ -223,12 +223,7 @@ class ssux_lock_impl ...@@ -223,12 +223,7 @@ class ssux_lock_impl
bool u_lock_try() bool u_lock_try()
{ {
if (!writer.wr_lock_try()) return writer.wr_lock_try();
return false;
IF_DBUG_ASSERT(uint32_t lk=,)
readers.fetch_add(1, std::memory_order_acquire);
DBUG_ASSERT(lk < WRITER - 1);
return true;
} }
bool wr_lock_try() bool wr_lock_try()
...@@ -248,9 +243,6 @@ class ssux_lock_impl ...@@ -248,9 +243,6 @@ class ssux_lock_impl
void u_lock() void u_lock()
{ {
writer.wr_lock(); writer.wr_lock();
IF_DBUG_ASSERT(uint32_t lk=,)
readers.fetch_add(1, std::memory_order_acquire);
DBUG_ASSERT(lk < WRITER - 1);
} }
void wr_lock() void wr_lock()
{ {
...@@ -272,15 +264,15 @@ class ssux_lock_impl ...@@ -272,15 +264,15 @@ class ssux_lock_impl
void u_wr_upgrade() void u_wr_upgrade()
{ {
DBUG_ASSERT(writer.is_locked()); DBUG_ASSERT(writer.is_locked());
uint32_t lk= readers.fetch_add(WRITER - 1, std::memory_order_acquire); uint32_t lk= readers.fetch_add(WRITER, std::memory_order_acquire);
if (lk != 1) if (lk)
wr_wait(lk - 1); wr_wait(lk);
} }
void wr_u_downgrade() void wr_u_downgrade()
{ {
DBUG_ASSERT(writer.is_locked()); DBUG_ASSERT(writer.is_locked());
DBUG_ASSERT(is_write_locked()); DBUG_ASSERT(is_write_locked());
readers.store(1, std::memory_order_release); readers.store(0, std::memory_order_release);
/* Note: Any pending rd_lock() will not be woken up until u_unlock() */ /* Note: Any pending rd_lock() will not be woken up until u_unlock() */
} }
...@@ -293,10 +285,6 @@ class ssux_lock_impl ...@@ -293,10 +285,6 @@ class ssux_lock_impl
} }
void u_unlock() void u_unlock()
{ {
IF_DBUG_ASSERT(uint32_t lk=,)
readers.fetch_sub(1, std::memory_order_release);
DBUG_ASSERT(lk);
DBUG_ASSERT(lk < WRITER);
writer.wr_unlock(); writer.wr_unlock();
} }
void wr_unlock() void wr_unlock()
......
...@@ -525,7 +525,7 @@ void ssux_lock::psi_u_wr_upgrade(const char *file, unsigned line) ...@@ -525,7 +525,7 @@ void ssux_lock::psi_u_wr_upgrade(const char *file, unsigned line)
{ {
PSI_rwlock_locker_state state; PSI_rwlock_locker_state state;
DBUG_ASSERT(lock.writer.is_locked()); DBUG_ASSERT(lock.writer.is_locked());
uint32_t lk= 1; uint32_t lk= 0;
const bool nowait= const bool nowait=
lock.readers.compare_exchange_strong(lk, ssux_lock_impl<false>::WRITER, lock.readers.compare_exchange_strong(lk, ssux_lock_impl<false>::WRITER,
std::memory_order_acquire, std::memory_order_acquire,
......
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