osd: fix race between op_wq and context_queue

        ThreadA                             		                   ThreadB
  sdata->shard_lock.Lock();
  if (sdata->pqueue->empty() &&
     !(is_smallest_thread_index && !sdata->context_queue.empty())) {

								    void queue(list<Context *>& ls) {
								        bool empty = false;
                                                                       {
                                                                         std::scoped_lock l(q_mutex);
                                                                         if (q.empty()) {
                                                                           q.swap(ls);
                                                                           empty = true;
                                                                         } else {
                                                                           q.insert(q.end(), ls.begin(), ls.end());
                                                                         }
                                                                       }

                                                                       if (empty) {
                                                                         mutex.Lock();
                                                                         cond.Signal();
                                                                         mutex.Unlock();
                                                                       }
                                                                    }

     sdata->sdata_wait_lock.Lock();
    if (!sdata->stop_waiting) {

Fix by simply rechecking that context_queue is empty after taking the
wait lock.  We still check it without taking that lock to keep the hot/busy
path fast (we avoid the wait lock in general) at the expense of taking
the context_queue qlock twice in the idle/wait path (where we don't care
so much about additional latency/cycles).

Fixes: http://tracker.ceph.com/issues/36473
Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 0c07b1b2945b1fa9426c50c30f0e856894142873..5479b598dc63fb3d85d5fbc07b986b0b7f56b047 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -10196,17 +10196,21 @@ void OSD::ShardedOpWQ::_process(uint32_t thread_index, heartbeat_handle_d *hb)
   auto& sdata = osd->shards[shard_index];
   ceph_assert(sdata);
 
-  // If all threads of shards do oncommits, there is a out-of-order problem.
-  // So we choose the thread which has the smallest thread_index(thread_index < num_shards) of shard
-  // to do oncommit callback.
+  // If all threads of shards do oncommits, there is a out-of-order
+  // problem.  So we choose the thread which has the smallest
+  // thread_index(thread_index < num_shards) of shard to do oncommit
+  // callback.
   bool is_smallest_thread_index = thread_index < osd->num_shards;
 
   // peek at spg_t
   sdata->shard_lock.Lock();
   if (sdata->pqueue->empty() &&
-     !(is_smallest_thread_index && !sdata->context_queue.empty())) {
+      (!is_smallest_thread_index || sdata->context_queue.empty())) {
     sdata->sdata_wait_lock.Lock();
-    if (!sdata->stop_waiting) {
+    if (is_smallest_thread_index && !sdata->context_queue.empty()) {
+      // we raced with a context_queue addition, don't wait
+      sdata->sdata_wait_lock.Unlock();
+    } else if (!sdata->stop_waiting) {
       dout(20) << __func__ << " empty q, waiting" << dendl;
       osd->cct->get_heartbeat_map()->clear_timeout(hb);
       sdata->shard_lock.Unlock();