From d76789444cc09df822a9b83097456e63cb0e030c Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@redhat.com>
Date: Thu, 25 Oct 2018 14:24:02 -0500
Subject: [PATCH] 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>
---
 src/osd/OSD.cc | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 0c07b1b2945..5479b598dc6 100644
--- 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();
-- 
2.39.5