From cb535910f1d25526307fb179a53c1d1def98e72f Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@redhat.com>
Date: Mon, 4 Jan 2016 11:11:53 -0500
Subject: [PATCH] os/bluestore: send all old bluefs IOContexts for reap by bdev

Doing it in the aio thread ensures that there isn't a use-after-free.
Note that this is only bluefs written files.

Signed-off-by: Sage Weil <sage@redhat.com>
---
 src/os/bluestore/BlockDevice.cc | 18 ++++++++++++++++++
 src/os/bluestore/BlockDevice.h  |  6 ++++++
 src/os/bluestore/BlueFS.cc      | 18 ++----------------
 src/os/bluestore/BlueFS.h       |  2 --
 4 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/src/os/bluestore/BlockDevice.cc b/src/os/bluestore/BlockDevice.cc
index 073a02cdc4799..1d7656b49ee12 100644
--- a/src/os/bluestore/BlockDevice.cc
+++ b/src/os/bluestore/BlockDevice.cc
@@ -45,6 +45,7 @@ BlockDevice::BlockDevice(aio_callback_t cb, void *cbpriv)
     size(0), block_size(0),
     fs(NULL), aio(false), dio(false),
     debug_lock("BlockDevice::debug_lock"),
+    ioc_reap_lock("BlockDevice::ioc_reap_lock"),
     aio_queue(g_conf->bdev_aio_max_queue_depth),
     aio_callback(cb),
     aio_callback_priv(cbpriv),
@@ -247,6 +248,15 @@ void BlockDevice::_aio_thread()
 	}
       }
     }
+    if (ioc_reap_count.read()) {
+      Mutex::Locker l(ioc_reap_lock);
+      for (auto p : ioc_reap_queue) {
+	dout(20) << __func__ << " reap ioc " << p << dendl;
+	delete p;
+      }
+      ioc_reap_queue.clear();
+      ioc_reap_count.dec();
+    }
   }
   dout(10) << __func__ << " end" << dendl;
 }
@@ -477,3 +487,11 @@ int BlockDevice::invalidate_cache(uint64_t off, uint64_t len)
   }
   return r;
 }
+
+void BlockDevice::queue_reap_ioc(IOContext *ioc)
+{
+  Mutex::Locker l(ioc_reap_lock);
+  if (ioc_reap_count.read() == 0)
+    ioc_reap_count.inc();
+  ioc_reap_queue.push_back(ioc);
+}
diff --git a/src/os/bluestore/BlockDevice.h b/src/os/bluestore/BlockDevice.h
index 83c55b50bee01..187a7f57fe59e 100644
--- a/src/os/bluestore/BlockDevice.h
+++ b/src/os/bluestore/BlockDevice.h
@@ -55,6 +55,10 @@ private:
   Mutex debug_lock;
   interval_set<uint64_t> debug_inflight;
 
+  Mutex ioc_reap_lock;
+  vector<IOContext*> ioc_reap_queue;
+  atomic_t ioc_reap_count;
+
   FS::aio_queue_t aio_queue;
   aio_callback_t aio_callback;
   void *aio_callback_priv;
@@ -101,6 +105,8 @@ public:
 	       IOContext *ioc);
   int flush();
 
+  void queue_reap_ioc(IOContext *ioc);
+
   // for managing buffered readers/writers
   int invalidate_cache(uint64_t off, uint64_t len);
   int open(string path);
diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 159fd46a921ab..2d24c283d49e2 100644
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -256,11 +256,6 @@ void BlueFS::umount()
 
   _close_writer(log_writer);
   log_writer = NULL;
-  // manually clean up it's iocs
-  for (auto p : ioc_reap_queue) {
-    delete p;
-  }
-  ioc_reap_queue.clear();
 
   block_all.clear();
   _stop_alloc();
@@ -1147,8 +1142,6 @@ void BlueFS::sync_metadata()
   }
   dout(10) << __func__ << dendl;
   utime_t start = ceph_clock_now(NULL);
-  vector<IOContext*> iocv;
-  iocv.swap(ioc_reap_queue);
   for (auto p : alloc) {
     p->commit_start();
   }
@@ -1156,9 +1149,6 @@ void BlueFS::sync_metadata()
   for (auto p : alloc) {
     p->commit_finish();
   }
-  for (auto p : iocv) {
-    delete p;
-  }
   utime_t end = ceph_clock_now(NULL);
   utime_t dur = end - start;
   dout(10) << __func__ << " done in " << dur << dendl;
@@ -1244,12 +1234,8 @@ int BlueFS::open_for_write(
 void BlueFS::_close_writer(FileWriter *h)
 {
   dout(10) << __func__ << " " << h << dendl;
-  for (auto i : h->iocv) {
-    if (i->has_aios()) {
-      ioc_reap_queue.push_back(i);
-    } else {
-      delete i;
-    }
+  for (unsigned i=0; i<bdev.size(); ++i) {
+    bdev[i]->queue_reap_ioc(h->iocv[i]);
   }
   h->iocv.clear();
   delete h;
diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h
index 11d101c3e9c07..2ce5ec331b853 100644
--- a/src/os/bluestore/BlueFS.h
+++ b/src/os/bluestore/BlueFS.h
@@ -180,8 +180,6 @@ private:
   vector<interval_set<uint64_t> > block_all;  ///< extents in bdev we own
   vector<Allocator*> alloc;                   ///< allocators for bdevs
 
-  vector<IOContext*> ioc_reap_queue;          ///< iocs from closed writers
-
   void _init_alloc();
   void _stop_alloc();
 
-- 
2.39.5