From a50cb4867a7f63bfc03b33bfb5be8d47d15ba64d Mon Sep 17 00:00:00 2001 From: Eugene Kosov Date: Wed, 2 Dec 2020 11:34:37 +0300 Subject: [PATCH] MDEV-24334 make monitor_set_tbl global variable thread-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Atomic_relaxed: add fetch_or() and fetch_and() innodb_init(): rely on a zero-initialization of a global variable monitor_set_tbl: make Atomic_relaxed array and use proper operations for setting bit, unsetting bit and reading bit Reviewed by: Marko Mäkelä --- include/my_atomic_wrapper.h | 5 +++++ storage/innobase/handler/ha_innodb.cc | 3 --- storage/innobase/include/srv0mon.h | 22 +++++++++++----------- storage/innobase/srv/srv0mon.cc | 4 ++-- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/include/my_atomic_wrapper.h b/include/my_atomic_wrapper.h index 64835e30ca7..c574fba4a8e 100644 --- a/include/my_atomic_wrapper.h +++ b/include/my_atomic_wrapper.h @@ -13,6 +13,7 @@ along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335 USA */ +#pragma once #ifdef __cplusplus #include /** @@ -50,6 +51,10 @@ public: { return m.fetch_sub(i, o); } Type fetch_xor(const Type i, std::memory_order o= std::memory_order_relaxed) { return m.fetch_xor(i, o); } + Type fetch_and(const Type i, std::memory_order o= std::memory_order_relaxed) + { return m.fetch_and(i, o); } + Type fetch_or(const Type i, std::memory_order o= std::memory_order_relaxed) + { return m.fetch_or(i, o); } bool compare_exchange_strong(Type& i1, const Type i2, std::memory_order o1= std::memory_order_relaxed, std::memory_order o2= std::memory_order_relaxed) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index a9206df377c..345f2a37d5d 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -4245,9 +4245,6 @@ static int innodb_init(void* p) } #endif /* MYSQL_DYNAMIC_PLUGIN */ - /* Currently, monitor counter information are not persistent. */ - memset(monitor_set_tbl, 0, sizeof monitor_set_tbl); - memset(innodb_counter_value, 0, sizeof innodb_counter_value); /* Do this as late as possible so server is fully starts up, diff --git a/storage/innobase/include/srv0mon.h b/storage/innobase/include/srv0mon.h index 45f9ae3d5e9..58e36676398 100644 --- a/storage/innobase/include/srv0mon.h +++ b/storage/innobase/include/srv0mon.h @@ -498,23 +498,23 @@ enum mon_option_t { /** This "monitor_set_tbl" is a bitmap records whether a particular monitor counter has been turned on or off */ -extern ulint monitor_set_tbl[(NUM_MONITOR + NUM_BITS_ULINT - 1) / - NUM_BITS_ULINT]; +extern Atomic_relaxed + monitor_set_tbl[(NUM_MONITOR + NUM_BITS_ULINT - 1) / NUM_BITS_ULINT]; /** Macros to turn on/off the control bit in monitor_set_tbl for a monitor counter option. */ -#define MONITOR_ON(monitor) \ - (monitor_set_tbl[unsigned(monitor) / NUM_BITS_ULINT] |= \ - (ulint(1) << (unsigned(monitor) % NUM_BITS_ULINT))) +#define MONITOR_ON(monitor) \ + (monitor_set_tbl[unsigned(monitor) / NUM_BITS_ULINT].fetch_or( \ + (ulint(1) << (unsigned(monitor) % NUM_BITS_ULINT)))) -#define MONITOR_OFF(monitor) \ - (monitor_set_tbl[unsigned(monitor) / NUM_BITS_ULINT] &= \ - ~(ulint(1) << (unsigned(monitor) % NUM_BITS_ULINT))) +#define MONITOR_OFF(monitor) \ + (monitor_set_tbl[unsigned(monitor) / NUM_BITS_ULINT].fetch_and( \ + ~(ulint(1) << (unsigned(monitor) % NUM_BITS_ULINT)))) /** Check whether the requested monitor is turned on/off */ -#define MONITOR_IS_ON(monitor) \ - (monitor_set_tbl[unsigned(monitor) / NUM_BITS_ULINT] & \ - (ulint(1) << (unsigned(monitor) % NUM_BITS_ULINT))) +#define MONITOR_IS_ON(monitor) \ + (monitor_set_tbl[unsigned(monitor) / NUM_BITS_ULINT] & \ + (ulint(1) << (unsigned(monitor) % NUM_BITS_ULINT))) /** The actual monitor counter array that records each monintor counter value */ diff --git a/storage/innobase/srv/srv0mon.cc b/storage/innobase/srv/srv0mon.cc index c759a5b75fe..e64d3b9f426 100644 --- a/storage/innobase/srv/srv0mon.cc +++ b/storage/innobase/srv/srv0mon.cc @@ -1424,8 +1424,8 @@ monitor_value_t innodb_counter_value[NUM_MONITOR]; /* monitor_set_tbl is used to record and determine whether a monitor has been turned on/off. */ -ulint monitor_set_tbl[(NUM_MONITOR + NUM_BITS_ULINT - - 1) / NUM_BITS_ULINT]; +Atomic_relaxed + monitor_set_tbl[(NUM_MONITOR + NUM_BITS_ULINT - 1) / NUM_BITS_ULINT]; /****************************************************************//** Get a monitor's "monitor_info" by its monitor id (index into the