From e8ab3fdc870b49ff4b590c769660ca3287bb9528 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 5 May 2017 14:39:18 -0400 Subject: [PATCH] os/bluestore: remove useless IOContext::num_reading If we are a syncrhonous read, we don't need this: we don't aio_wait for sync reads. If we are an aio_read, we are in the aio_running count anyway, and there is also no purpose for this counter. I'm a bit unsure about the NVME use of this counter; I switched it to use num_running (pretty sure we aren't mixing reads and writes on a single IOContext) *but* it might make more sense to switch to a private counter. Signed-off-by: Sage Weil --- src/os/bluestore/BlockDevice.cc | 6 +++--- src/os/bluestore/BlockDevice.h | 1 - src/os/bluestore/KernelDevice.cc | 2 -- src/os/bluestore/NVMEDevice.cc | 6 +++--- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/os/bluestore/BlockDevice.cc b/src/os/bluestore/BlockDevice.cc index 751db6d8a2c8..cdc13ff36374 100644 --- a/src/os/bluestore/BlockDevice.cc +++ b/src/os/bluestore/BlockDevice.cc @@ -34,10 +34,10 @@ void IOContext::aio_wait() { std::unique_lock l(lock); // see _aio_thread for waker logic - while (num_running.load() > 0 || num_reading.load() > 0) { + while (num_running.load() > 0) { dout(10) << __func__ << " " << this - << " waiting for " << num_running.load() << " aios and/or " - << num_reading.load() << " readers to complete" << dendl; + << " waiting for " << num_running.load() << " aios to complete" + << dendl; cond.wait(l); } dout(20) << __func__ << " " << this << " done" << dendl; diff --git a/src/os/bluestore/BlockDevice.h b/src/os/bluestore/BlockDevice.h index 3057c00e0e8d..54d284061cf5 100644 --- a/src/os/bluestore/BlockDevice.h +++ b/src/os/bluestore/BlockDevice.h @@ -43,7 +43,6 @@ struct IOContext { std::list running_aios; ///< submitting or submitted std::atomic_int num_pending = {0}; std::atomic_int num_running = {0}; - std::atomic_int num_reading = {0}; explicit IOContext(CephContext* cct, void *p) : cct(cct), priv(p) diff --git a/src/os/bluestore/KernelDevice.cc b/src/os/bluestore/KernelDevice.cc index 2092d88f7363..9f4ba718c03b 100644 --- a/src/os/bluestore/KernelDevice.cc +++ b/src/os/bluestore/KernelDevice.cc @@ -629,7 +629,6 @@ int KernelDevice::read(uint64_t off, uint64_t len, bufferlist *pbl, assert(off + len <= size); _aio_log_start(ioc, off, len); - ++ioc->num_reading; bufferptr p = buffer::create_page_aligned(len); int r = ::pread(buffered ? fd_buffered : fd_direct, @@ -647,7 +646,6 @@ int KernelDevice::read(uint64_t off, uint64_t len, bufferlist *pbl, out: _aio_log_finish(ioc, off, len); - --ioc->num_reading; return r < 0 ? r : 0; } diff --git a/src/os/bluestore/NVMEDevice.cc b/src/os/bluestore/NVMEDevice.cc index 5f5264395c42..2ea4eec33508 100644 --- a/src/os/bluestore/NVMEDevice.cc +++ b/src/os/bluestore/NVMEDevice.cc @@ -846,7 +846,7 @@ void io_complete(void *t, const struct spdk_nvme_cpl *completion) delete task; } else { task->return_code = 0; - if(!--ctx->num_reading) { + if (!--ctx->num_running) { task->io_wake(); } } @@ -1052,7 +1052,7 @@ int NVMEDevice::read(uint64_t off, uint64_t len, bufferlist *pbl, t->fill_cb = [buf, t]() { t->copy_to_buf(buf, 0, t->len); }; - ++ioc->num_reading; + ++ioc->num_running; if(queue_id == -1) queue_id = ceph_gettid(); driver->get_queue(queue_id)->queue_task(t); @@ -1118,7 +1118,7 @@ int NVMEDevice::read_random(uint64_t off, uint64_t len, char *buf, bool buffered t->fill_cb = [buf, t, off, len]() { t->copy_to_buf(buf, off-t->offset, len); }; - ++ioc.num_reading; + ++ioc.num_running; if(queue_id == -1) queue_id = ceph_gettid(); driver->get_queue(queue_id)->queue_task(t); -- 2.47.3