From 48c542ac1c18da65f6efa0666692c7cbc0e19113 Mon Sep 17 00:00:00 2001 From: "Adam C. Emerson" Date: Mon, 14 Sep 2015 13:30:38 -0400 Subject: [PATCH] concurrency: make C++11 style debugging mutices Signed-off-by: Adam C. Emerson --- src/CMakeLists.txt | 1 + src/common/Makefile.am | 4 +- src/common/mutex_debug.cc | 97 ++++++++++++++ src/common/mutex_debug.h | 191 ++++++++++++++++++++++++++++ src/include/uuid.h | 6 +- src/test/CMakeLists.txt | 13 ++ src/test/Makefile.am | 5 + src/test/common/test_mutex_debug.cc | 101 +++++++++++++++ 8 files changed, 416 insertions(+), 2 deletions(-) create mode 100644 src/common/mutex_debug.cc create mode 100644 src/common/mutex_debug.h create mode 100644 src/test/common/test_mutex_debug.cc diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 619519c2c599a..42714b458c0d0 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -215,6 +215,7 @@ set(libcommon_files common/PrebufferedStreambuf.cc common/BackTrace.cc common/perf_counters.cc + common/mutex_debug.cc common/Mutex.cc common/OutputDataSocket.cc common/admin_socket.cc diff --git a/src/common/Makefile.am b/src/common/Makefile.am index 964a3395867b6..f86d52327fc87 100644 --- a/src/common/Makefile.am +++ b/src/common/Makefile.am @@ -8,6 +8,7 @@ libcommon_internal_la_SOURCES = \ common/SloppyCRCMap.cc \ common/BackTrace.cc \ common/perf_counters.cc \ + common/mutex_debug.cc \ common/Mutex.cc \ common/OutputDataSocket.cc \ common/admin_socket.cc \ @@ -275,7 +276,8 @@ noinst_HEADERS += \ common/PluginRegistry.h \ common/ceph_time.h \ common/ceph_timer.h \ - common/align.h + common/align.h \ + common/mutex_debug.h if ENABLE_XIO noinst_HEADERS += \ diff --git a/src/common/mutex_debug.cc b/src/common/mutex_debug.cc new file mode 100644 index 0000000000000..a854358617116 --- /dev/null +++ b/src/common/mutex_debug.cc @@ -0,0 +1,97 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab +/* + * Ceph - scalable distributed file system + * + * Copyright (C) 2004-2006 Sage Weil + * + * This is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License version 2.1, as published by the Free Software + * Foundation. See file COPYING. + * + */ +#include + +#include +#include +#include + +#include "common/mutex_debug.h" +#include "common/perf_counters.h" +#include "common/ceph_context.h" +#include "common/config.h" +#include "include/stringify.h" + +namespace ceph { +namespace mutex_debug_detail { +enum { + l_mutex_first = 999082, + l_mutex_wait, + l_mutex_last +}; + +mutex_debugging_base::mutex_debugging_base(const std::string &n, bool bt, + CephContext *cct) : + id(-1), backtrace(bt), nlock(0), locked_by(thread::id()), + cct(cct), logger(0) { + if (n.empty()) { + uuid_d uu; + uu.generate_random(); + name = string("Unnamed-Mutex-") + uu.to_string(); + } else { + name = n; + } + if (cct) { + PerfCountersBuilder b(cct, string("mutex-") + name, + l_mutex_first, l_mutex_last); + b.add_time_avg(l_mutex_wait, "wait", + "Average time of mutex in locked state"); + logger = b.create_perf_counters(); + cct->get_perfcounters_collection()->add(logger); + logger->set(l_mutex_wait, 0); + } + if (g_lockdep) + _register(); +} + +mutex_debugging_base::~mutex_debugging_base() { + assert(nlock == 0); + if (cct && logger) { + cct->get_perfcounters_collection()->remove(logger); + delete logger; + } + if (g_lockdep) { + lockdep_unregister(id); + } +} + +void mutex_debugging_base::_register() { + id = lockdep_register(name.c_str()); +} +void mutex_debugging_base::_will_lock() { // about to lock + id = lockdep_will_lock(name.c_str(), id, backtrace); +} +void mutex_debugging_base::_locked() { // just locked + id = lockdep_locked(name.c_str(), id, backtrace); +} +void mutex_debugging_base::_will_unlock() { // about to unlock + id = lockdep_will_unlock(name.c_str(), id); +} + +ceph::mono_time mutex_debugging_base::before_lock_blocks() { + if (logger && cct && cct->_conf->mutex_perf_counter) + return ceph::mono_clock::now(); + return ceph::mono_time::min(); +} + +void mutex_debugging_base::after_lock_blocks(ceph::mono_time start, + bool no_lockdep) { + if (logger && cct && cct->_conf->mutex_perf_counter) + logger->tinc(l_mutex_wait, + ceph::mono_clock::now() - start); + if (!no_lockdep && g_lockdep) + _locked(); +} +} // namespace mutex_debug_detail +} // namespace ceph diff --git a/src/common/mutex_debug.h b/src/common/mutex_debug.h new file mode 100644 index 0000000000000..c92a88f4afaa0 --- /dev/null +++ b/src/common/mutex_debug.h @@ -0,0 +1,191 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab +/* + * Ceph - scalable distributed file system + * + * Copyright (C) 2004-2006 Sage Weil + * + * This is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License version 2.1, as published by the Free Software + * Foundation. See file COPYING. + * + */ + +#ifndef CEPH_COMMON_MUTEX_DEBUG_H +#define CEPH_COMMON_MUTEX_DEBUG_H + +#include +#include + +#include + +#include "include/assert.h" + +#include "ceph_time.h" +#include "likely.h" +#include "lockdep.h" + +class CephContext; +class PerfCounters; + +namespace ceph { +namespace mutex_debug_detail { +class mutex_debugging_base { +protected: + std::string name; + int id; + bool backtrace; // gather backtrace on lock acquisition + + int nlock; + std::thread::id locked_by; + CephContext *cct; + PerfCounters *logger; + + void _register(); + void _will_lock(); // about to lock + void _locked(); // just locked + void _will_unlock(); // about to unlock + + mutex_debugging_base(const std::string &n = std::string(), bool bt = false, + CephContext *cct = nullptr); + ~mutex_debugging_base(); + + ceph::mono_time before_lock_blocks(); + void after_lock_blocks(ceph::mono_time start, + bool no_lockdep); + +public: + bool is_locked() const { + return (nlock > 0); + } + bool is_locked_by_me() const { + return nlock > 0 && locked_by == std::this_thread::get_id(); + } + operator bool() const { + return nlock > 0 && locked_by == std::this_thread::get_id(); + } +}; + +template +class mutex_debugging : public mutex_debugging_base { + Mutex* impl; + +public: + mutex_debugging(const std::string &n = std::string(), bool bt = false, + CephContext *cct = nullptr) : + mutex_debugging_base(n, bt, cct), impl(static_cast(this)) {} + + ~mutex_debugging() = default; + + void _post_lock() { + if (!impl->recursive) + assert(nlock == 0); + locked_by = std::this_thread::get_id(); + nlock++; + } + + void _pre_unlock() { + assert(nlock > 0); + --nlock; + assert(locked_by == std::this_thread::get_id()); + if (!impl->recursive) + assert(nlock == 0); + if (nlock == 0) + locked_by = std::thread::id(); + } + + bool try_lock(bool no_lockdep = false) { + bool locked = impl->try_lock_impl(); + if (locked) { + if (g_lockdep && !no_lockdep) + _locked(); + _post_lock(); + } + return locked; + } + + void lock(bool no_lockdep = false) { + if (g_lockdep && !no_lockdep) + _will_lock(); + + if (try_lock()) + return; + + auto t = before_lock_blocks(); + impl->lock_impl(); + after_lock_blocks(t, no_lockdep); + _post_lock(); + } + + void unlock(bool no_lockdep = false) { + _pre_unlock(); + if (!no_lockdep && g_lockdep) + _will_unlock(); + impl->unlock_impl(); + } +}; + +// Since this is a /debugging/ mutex just define it in terms of the +// pthread error check mutex. +template +class mutex_debug_impl : public mutex_debugging > { +private: + pthread_mutex_t m; +public: + static constexpr bool recursive = Recursive; + + // Mutex concept is DefaultConstructible + mutex_debug_impl(const std::string &n = std::string(), bool bt = false, + CephContext *cct = nullptr) : + mutex_debugging >(n, bt, cct) { + if (recursive) + m = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP; + else + m = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP; + } + // Mutex is Destructible + ~mutex_debug_impl() = default; + + // Mutex concept is non-Copyable + mutex_debug_impl(const mutex_debug_impl&) = delete; + mutex_debug_impl& operator =(const mutex_debug_impl&) = delete; + + // Mutex concept is non-Movable + mutex_debug_impl(mutex_debug_impl&&) = delete; + mutex_debug_impl& operator =(mutex_debug_impl&&) = delete; + + void lock_impl() { + int r = pthread_mutex_lock(&m); + // Allowed error codes for Mutex concept + if (unlikely(r == EPERM || + r == EDEADLK || + r == EBUSY)) { + throw std::system_error(r, std::generic_category()); + } + assert(r == 0); + } + + void unlock_impl() noexcept { + int r = pthread_mutex_unlock(&m); + assert(r == 0); + } + + bool try_lock_impl() { + int r = pthread_mutex_trylock(&m); + switch (r) { + case 0: + return true; + case EBUSY: + return false; + default: + throw std::system_error(r, std::generic_category()); + } + } +}; +} // namespace mutex_debug_detail +typedef mutex_debug_detail::mutex_debug_impl mutex_debug; +typedef mutex_debug_detail::mutex_debug_impl mutex_recursive_debug; +} // namespace ceph + +#endif diff --git a/src/include/uuid.h b/src/include/uuid.h index 03e6b5ad32c79..bd888f863da10 100644 --- a/src/include/uuid.h +++ b/src/include/uuid.h @@ -44,10 +44,14 @@ struct uuid_d { memcpy(s, boost::uuids::to_string(uuid).c_str(), 37); } + std::string to_string() const { + return boost::uuids::to_string(uuid); + } + char *bytes() const { return (char*)uuid.data; } - + void encode(bufferlist& bl) const { ::encode_raw(uuid, bl); } diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 114e97764c151..4f1fec6e2ae36 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -544,6 +544,19 @@ target_link_libraries(unittest_str_map common global set_target_properties(unittest_str_map PROPERTIES COMPILE_FLAGS ${UNITTEST_CXX_FLAGS}) +# unittest_shared_mutex +set(unittest_mutex_debug_srcs + common/test_mutex_debug.cc + ) +add_executable(unittest_mutex_debug + ${unittest_mutex_debug_srcs} + $ + ) +target_link_libraries(unittest_mutex_debug global + ${BLKID_LIBRARIES} ${CMAKE_DL_LIBS} ${TCMALLOC_LIBS} ${UNITTEST_LIBS} ${EXTRALIBS}) +set_target_properties(unittest_mutex_debug + PROPERTIES COMPILE_FLAGS ${UNITTEST_CXX_FLAGS}) + # unittest_sharedptr_registry add_executable(unittest_sharedptr_registry EXCLUDE_FROM_ALL common/test_sharedptr_registry.cc diff --git a/src/test/Makefile.am b/src/test/Makefile.am index 2bcb8def33236..873c69ff78391 100644 --- a/src/test/Makefile.am +++ b/src/test/Makefile.am @@ -178,6 +178,11 @@ unittest_str_map_CXXFLAGS = $(UNITTEST_CXXFLAGS) unittest_str_map_LDADD = $(UNITTEST_LDADD) $(CEPH_GLOBAL) check_TESTPROGRAMS += unittest_str_map +unittest_mutex_debug_SOURCES = test/common/test_mutex_debug.cc +unittest_mutex_debug_CXXFLAGS = $(UNITTEST_CXXFLAGS) +unittest_mutex_debug_LDADD = $(UNITTEST_LDADD) $(CEPH_GLOBAL) ${EXTRALIBS} +check_TESTPROGRAMS += unittest_mutex_debug + unittest_sharedptr_registry_SOURCES = test/common/test_sharedptr_registry.cc unittest_sharedptr_registry_CXXFLAGS = $(UNITTEST_CXXFLAGS) unittest_sharedptr_registry_LDADD = $(UNITTEST_LDADD) $(CEPH_GLOBAL) diff --git a/src/test/common/test_mutex_debug.cc b/src/test/common/test_mutex_debug.cc new file mode 100644 index 0000000000000..49cd499557f0d --- /dev/null +++ b/src/test/common/test_mutex_debug.cc @@ -0,0 +1,101 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 &smarttab +/* + * Ceph - scalable distributed file system + * + * Copyright (C) 2011 New Dream Network + * + * This is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License version 2, as published by the Free Software + * Foundation. See file COPYING. + * + */ + +#include +#include +#include + +#include "common/mutex_debug.h" + +#include "gtest/gtest.h" + + +template +static bool test_try_lock(Mutex* m) { + if (!m->try_lock()) + return false; + m->unlock(); + return true; +} + +template +static void test_lock() { + Mutex m; + auto ttl = &test_try_lock; + + m.lock(); + ASSERT_TRUE(m.is_locked()); + auto f1 = std::async(std::launch::async, ttl, &m); + ASSERT_FALSE(f1.get()); + + ASSERT_TRUE(m.is_locked()); + ASSERT_TRUE(!!m); + + m.unlock(); + ASSERT_FALSE(m.is_locked()); + ASSERT_FALSE(!!m); + + auto f3 = std::async(std::launch::async, ttl, &m); + ASSERT_TRUE(f3.get()); + + ASSERT_FALSE(m.is_locked()); + ASSERT_FALSE(!!m); +} + +TEST(MutexDebug, Lock) { + test_lock(); +} + +TEST(MutexDebug, NotRecursive) { + ceph::mutex_debug m; + auto ttl = &test_try_lock; + + ASSERT_NO_THROW(m.lock()); + ASSERT_TRUE(m.is_locked()); + ASSERT_FALSE(std::async(std::launch::async, ttl, &m).get()); + + ASSERT_THROW(m.lock(), std::system_error); + ASSERT_TRUE(m.is_locked()); + ASSERT_FALSE(std::async(std::launch::async, ttl, &m).get()); + + ASSERT_NO_THROW(m.unlock()); + ASSERT_FALSE(m.is_locked()); + ASSERT_TRUE(std::async(std::launch::async, ttl, &m).get()); +} + +TEST(MutexRecursiveDebug, Lock) { + test_lock(); +} + + +TEST(MutexRecursiveDebug, Recursive) { + ceph::mutex_recursive_debug m; + auto ttl = &test_try_lock; + + ASSERT_NO_THROW(m.lock()); + ASSERT_TRUE(m.is_locked()); + ASSERT_FALSE(std::async(std::launch::async, ttl, &m).get()); + + ASSERT_NO_THROW(m.lock()); + ASSERT_TRUE(m.is_locked()); + ASSERT_FALSE(std::async(std::launch::async, ttl, &m).get()); + + ASSERT_NO_THROW(m.unlock()); + ASSERT_TRUE(m.is_locked()); + ASSERT_FALSE(std::async(std::launch::async, ttl, &m).get()); + + ASSERT_NO_THROW(m.unlock()); + ASSERT_FALSE(m.is_locked()); + ASSERT_TRUE(std::async(std::launch::async, ttl, &m).get()); +} -- 2.39.5