From c3f70dc4a86efb807d891de33b1f07e829478434 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 14 Sep 2018 17:19:12 -0500 Subject: [PATCH] common/ceph_mutex: ceph::{mutex,condition_variable,lock_guard} If CEPH_DEBUG_MUTEX is defined, use the [recursive_]mutex_debug classes that implement lockdep and a bucnh of other random debug checks. Also typedef ceph::condition_variable to std::condition_variable_debug, which adds addition assertions and debug checks. If CEPH_DEBUG_MUTEX is not defined, then use the bare-bones C++ std::mutex primitives... or as close as we can get to them. Since the [recursive_]mutex_debug classes take a string argument for the lockdep piece, define factory functions ceph::make_[recursive_]mutex that either pass arguments to the debug implementations or toss them out. Signed-off-by: Sage Weil --- CMakeLists.txt | 2 + src/CMakeLists.txt | 4 + src/common/ceph_mutex.h | 81 +++++++++++++++++++ src/test/common/CMakeLists.txt | 7 ++ src/test/common/test_lockdep.cc | 69 ++++++++++++++++ .../librados_test_stub/TestWatchNotify.cc | 1 + 6 files changed, 164 insertions(+) create mode 100644 src/common/ceph_mutex.h create mode 100644 src/test/common/test_lockdep.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 27f8d0ce4ac03..65483f8690650 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -322,6 +322,8 @@ if(WITH_LZ4) set(HAVE_LZ4 ${LZ4_FOUND}) endif(WITH_LZ4) +option(WITH_CEPH_DEBUG_MUTEX "Use debug ceph::mutex with lockdep" OFF) + #if allocator is set on command line make sure it matches below strings set(ALLOCATOR "" CACHE STRING "specify memory allocator to use. currently tcmalloc, tcmalloc_minimal, \ diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index aa42a311b49d0..874eabfaa13a2 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -66,6 +66,10 @@ if(NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "Default BUILD_TYPE is RelWithDebInfo, other options are: Debug, Release, and MinSizeRel." FORCE) endif() +if(WITH_CEPH_DEBUG_MUTEX OR CMAKE_BUILD_TYPE STREQUAL Debug) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DCEPH_DEBUG_MUTEX") +endif() + include(CheckCCompilerFlag) if(CMAKE_CXX_COMPILER_ID STREQUAL GNU) CHECK_C_COMPILER_FLAG("-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2" HAS_FORTIFY_SOURCE) diff --git a/src/common/ceph_mutex.h b/src/common/ceph_mutex.h new file mode 100644 index 0000000000000..3c105ef7a6229 --- /dev/null +++ b/src/common/ceph_mutex.h @@ -0,0 +1,81 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#pragma once + +// What and why +// ============ +// +// For general code making use of mutexes, use these ceph:: types. +// The key requirement is that you make use of the ceph::make_mutex() +// and make_recursive_mutex() factory methods, which take a string +// naming the mutex for the purposes of the lockdep debug variant. +// +// For legacy Mutex users that passed recursive=true, use +// ceph::make_recursive_mutex. For legacy Mutex users that passed +// lockdep=false, use std::mutex directly. + +#ifdef CEPH_DEBUG_MUTEX + +// ============================================================================ +// debug (lockdep-capable, various sanity checks and asserts) +// ============================================================================ + +#include "common/mutex_debug.h" +#include "common/condition_variable_debug.h" + +namespace ceph { + typedef ceph::mutex_debug mutex; + typedef ceph::mutex_recursive_debug recursive_mutex; + typedef ceph::condition_variable_debug condition_variable; + + // pass arguments to mutex_debug ctor + template + mutex make_mutex(Args&& ...args) { + return {std::forward(args)...}; + } + + // pass arguments to recursive_mutex_debug ctor + template + recursive_mutex make_recursive_mutex(Args&& ...args) { + return {std::forward(args)...}; + } + + // debug methods + #define ceph_mutex_is_locked(m) ((m).is_locked()) + #define ceph_mutex_is_locked_by_me(m) ((m).is_locked_by_me()) +} + +#else + +// ============================================================================ +// release (fast and minimal) +// ============================================================================ + +#include +#include + +namespace ceph { + + typedef std::mutex mutex; + typedef std::recursive_mutex recursive_mutex; + typedef std::condition_variable condition_variable; + + // discard arguments to make_mutex (they are for debugging only) + template + std::mutex make_mutex(Args&& ...args) { + return {}; + } + template + std::recursive_mutex make_recursive_mutex(Args&& ...args) { + return {}; + } + + // debug methods. Note that these can blindly return true + // because any code that does anything other than assert these + // are true is broken. + #define ceph_mutex_is_locked(m) true + #define ceph_mutex_is_locked_by_me(m) true +} + +#endif diff --git a/src/test/common/CMakeLists.txt b/src/test/common/CMakeLists.txt index c793d1a6bb8ab..5692c3de0006d 100644 --- a/src/test/common/CMakeLists.txt +++ b/src/test/common/CMakeLists.txt @@ -21,6 +21,13 @@ if(HAVE_BLKID) target_link_libraries(unittest_blkdev ceph-common ${BLKID_LIBRARIES}) endif() +# unittest_lockdep +add_executable(unittest_lockdep + test_lockdep.cc + ) +add_ceph_unittest(unittest_lockdep) +target_link_libraries(unittest_lockdep ceph-common global) + # unittest_bloom_filter add_executable(unittest_bloom_filter test_bloom_filter.cc diff --git a/src/test/common/test_lockdep.cc b/src/test/common/test_lockdep.cc new file mode 100644 index 0000000000000..054001235141a --- /dev/null +++ b/src/test/common/test_lockdep.cc @@ -0,0 +1,69 @@ +// -*- mode:C; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "common/ceph_context.h" +#include "include/util.h" +#include "gtest/gtest.h" +#include "common/ceph_mutex.h" +#include "global/global_context.h" +#include "global/global_init.h" + +TEST(lockdep, abba) +{ + ASSERT_TRUE(g_lockdep); + + ceph::mutex a(ceph::make_mutex("a")), b(ceph::make_mutex("b")); + a.lock(); + ASSERT_TRUE(ceph_mutex_is_locked(a)); + ASSERT_TRUE(ceph_mutex_is_locked_by_me(a)); + b.lock(); + ASSERT_TRUE(ceph_mutex_is_locked(b)); + ASSERT_TRUE(ceph_mutex_is_locked_by_me(b)); + a.unlock(); + b.unlock(); + + b.lock(); + EXPECT_DEATH(a.lock(), ""); + b.unlock(); +} + +TEST(lockdep, recursive) +{ + ASSERT_TRUE(g_lockdep); + + ceph::mutex a(ceph::make_mutex("a")); + a.lock(); + EXPECT_DEATH(a.lock(), ""); + a.unlock(); + + ceph::recursive_mutex b(ceph::make_recursive_mutex("b")); + b.lock(); + ASSERT_TRUE(ceph_mutex_is_locked(b)); + ASSERT_TRUE(ceph_mutex_is_locked_by_me(b)); + b.lock(); + b.unlock(); + b.unlock(); +} + +int main(int argc, char **argv) { +#ifdef NDEBUG + cout << "NDEBUG is defined" << std::endl; +#else + cout << "NDEBUG is NOT defined" << std::endl; +#endif +#ifndef CEPH_DEBUG_MUTEX + cerr << "WARNING: CEPH_DEBUG_MUTEX is not defined, lockdep will not work" + << std::endl; + exit(0); +#endif + std::vector args(argv, argv + argc); + args.push_back("--lockdep"); + auto cct = global_init(NULL, args, + CEPH_ENTITY_TYPE_CLIENT, + CODE_ENVIRONMENT_UTILITY, + CINIT_FLAG_NO_MON_CONFIG); + common_init_finish(g_ceph_context); + + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/src/test/librados_test_stub/TestWatchNotify.cc b/src/test/librados_test_stub/TestWatchNotify.cc index 85fa9393a3b35..88cfcbe316dae 100644 --- a/src/test/librados_test_stub/TestWatchNotify.cc +++ b/src/test/librados_test_stub/TestWatchNotify.cc @@ -3,6 +3,7 @@ #include "test/librados_test_stub/TestWatchNotify.h" #include "include/Context.h" +#include "common/Cond.h" #include "include/stringify.h" #include "common/Finisher.h" #include "test/librados_test_stub/TestCluster.h" -- 2.39.5