From 0a75f504ccfa12c4cc4c001fae65c844daba39ad Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 7 Jul 2019 12:33:41 +0800 Subject: [PATCH] erasure-code: s/Mutex/ceph::mutex/ Signed-off-by: Kefu Chai --- src/erasure-code/ErasureCodePlugin.cc | 23 +++-- src/erasure-code/ErasureCodePlugin.h | 8 +- .../isa/ErasureCodeIsaTableCache.cc | 22 ++--- .../isa/ErasureCodeIsaTableCache.h | 12 ++- .../shec/ErasureCodeShecTableCache.cc | 16 ++-- .../shec/ErasureCodeShecTableCache.h | 14 ++- .../erasure-code/ErasureCodePluginHangs.cc | 2 +- .../erasure-code/TestErasureCodePlugin.cc | 87 +++++++++---------- .../TestErasureCodeShec_thread.cc | 2 +- src/test/erasure-code/ceph_erasure_code.cc | 2 +- 10 files changed, 88 insertions(+), 100 deletions(-) diff --git a/src/erasure-code/ErasureCodePlugin.cc b/src/erasure-code/ErasureCodePlugin.cc index bc595629bd8..a42bd957a11 100644 --- a/src/erasure-code/ErasureCodePlugin.cc +++ b/src/erasure-code/ErasureCodePlugin.cc @@ -22,6 +22,7 @@ #include "ErasureCodePlugin.h" #include "common/errno.h" #include "include/str_list.h" +#include "include/ceph_assert.h" using namespace std; @@ -34,14 +35,11 @@ using namespace std; #define PLUGIN_INIT_FUNCTION "__erasure_code_init" #define PLUGIN_VERSION_FUNCTION "__erasure_code_version" +namespace ceph { + ErasureCodePluginRegistry ErasureCodePluginRegistry::singleton; -ErasureCodePluginRegistry::ErasureCodePluginRegistry() : - lock("ErasureCodePluginRegistry::lock"), - loading(false), - disable_dlclose(false) -{ -} +ErasureCodePluginRegistry::ErasureCodePluginRegistry() = default; ErasureCodePluginRegistry::~ErasureCodePluginRegistry() { @@ -59,7 +57,7 @@ ErasureCodePluginRegistry::~ErasureCodePluginRegistry() int ErasureCodePluginRegistry::remove(const std::string &name) { - ceph_assert(lock.is_locked()); + ceph_assert(ceph_mutex_is_locked(lock)); if (plugins.find(name) == plugins.end()) return -ENOENT; std::map::iterator plugin = plugins.find(name); @@ -73,7 +71,7 @@ int ErasureCodePluginRegistry::remove(const std::string &name) int ErasureCodePluginRegistry::add(const std::string &name, ErasureCodePlugin* plugin) { - ceph_assert(lock.is_locked()); + ceph_assert(ceph_mutex_is_locked(lock)); if (plugins.find(name) != plugins.end()) return -EEXIST; plugins[name] = plugin; @@ -82,7 +80,7 @@ int ErasureCodePluginRegistry::add(const std::string &name, ErasureCodePlugin *ErasureCodePluginRegistry::get(const std::string &name) { - ceph_assert(lock.is_locked()); + ceph_assert(ceph_mutex_is_locked(lock)); if (plugins.find(name) != plugins.end()) return plugins[name]; else @@ -97,7 +95,7 @@ int ErasureCodePluginRegistry::factory(const std::string &plugin_name, { ErasureCodePlugin *plugin; { - Mutex::Locker l(lock); + std::lock_guard l{lock}; plugin = get(plugin_name); if (plugin == 0) { loading = true; @@ -128,7 +126,7 @@ int ErasureCodePluginRegistry::load(const std::string &plugin_name, ErasureCodePlugin **plugin, ostream *ss) { - ceph_assert(lock.is_locked()); + ceph_assert(ceph_mutex_is_locked(lock)); std::string fname = directory + "/" PLUGIN_PREFIX + plugin_name + PLUGIN_SUFFIX; void *library = dlopen(fname.c_str(), RTLD_NOW); @@ -187,7 +185,7 @@ int ErasureCodePluginRegistry::preload(const std::string &plugins, const std::string &directory, ostream *ss) { - Mutex::Locker l(lock); + std::lock_guard l{lock}; list plugins_list; get_str_list(plugins, plugins_list); for (list::iterator i = plugins_list.begin(); @@ -200,3 +198,4 @@ int ErasureCodePluginRegistry::preload(const std::string &plugins, } return 0; } +} diff --git a/src/erasure-code/ErasureCodePlugin.h b/src/erasure-code/ErasureCodePlugin.h index a92d0e2cc61..f3d122bd590 100644 --- a/src/erasure-code/ErasureCodePlugin.h +++ b/src/erasure-code/ErasureCodePlugin.h @@ -18,7 +18,7 @@ #ifndef CEPH_ERASURE_CODE_PLUGIN_H #define CEPH_ERASURE_CODE_PLUGIN_H -#include "common/Mutex.h" +#include "common/ceph_mutex.h" #include "ErasureCodeInterface.h" extern "C" { @@ -44,9 +44,9 @@ namespace ceph { class ErasureCodePluginRegistry { public: - Mutex lock; - bool loading; - bool disable_dlclose; + ceph::mutex lock = ceph::make_mutex("ErasureCodePluginRegistry::lock"); + bool loading = false; + bool disable_dlclose = false; std::map plugins; static ErasureCodePluginRegistry singleton; diff --git a/src/erasure-code/isa/ErasureCodeIsaTableCache.cc b/src/erasure-code/isa/ErasureCodeIsaTableCache.cc index bc34a8f961f..8a3318aa118 100644 --- a/src/erasure-code/isa/ErasureCodeIsaTableCache.cc +++ b/src/erasure-code/isa/ErasureCodeIsaTableCache.cc @@ -47,7 +47,7 @@ _tc_prefix(std::ostream* _dout) ErasureCodeIsaTableCache::~ErasureCodeIsaTableCache() { - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; codec_technique_tables_t::const_iterator ttables_it; codec_tables_t::const_iterator tables_it; @@ -101,7 +101,7 @@ ErasureCodeIsaTableCache::~ErasureCodeIsaTableCache() int ErasureCodeIsaTableCache::getDecodingTableCacheSize(int matrixtype) { - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; if (decoding_tables[matrixtype]) return decoding_tables[matrixtype]->size(); else @@ -114,7 +114,7 @@ ErasureCodeIsaTableCache::lru_map_t* ErasureCodeIsaTableCache::getDecodingTables(int matrix_type) { // the caller must hold the guard mutex: - // => Mutex::Locker lock(codec_tables_guard); + // => std::lock_guard lock{codec_tables_guard}; // create an lru_map if not yet allocated if (!decoding_tables[matrix_type]) { @@ -129,7 +129,7 @@ ErasureCodeIsaTableCache::lru_list_t* ErasureCodeIsaTableCache::getDecodingTablesLru(int matrix_type) { // the caller must hold the guard mutex: - // => Mutex::Locker lock(codec_tables_guard); + // => std::lock_guard lock{codec_tables_guard}; // create an lru_list if not yet allocated if (!decoding_tables_lru[matrix_type]) { @@ -143,7 +143,7 @@ ErasureCodeIsaTableCache::getDecodingTablesLru(int matrix_type) unsigned char** ErasureCodeIsaTableCache::getEncodingTable(int matrix, int k, int m) { - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; return getEncodingTableNoLock(matrix,k,m); } @@ -165,7 +165,7 @@ ErasureCodeIsaTableCache::getEncodingTableNoLock(int matrix, int k, int m) unsigned char** ErasureCodeIsaTableCache::getEncodingCoefficient(int matrix, int k, int m) { - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; return getEncodingCoefficientNoLock(matrix,k,m); } @@ -187,7 +187,7 @@ ErasureCodeIsaTableCache::getEncodingCoefficientNoLock(int matrix, int k, int m) unsigned char* ErasureCodeIsaTableCache::setEncodingTable(int matrix, int k, int m, unsigned char* ec_in_table) { - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; unsigned char** ec_out_table = getEncodingTableNoLock(matrix, k, m); if (*ec_out_table) { // somebody might have deposited this table in the meanwhile, so clean @@ -206,7 +206,7 @@ ErasureCodeIsaTableCache::setEncodingTable(int matrix, int k, int m, unsigned ch unsigned char* ErasureCodeIsaTableCache::setEncodingCoefficient(int matrix, int k, int m, unsigned char* ec_in_coeff) { - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; unsigned char** ec_out_coeff = getEncodingCoefficientNoLock(matrix, k, m); if (*ec_out_coeff) { // somebody might have deposited these coefficients in the meanwhile, so clean @@ -222,7 +222,7 @@ ErasureCodeIsaTableCache::setEncodingCoefficient(int matrix, int k, int m, unsig // ----------------------------------------------------------------------------- -Mutex* +ceph::mutex* ErasureCodeIsaTableCache::getLock() { return &codec_tables_guard; @@ -246,7 +246,7 @@ ErasureCodeIsaTableCache::getDecodingTableFromCache(std::string &signature, // we try to fetch a decoding table from an LRU cache bool found = false; - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; lru_map_t* decode_tbls_map = getDecodingTables(matrixtype); @@ -286,7 +286,7 @@ ErasureCodeIsaTableCache::putDecodingTableToCache(std::string &signature, ceph::buffer::ptr cachetable; - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; lru_map_t* decode_tbls_map = getDecodingTables(matrixtype); diff --git a/src/erasure-code/isa/ErasureCodeIsaTableCache.h b/src/erasure-code/isa/ErasureCodeIsaTableCache.h index c9ba7b2301a..8110a4660df 100644 --- a/src/erasure-code/isa/ErasureCodeIsaTableCache.h +++ b/src/erasure-code/isa/ErasureCodeIsaTableCache.h @@ -26,7 +26,7 @@ #define CEPH_ERASURE_CODE_ISA_TABLE_CACHE_H // ----------------------------------------------------------------------------- -#include "common/Mutex.h" +#include "common/ceph_mutex.h" #include "erasure-code/ErasureCodeInterface.h" // ----------------------------------------------------------------------------- #include @@ -55,14 +55,12 @@ public: typedef std::map< std::string, lru_entry_t > lru_map_t; typedef std::list< std::string > lru_list_t; - ErasureCodeIsaTableCache() : - codec_tables_guard("isa-lru-cache") - { - } + ErasureCodeIsaTableCache() = default; virtual ~ErasureCodeIsaTableCache(); - Mutex codec_tables_guard; // mutex used to protect modifications in encoding/decoding table maps + // mutex used to protect modifications in encoding/decoding table maps + ceph::mutex codec_tables_guard = ceph::make_mutex("isa-lru-cache"); bool getDecodingTableFromCache(std::string &signature, unsigned char* &table, @@ -98,7 +96,7 @@ private: lru_list_t* getDecodingTablesLru(int matrix_type); - Mutex* getLock(); + ceph::mutex* getLock(); }; diff --git a/src/erasure-code/shec/ErasureCodeShecTableCache.cc b/src/erasure-code/shec/ErasureCodeShecTableCache.cc index 891cada6432..2f9a60b62bc 100644 --- a/src/erasure-code/shec/ErasureCodeShecTableCache.cc +++ b/src/erasure-code/shec/ErasureCodeShecTableCache.cc @@ -41,7 +41,7 @@ _tc_prefix(std::ostream* _dout) { ErasureCodeShecTableCache::~ErasureCodeShecTableCache() { - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; // clean-up all allocated tables { @@ -94,7 +94,7 @@ ErasureCodeShecTableCache::~ErasureCodeShecTableCache() ErasureCodeShecTableCache::lru_map_t* ErasureCodeShecTableCache::getDecodingTables(int technique) { // the caller must hold the guard mutex: - // => Mutex::Locker lock(codec_tables_guard); + // => std::lock_guard lock{codec_tables_guard}; // create an lru_map if not yet allocated if (!decoding_tables[technique]) { @@ -106,7 +106,7 @@ ErasureCodeShecTableCache::getDecodingTables(int technique) { ErasureCodeShecTableCache::lru_list_t* ErasureCodeShecTableCache::getDecodingTablesLru(int technique) { // the caller must hold the guard mutex: - // => Mutex::Locker lock(codec_tables_guard); + // => std::lock_guard lock{codec_tables_guard}; // create an lru_list if not yet allocated if (!decoding_tables_lru[technique]) { @@ -118,7 +118,7 @@ ErasureCodeShecTableCache::getDecodingTablesLru(int technique) { int** ErasureCodeShecTableCache::getEncodingTable(int technique, int k, int m, int c, int w) { - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; return getEncodingTableNoLock(technique,k,m,c,w); } @@ -138,7 +138,7 @@ ErasureCodeShecTableCache::getEncodingTableNoLock(int technique, int k, int m, i int* ErasureCodeShecTableCache::setEncodingTable(int technique, int k, int m, int c, int w, int* ec_in_table) { - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; int** ec_out_table = getEncodingTableNoLock(technique, k, m, c, w); if (*ec_out_table) { // somebody might have deposited this table in the meanwhile, so clean @@ -152,7 +152,7 @@ ErasureCodeShecTableCache::setEncodingTable(int technique, int k, int m, int c, } } -Mutex* +ceph::mutex* ErasureCodeShecTableCache::getLock() { return &codec_tables_guard; @@ -193,7 +193,7 @@ ErasureCodeShecTableCache::getDecodingTableFromCache(int* decoding_matrix, // -------------------------------------------------------------------------- uint64_t signature = getDecodingCacheSignature(k, m, c, w, erased, avails); - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; dout(20) << "[ get table ] = " << signature << dendl; @@ -248,7 +248,7 @@ ErasureCodeShecTableCache::putDecodingTableToCache(int* decoding_matrix, // LRU decoding matrix cache // -------------------------------------------------------------------------- - Mutex::Locker lock(codec_tables_guard); + std::lock_guard lock{codec_tables_guard}; uint64_t signature = getDecodingCacheSignature(k, m, c, w, erased, avails); dout(20) << "[ put table ] = " << signature << dendl; diff --git a/src/erasure-code/shec/ErasureCodeShecTableCache.h b/src/erasure-code/shec/ErasureCodeShecTableCache.h index e4eaf0f0eaa..bed7fa01859 100644 --- a/src/erasure-code/shec/ErasureCodeShecTableCache.h +++ b/src/erasure-code/shec/ErasureCodeShecTableCache.h @@ -21,7 +21,7 @@ #define CEPH_ERASURE_CODE_SHEC_TABLE_CACHE_H // ----------------------------------------------------------------------------- -#include "common/Mutex.h" +#include "common/ceph_mutex.h" #include "erasure-code/ErasureCodeInterface.h" // ----------------------------------------------------------------------------- #include @@ -78,14 +78,10 @@ class ErasureCodeShecTableCache { typedef std::map< uint64_t, lru_entry_t > lru_map_t; typedef std::list< uint64_t > lru_list_t; - ErasureCodeShecTableCache() : - codec_tables_guard("shec-lru-cache") - { - } - + ErasureCodeShecTableCache() = default; virtual ~ErasureCodeShecTableCache(); - - Mutex codec_tables_guard; // mutex used to protect modifications in encoding/decoding table maps + // mutex used to protect modifications in encoding/decoding table maps + ceph::mutex codec_tables_guard = ceph::make_mutex("shec-lru-cache"); bool getDecodingTableFromCache(int* matrix, int* dm_row, int* dm_column, @@ -118,7 +114,7 @@ class ErasureCodeShecTableCache { uint64_t getDecodingCacheSignature(int k, int m, int c, int w, int *want, int *avails); - Mutex* getLock(); + ceph::mutex* getLock(); }; #endif diff --git a/src/test/erasure-code/ErasureCodePluginHangs.cc b/src/test/erasure-code/ErasureCodePluginHangs.cc index 55b9e5a4a80..037fff1bbf4 100644 --- a/src/test/erasure-code/ErasureCodePluginHangs.cc +++ b/src/test/erasure-code/ErasureCodePluginHangs.cc @@ -22,6 +22,6 @@ extern "C" const char *__erasure_code_version() { return CEPH_GIT_NICE_VER; } extern "C" int __erasure_code_init(char *plugin_name, char *directory) { - sleep(1000); + sleep(10); return 0; } diff --git a/src/test/erasure-code/TestErasureCodePlugin.cc b/src/test/erasure-code/TestErasureCodePlugin.cc index 08404b6e503..c00d2997fbd 100644 --- a/src/test/erasure-code/TestErasureCodePlugin.cc +++ b/src/test/erasure-code/TestErasureCodePlugin.cc @@ -25,58 +25,53 @@ #include "gtest/gtest.h" -class ErasureCodePluginRegistryTest : public ::testing::Test { -protected: - - class Thread_factory : public Thread { - public: - static void cleanup(void *arg) { - ErasureCodePluginRegistry &instance = ErasureCodePluginRegistry::instance(); - if (instance.lock.is_locked()) - instance.lock.Unlock(); - } - - void *entry() override { - ErasureCodeProfile profile; - ErasureCodePluginRegistry &instance = ErasureCodePluginRegistry::instance(); - ErasureCodeInterfaceRef erasure_code; - pthread_cleanup_push(cleanup, NULL); - instance.factory("hangs", - g_conf().get_val("erasure_code_dir"), - profile, &erasure_code, &cerr); - pthread_cleanup_pop(0); - return NULL; - } - }; - -}; +class ErasureCodePluginRegistryTest : public ::testing::Test {}; TEST_F(ErasureCodePluginRegistryTest, factory_mutex) { ErasureCodePluginRegistry &instance = ErasureCodePluginRegistry::instance(); - EXPECT_TRUE(instance.lock.TryLock()); - instance.lock.Unlock(); - + { + unique_lock l{instance.lock, std::try_to_lock}; + EXPECT_TRUE(l.owns_lock()); + } // // Test that the loading of a plugin is protected by a mutex. - // - useconds_t delay = 0; - const useconds_t DELAY_MAX = 20 * 1000 * 1000; - Thread_factory sleep_forever; - sleep_forever.create("sleep_forever"); - do { - cout << "Trying (1) with delay " << delay << "us\n"; - if (delay > 0) - usleep(delay); - if (!instance.loading) - delay = ( delay + 1 ) * 2; - } while(!instance.loading && delay < DELAY_MAX); - ASSERT_TRUE(delay < DELAY_MAX); - - EXPECT_FALSE(instance.lock.TryLock()); - EXPECT_EQ(0, pthread_cancel(sleep_forever.get_thread_id())); - EXPECT_EQ(0, sleep_forever.join()); + std::thread sleep_for_10_secs([] { + ErasureCodeProfile profile; + ErasureCodePluginRegistry &instance = ErasureCodePluginRegistry::instance(); + ErasureCodeInterfaceRef erasure_code; + instance.factory("hangs", + g_conf().get_val("erasure_code_dir"), + profile, &erasure_code, &cerr); + }); + auto wait_until = [&instance](bool loading, unsigned max_secs) { + auto delay = 0ms; + const auto DELAY_MAX = std::chrono::seconds(max_secs); + for (; delay < DELAY_MAX; delay = (delay + 1ms) * 2) { + cout << "Trying (1) with delay " << delay << "us\n"; + if (delay.count() > 0) { + std::this_thread::sleep_for(delay); + } + if (instance.loading == loading) { + return true; + } + } + return false; + }; + // should be loading in 5 seconds + ASSERT_TRUE(wait_until(true, 5)); + { + unique_lock l{instance.lock, std::try_to_lock}; + EXPECT_TRUE(!l.owns_lock()); + } + // should finish loading in 15 seconds + ASSERT_TRUE(wait_until(false, 15)); + { + unique_lock l{instance.lock, std::try_to_lock}; + EXPECT_TRUE(l.owns_lock()); + } + sleep_for_10_secs.join(); } TEST_F(ErasureCodePluginRegistryTest, all) @@ -116,7 +111,7 @@ TEST_F(ErasureCodePluginRegistryTest, all) EXPECT_TRUE(erasure_code.get()); ErasureCodePlugin *plugin = 0; { - Mutex::Locker l(instance.lock); + std::lock_guard l{instance.lock}; EXPECT_EQ(-EEXIST, instance.load("example", directory, &plugin, &cerr)); EXPECT_EQ(-ENOENT, instance.remove("does not exist")); EXPECT_EQ(0, instance.remove("example")); diff --git a/src/test/erasure-code/TestErasureCodeShec_thread.cc b/src/test/erasure-code/TestErasureCodeShec_thread.cc index 4a4997912d4..c3e6d8eb308 100644 --- a/src/test/erasure-code/TestErasureCodeShec_thread.cc +++ b/src/test/erasure-code/TestErasureCodeShec_thread.cc @@ -94,7 +94,7 @@ void* thread1(void* pParam) instance.disable_dlclose = true; { - Mutex::Locker l(instance.lock); + std::lock_guard l{instance.lock}; __erasure_code_init((char*) "shec", (char*) ""); } std::cout << "__erasure_code_init finish " << std::endl; diff --git a/src/test/erasure-code/ceph_erasure_code.cc b/src/test/erasure-code/ceph_erasure_code.cc index 19eef327cb4..1f2fb61e6d7 100644 --- a/src/test/erasure-code/ceph_erasure_code.cc +++ b/src/test/erasure-code/ceph_erasure_code.cc @@ -125,7 +125,7 @@ int ErasureCodeCommand::run() { int ErasureCodeCommand::plugin_exists() { ErasureCodePluginRegistry &instance = ErasureCodePluginRegistry::instance(); ErasureCodePlugin *plugin = 0; - Mutex::Locker l(instance.lock); + std::lock_guard l{instance.lock}; stringstream ss; int code = instance.load(vm["plugin_exists"].as(), g_conf().get_val("erasure_code_dir"), &plugin, &ss); -- 2.39.5