From: Adam Kupczyk <akupczyk@redhat.com>
Date: Thu, 11 Mar 2021 13:03:00 +0000 (+0100)
Subject: blk/BlockDevice: Remove reap_ioc logic
X-Git-Tag: v17.1.0~2584^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=4133e1b459cae9f7ca356c71c1725607567b47bc;p=ceph.git

blk/BlockDevice: Remove reap_ioc logic

queue_reap_ioc and reap_ioc logic was necessary for BlueFS's _close_writer() method.
At one point it did not perform aio_wait(), and there was race condition in access/deletion to IOContext object.
Now we can simply delete it after successfull aio_wait().

Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>
---

diff --git a/src/blk/BlockDevice.cc b/src/blk/BlockDevice.cc
index 6804ee50cbca..d5d3169e9131 100644
--- a/src/blk/BlockDevice.cc
+++ b/src/blk/BlockDevice.cc
@@ -176,27 +176,6 @@ BlockDevice *BlockDevice::create(
   return create_with_type(device_type, cct, path, cb, cbpriv, d_cb, d_cbpriv);
 }
 
-void BlockDevice::queue_reap_ioc(IOContext *ioc)
-{
-  std::lock_guard l(ioc_reap_lock);
-  if (ioc_reap_count.load() == 0)
-    ++ioc_reap_count;
-  ioc_reap_queue.push_back(ioc);
-}
-
-void BlockDevice::reap_ioc()
-{
-  if (ioc_reap_count.load()) {
-    std::lock_guard 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;
-  }
-}
-
 bool BlockDevice::is_valid_io(uint64_t off, uint64_t len) const {
   bool ret = (off % block_size == 0 &&
     len % block_size == 0 &&
diff --git a/src/blk/BlockDevice.h b/src/blk/BlockDevice.h
index 191eb8ec908a..44ed3de4d8c6 100644
--- a/src/blk/BlockDevice.h
+++ b/src/blk/BlockDevice.h
@@ -263,9 +263,6 @@ public:
   virtual int queue_discard(interval_set<uint64_t> &to_release) { return -1; }
   virtual void discard_drain() { return; }
 
-  void queue_reap_ioc(IOContext *ioc);
-  void reap_ioc();
-
   // for managing buffered readers/writers
   virtual int invalidate_cache(uint64_t off, uint64_t len) = 0;
   virtual int open(const std::string& path) = 0;
diff --git a/src/blk/kernel/KernelDevice.cc b/src/blk/kernel/KernelDevice.cc
index d41cda05b63d..09a4b687ef06 100644
--- a/src/blk/kernel/KernelDevice.cc
+++ b/src/blk/kernel/KernelDevice.cc
@@ -648,7 +648,6 @@ void KernelDevice::_aio_thread()
 	}
       }
     }
-    reap_ioc();
     if (cct->_conf->bdev_inject_crash) {
       ++inject_crash_count;
       if (inject_crash_count * cct->_conf->bdev_aio_poll_ms / 1000 >
@@ -660,7 +659,6 @@ void KernelDevice::_aio_thread()
       }
     }
   }
-  reap_ioc();
   dout(10) << __func__ << " end" << dendl;
 }
 
diff --git a/src/blk/spdk/NVMEDevice.cc b/src/blk/spdk/NVMEDevice.cc
index 935d3bbd11c1..78fcb035101a 100644
--- a/src/blk/spdk/NVMEDevice.cc
+++ b/src/blk/spdk/NVMEDevice.cc
@@ -126,7 +126,6 @@ class SharedDriverQueueData {
   uint32_t block_size;
   uint32_t max_queue_depth;
   struct spdk_nvme_qpair *qpair;
-  bool reap_io = false;
   int alloc_buf_from_pool(Task *t, bool write);
 
   public:
@@ -159,16 +158,11 @@ class SharedDriverQueueData {
       }
       data_buf_list.push_front(*reinterpret_cast<data_cache_buf *>(b));
     }
-
-    bdev->queue_number++;
-    if (bdev->queue_number.load() == 1)
-      reap_io = true;
   }
 
   ~SharedDriverQueueData() {
     if (qpair) {
       spdk_nvme_ctrlr_free_io_qpair(qpair);
-      bdev->queue_number--;
     }
 
     data_buf_list.clear_and_dispose(spdk_dma_free);
@@ -419,8 +413,6 @@ void SharedDriverQueueData::_aio_handle(Task *t, IOContext *ioc)
     }
   }
 
-  if (reap_io)
-    bdev->reap_ioc();
   dout(20) << __func__ << " end" << dendl;
 }
 
diff --git a/src/blk/spdk/NVMEDevice.h b/src/blk/spdk/NVMEDevice.h
index 352856093d80..cbd094d655d1 100644
--- a/src/blk/spdk/NVMEDevice.h
+++ b/src/blk/spdk/NVMEDevice.h
@@ -48,7 +48,6 @@ class NVMEDevice : public BlockDevice {
   string name;
 
  public:
-  std::atomic_int queue_number = {0};
   SharedDriverData *get_driver() { return driver; }
 
   NVMEDevice(CephContext* cct, aio_callback_t cb, void *cbpriv);
diff --git a/src/blk/zoned/HMSMRDevice.cc b/src/blk/zoned/HMSMRDevice.cc
index 8a30be9b0a24..dac61c879e73 100644
--- a/src/blk/zoned/HMSMRDevice.cc
+++ b/src/blk/zoned/HMSMRDevice.cc
@@ -656,7 +656,6 @@ void HMSMRDevice::_aio_thread()
 	}
       }
     }
-    reap_ioc();
     if (cct->_conf->bdev_inject_crash) {
       ++inject_crash_count;
       if (inject_crash_count * cct->_conf->bdev_aio_poll_ms / 1000 >
@@ -668,7 +667,6 @@ void HMSMRDevice::_aio_thread()
       }
     }
   }
-  reap_ioc();
   dout(10) << __func__ << " end" << dendl;
 }
 
diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index b87bc4fd50b2..b26f1b3bafc6 100644
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -3285,7 +3285,7 @@ void BlueFS::_close_writer(FileWriter *h)
     if (bdev[i]) {
       if (h->iocv[i]) {
 	h->iocv[i]->aio_wait();
-	bdev[i]->queue_reap_ioc(h->iocv[i]);
+	delete h->iocv[i];
       }
     }
   }