From 98226c2226e6ea197d41eb13890e2ad1b698486c Mon Sep 17 00:00:00 2001 From: Colin Patrick McCabe Date: Wed, 1 Jun 2011 11:13:45 -0700 Subject: [PATCH] DoutStreambuf: de-globalize dout lock Signed-off-by: Colin McCabe --- src/common/DoutStreambuf.cc | 18 ++++++++++--- src/common/DoutStreambuf.h | 7 ++++++ src/common/assert.cc | 50 +++---------------------------------- src/common/ceph_context.cc | 23 ++++++++++++----- src/common/ceph_context.h | 7 ++++++ src/common/debug.h | 18 ++++++++----- 6 files changed, 62 insertions(+), 61 deletions(-) diff --git a/src/common/DoutStreambuf.cc b/src/common/DoutStreambuf.cc index 341d4a5b59b37..5aec02b8b7a2d 100644 --- a/src/common/DoutStreambuf.cc +++ b/src/common/DoutStreambuf.cc @@ -20,6 +20,7 @@ #include "common/errno.h" #include "common/safe_io.h" #include "common/simple_spin.h" +#include "common/debug.h" #include #include @@ -160,6 +161,16 @@ DoutStreambuf::DoutStreambuf() // Initialize output_buffer _clear_output_buffer(); + int ret; + pthread_mutexattr_t attr; + ret = pthread_mutexattr_init(&attr); + assert(ret == 0); + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); + ret = pthread_mutex_init(&lock, &attr); + assert(ret == 0); + ret = pthread_mutexattr_destroy(&attr); + assert(ret == 0); + simple_spin_lock(&dout_emergency_lock); for (size_t i = 0; i < NUM_DOUT_EMERG_STREAMS; ++i) { if (dout_emerg_streams[i] == 0) { @@ -177,6 +188,7 @@ DoutStreambuf::~DoutStreambuf() TEMP_FAILURE_RETRY(::close(ofd)); ofd = -1; } + pthread_mutex_destroy(&lock); simple_spin_lock(&dout_emergency_lock); for (size_t i = 0; i < NUM_DOUT_EMERG_STREAMS; ++i) { if (dout_emerg_streams[i] == this) { @@ -253,7 +265,7 @@ DoutStreambuf::overflow(DoutStreambuf::int_type c) template void DoutStreambuf::handle_stderr_shutdown() { - DoutLocker _dout_locker; + DoutLocker _dout_locker(&lock); flags &= ~DOUTSB_FLAG_STDERR; } @@ -272,7 +284,7 @@ template void DoutStreambuf:: handle_conf_change(const md_config_t *conf, const std::set &changed) { - DoutLocker _dout_locker; + DoutLocker _dout_locker(&lock); type_name = conf->name.get_type_name(); flags = 0; @@ -331,7 +343,7 @@ template int DoutStreambuf:: handle_pid_change(const md_config_t *conf) { - DoutLocker _dout_locker; + DoutLocker _dout_locker(&lock); if (!(flags & DOUTSB_FLAG_OFILE)) return 0; diff --git a/src/common/DoutStreambuf.h b/src/common/DoutStreambuf.h index b21d1e020deae..472dfefbe1ac9 100644 --- a/src/common/DoutStreambuf.h +++ b/src/common/DoutStreambuf.h @@ -23,9 +23,11 @@ #include "common/config.h" #include +#include #include class md_config_t; +class CephContext; class EmergencyLogger { public: @@ -118,6 +120,11 @@ private: std::string opath; std::string symlink_dir; std::string isym_path; + + // Mutex that protects this output stream + pthread_mutex_t lock; + + friend class CephContext; }; // Secret evil interfaces for writing logs without taking the lock. diff --git a/src/common/assert.cc b/src/common/assert.cc index fa03a7dfcfa93..1c56cdf63889d 100644 --- a/src/common/assert.cc +++ b/src/common/assert.cc @@ -23,40 +23,11 @@ #include "common/config.h" #include "include/assert.h" -static bool dout_trylock(void) -{ - time_t cur; - time_t end_time = time(NULL); - end_time += 10; - - do { - int ret = pthread_mutex_trylock(&_dout_lock); - if (ret == 0) { - // We got the mutex. - return true; - } - else if ((ret == EDEADLK) || (ret == EAGAIN)) { - // This thread already owns the mutex. - // The assertion happened inside a call to dout()? - return false; - } - else if (ret == EINVAL) { - // We can never get the mutex. - return false; - } - // Otherwise, keep trying. - usleep(1000); - cur = time(NULL); - } while (cur < end_time); - - // If 10 seconds go by and we can't get the mutex, just bail. - return false; -} - namespace ceph { void __ceph_assert_fail(const char *assertion, const char *file, int line, const char *func) { - int got_lock = dout_trylock(); + DoutLocker dout_locker; + g_ceph_context.dout_trylock(&dout_locker); char buf[8096]; BackTrace *bt = new BackTrace(1); @@ -76,32 +47,19 @@ namespace ceph { "is needed to interpret this.\n"); dout_emergency(oss.str()); - if (got_lock) { - int ret = pthread_mutex_unlock(&_dout_lock); - if (ret) { - ; // ignored - } - } - throw FailedAssertion(bt); } void __ceph_assert_warn(const char *assertion, const char *file, int line, const char *func) { - int got_lock = dout_trylock(); + DoutLocker dout_locker; + g_ceph_context.dout_trylock(&dout_locker); char buf[8096]; snprintf(buf, sizeof(buf), "WARNING: assert(%s) at: %s: %d: %s()\n", assertion, file, line, func); dout_emergency(buf); - - if (got_lock) { - int ret = pthread_mutex_unlock(&_dout_lock); - if (ret) { - ; // ignored - } - } } } diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc index d581dda8c7700..711f6b643b0a9 100644 --- a/src/common/ceph_context.cc +++ b/src/common/ceph_context.cc @@ -17,6 +17,7 @@ #include "common/Thread.h" #include "common/ceph_context.h" #include "common/config.h" +#include "common/debug.h" #include #include @@ -29,12 +30,6 @@ md_config_t &g_conf(*g_ceph_context._conf); std::ostream *_dout(&g_ceph_context._dout); DoutStreambuf ::traits_type> *_doss(g_ceph_context._doss); -/* - * The dout lock protects calls to dout() - * TODO: needs to become part of CephContext - */ -pthread_mutex_t _dout_lock = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP; - class CephContextServiceThread : public Thread { public: @@ -135,6 +130,22 @@ reopen_logs() pthread_spin_unlock(&_service_thread_lock); } +void CephContext:: +dout_lock(DoutLocker *locker) +{ + pthread_mutex_t *lock = &_doss->lock; + pthread_mutex_lock(lock); + locker->lock = lock; +} + +void CephContext:: +dout_trylock(DoutLocker *locker) +{ + pthread_mutex_t *lock = &_doss->lock; + if (pthread_mutex_trylock(lock) == 0) + locker->lock = lock; +} + void CephContext:: join_service_thread() { diff --git a/src/common/ceph_context.h b/src/common/ceph_context.h index 1d80a772655f9..ef16583e4eff9 100644 --- a/src/common/ceph_context.h +++ b/src/common/ceph_context.h @@ -24,6 +24,7 @@ class DoutStreambuf; class md_config_t; class md_config_obs_t; class CephContextServiceThread; +class DoutLocker; /* A CephContext represents the context held by a single library user. * There can be multiple CephContexts in the same process. @@ -46,6 +47,12 @@ public: /* Reopen the log files */ void reopen_logs(); + /* Lock the dout lock. */ + void dout_lock(DoutLocker *locker); + + /* Try to lock the dout lock. */ + void dout_trylock(DoutLocker *locker); + private: /* Stop and join the Ceph Context's service thread */ void join_service_thread(); diff --git a/src/common/debug.h b/src/common/debug.h index d9c241eff4c73..47f2bd56635f1 100644 --- a/src/common/debug.h +++ b/src/common/debug.h @@ -25,9 +25,6 @@ #include #include - -extern pthread_mutex_t _dout_lock; - extern void dout_emergency(const char * const str); extern void dout_emergency(const std::string &str); @@ -35,12 +32,20 @@ extern void dout_emergency(const std::string &str); class DoutLocker { public: - DoutLocker() { - pthread_mutex_lock(&_dout_lock); + DoutLocker(pthread_mutex_t *lock_) + : lock(lock_) + { + pthread_mutex_lock(lock); + } + DoutLocker() + : lock(NULL) + { } ~DoutLocker() { - pthread_mutex_unlock(&_dout_lock); + if (lock) + pthread_mutex_unlock(lock); } + pthread_mutex_t *lock; }; static inline void _dout_begin_line(signed int prio) { @@ -74,6 +79,7 @@ inline std::ostream& operator<<(std::ostream& out, _bad_endl_use_dendl_t) { char __array[((v >= -1) && (v <= 200)) ? 0 : -1] __attribute__((unused)); \ }\ DoutLocker __dout_locker; \ + g_ceph_context.dout_lock(&__dout_locker); \ _dout_begin_line(v); \ #define dout(v) \ -- 2.39.5