From 07e4b6b2766df4a4774b2887ce4e593069c60178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 16 Dec 2020 17:45:01 +0200 Subject: MDEV-24167 fixup: Wake up all update_lock() in u_unlock() It turns out that the hang that was fixed in commit 43d3dad11468bf45d2098ebca70002c49104f298 for the SRW_LOCK_DUMMY implementation is also possible in the futex implementation. We have observed hangs of ssux_lock_low::u_unlock() on Windows where the undesirable value is rw_lock::UPDATER, in the test mariabackup.xb_compressed_encrypted. The exact sequence of events to the hang is not known, but it seems that u_unlock() had better always wake up one thread. Possibly, the case involves multiple blocked u_unlock(). On a busy server, the hang might be 'rescued' by a subsequent lock acquisition and release that is executed by another thread. rw_lock::update_unlock(): Change the return type to void. ssux_lock_low::u_unlock(): Always invoke readers_wake() [sic], to wake up any pending update_lock() or write_lock(). On futex implementation, this will wake up all waiters. On SRW_LOCK_DUMMY, writer_wake() and readers_wake() do the same thing: wake up one write_lock(), or all update_lock() waiters. --- storage/innobase/include/rw_lock.h | 9 ++++----- storage/innobase/sync/srw_lock.cc | 7 +------ 2 files changed, 5 insertions(+), 11 deletions(-) (limited to 'storage') diff --git a/storage/innobase/include/rw_lock.h b/storage/innobase/include/rw_lock.h index ac01c28f346..cae71e8f6c7 100644 --- a/storage/innobase/include/rw_lock.h +++ b/storage/innobase/include/rw_lock.h @@ -132,14 +132,13 @@ public: DBUG_ASSERT(!(l & WRITER)); /* no write lock must have existed */ return (~WRITER_PENDING & l) == 1; } - /** Release an update lock. - @return whether any writers may have to be woken up */ - bool update_unlock() + /** Release an update lock */ + void update_unlock() { - auto l= lock.fetch_and(~UPDATER, std::memory_order_release); + IF_DBUG_ASSERT(auto l=,) + lock.fetch_and(~UPDATER, std::memory_order_release); /* the update lock must have existed */ DBUG_ASSERT((l & (WRITER | UPDATER)) == UPDATER); - return !(~(WRITER_PENDING | UPDATER) & l); } /** Release an exclusive lock */ void write_unlock() diff --git a/storage/innobase/sync/srw_lock.cc b/storage/innobase/sync/srw_lock.cc index 17260ebb318..45bf7a29c00 100644 --- a/storage/innobase/sync/srw_lock.cc +++ b/storage/innobase/sync/srw_lock.cc @@ -233,13 +233,8 @@ void ssux_lock_low::rd_unlock() { if (read_unlock()) writer_wake(); } void ssux_lock_low::u_unlock() { -#ifdef SRW_LOCK_DUMMY update_unlock(); - writer_wake(); /* Wake up either write_lock() or update_lock() */ -#else - if (update_unlock()) - writer_wake(); /* Wake up one waiter (hopefully the writer) */ -#endif + readers_wake(); /* Wake up all write_lock(), update_lock() */ } void ssux_lock_low::wr_unlock() { write_unlock(); readers_wake(); } -- cgit v1.2.1