From: Sage Weil <sage@inktank.com>
Date: Fri, 6 Jul 2012 02:12:22 +0000 (-0700)
Subject: cond: assert that we are holding the same mutex as the waiter
X-Git-Tag: v0.50~107^2~5
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=8de0c227e28319b2d31d15e6e34ad6b82c1fc186;p=ceph.git

cond: assert that we are holding the same mutex as the waiter

Try to verify that we are holding the same mutex that the waiter is
waiting on.  Specifically:

 * only wait on a single mutex for this cond
 * remember which mutex that is
 * if we signal and someone has waited, try to make sure we are holding
   the mutex as well.  (Mutex::is_locked() is unsufficient here; it doesn't
   ensure that *our* thread tool the mutex.  it is necessary, though!)

Introduce a sloppy_signal() method that can be used if we actually mean
to signal the cond without holding the proper lock (and, presumably,
don't care about losing a signal).

Signed-off-by: Sage Weil <sage@inktank.com>
---

diff --git a/src/common/Cond.h b/src/common/Cond.h
index 70441f87c2ec..5cd0267a2234 100644
--- a/src/common/Cond.h
+++ b/src/common/Cond.h
@@ -29,12 +29,14 @@ class Cond {
   // my bits
   pthread_cond_t _c;
 
+  Mutex *waiter_mutex;
+
   // don't allow copying.
   void operator=(Cond &C) {}
   Cond( const Cond &C ) {}
 
  public:
-  Cond() {
+  Cond() : waiter_mutex(NULL) {
     int r = pthread_cond_init(&_c,NULL);
     assert(r == 0);
   }
@@ -44,6 +46,11 @@ class Cond {
 
   int Wait(Mutex &mutex)  { 
     assert(mutex.is_locked());
+
+    // make sure this cond is used with one mutex only
+    assert(waiter_mutex == NULL || waiter_mutex == &mutex);
+    waiter_mutex = &mutex;
+
     --mutex.nlock;
     int r = pthread_cond_wait(&_c, &mutex._m);
     ++mutex.nlock;
@@ -53,6 +60,11 @@ class Cond {
   int Wait(Mutex &mutex, char* s)  { 
     //cout << "Wait: " << s << endl;
     assert(mutex.is_locked());
+
+    // make sure this cond is used with one mutex only
+    assert(waiter_mutex == NULL || waiter_mutex == &mutex);
+    waiter_mutex = &mutex;
+
     --mutex.nlock;
     int r = pthread_cond_wait(&_c, &mutex._m);
     ++mutex.nlock;
@@ -61,6 +73,11 @@ class Cond {
 
   int WaitUntil(Mutex &mutex, utime_t when) {
     assert(mutex.is_locked());
+
+    // make sure this cond is used with one mutex only
+    assert(waiter_mutex == NULL || waiter_mutex == &mutex);
+    waiter_mutex = &mutex;
+
     struct timespec ts;
     when.to_timespec(&ts);
     --mutex.nlock;
@@ -74,16 +91,32 @@ class Cond {
     return WaitUntil(mutex, when);
   }
 
+  int SloppySignal() { 
+    int r = pthread_cond_broadcast(&_c);
+    return r;
+  }
   int Signal() { 
+    // make sure signaler is holding the waiter's lock.
+    assert(waiter_mutex == NULL ||
+	   waiter_mutex->is_locked());
+
     //int r = pthread_cond_signal(&_c);
     int r = pthread_cond_broadcast(&_c);
     return r;
   }
   int SignalOne() { 
+    // make sure signaler is holding the waiter's lock.
+    assert(waiter_mutex == NULL ||
+	   waiter_mutex->is_locked());
+
     int r = pthread_cond_signal(&_c);
     return r;
   }
   int SignalAll() { 
+    // make sure signaler is holding the waiter's lock.
+    assert(waiter_mutex == NULL ||
+	   waiter_mutex->is_locked());
+
     //int r = pthread_cond_signal(&_c);
     int r = pthread_cond_broadcast(&_c);
     return r;
diff --git a/src/os/FileJournal.cc b/src/os/FileJournal.cc
index 2a832d74da30..3f3db148e718 100644
--- a/src/os/FileJournal.cc
+++ b/src/os/FileJournal.cc
@@ -734,7 +734,7 @@ int FileJournal::check_for_full(uint64_t seq, off64_t pos, off64_t size)
     if (room < (header.max_size >> 1) &&
 	room + size > (header.max_size >> 1)) {
       dout(10) << " passing half full mark, triggering commit" << dendl;
-      do_sync_cond->Signal();  // initiate a real commit so we can trim
+      do_sync_cond->SloppySignal();  // initiate a real commit so we can trim
     }
   }