osd: fix mutual exclusion for _dispatch

We want only one thread dispatching messages (either new or requeued), so
that we can preserve ordering.  Previously we weren't doing so for all
callers of do_waiters (tick() and the first in ms_dispatch()).

This fixes osd_sub_op(_reply) ordering problems that trigger the
now-famous repop queue assert.

Signed-off-by: Sage Weil <sage@newdream.net>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 95e09df931e7dbd15c9ecd363dc4e973617f3988..925ac42c1432d16c8d7f246577aefadeec6e0c65 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -1569,8 +1569,12 @@ void OSD::tick()
   // only do waiters if dispatch() isn't currently running.  (if it is,
   // it'll do the waiters, and doing them here may screw up ordering
   // of op_queue vs handle_osd_map.)
-  if (!dispatch_running)
+  if (!dispatch_running) {
+    dispatch_running = true;
     do_waiters();
+    dispatch_running = false;
+    dispatch_cond.Signal();
+  }
 }
 
 // =========================================
@@ -2048,11 +2052,21 @@ bool OSD::ms_dispatch(Message *m)
 {
   // lock!
   osd_lock.Lock();
+  while (dispatch_running) {
+    dout(10) << "ms_dispatch waiting for other dispatch thread to complete" << dendl;
+    dispatch_cond.Wait(osd_lock);
+  }
+  dispatch_running = true;
+
   do_waiters();
-  ++dispatch_running;
   _dispatch(m);
-  --dispatch_running;
   do_waiters();
+
+  dispatch_running = false;
+  
+  // no need to signal here, since tick() doesn't wait.
+  //dispatch_cond.Signal();
+
   osd_lock.Unlock();
   return true;
 }

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index 5e9e16b3f0aa2e79d00392f000be9d33551338a4..429d02f8c3e34767edbd0802cb5bda0391064a9a 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -132,6 +132,7 @@ protected:
     }
   };
 
+  Cond dispatch_cond;
   int dispatch_running;
 
   void open_logger();