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 6804ee50cbcaa0f08af053d272c568a0edcabf88..d5d3169e9131e0f58b00d5a30c8a39b51d9a09e3 100644 (file)
--- 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 191eb8ec908af8ebd0c32a5dbf8d96c0e8545add..44ed3de4d8c610ab8219d300b063d86bf4acc054 100644 (file)
--- 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 d41cda05b63d470b0eac9e7dc28a8486aa8d91b4..09a4b687ef062b11d12347e865f17f7f88b23cd2 100644 (file)
--- 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 935d3bbd11c165ddc9bc266082783af706012f1e..78fcb035101a219f8b7bfcfa12820b89cb2edece 100644 (file)
--- 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 352856093d802600a48440e10b03e9c36b42051e..cbd094d655d1cd5b82b2c9b29d72e4488b0ea3bd 100644 (file)
--- 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 8a30be9b0a2421864308db0a12cd12a86a2d1821..dac61c879e735adecd219bae5afacb084225e732 100644 (file)
--- 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 b87bc4fd50b299e166e99c56bc4d79c8ff6a95fc..b26f1b3bafc6518952130bbba17685c7916f39a3 100644 (file)
--- 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];
       }
     }
   }