From: Jason Dillaman Date: Sat, 17 Jan 2015 06:39:07 +0000 (-0500) Subject: librbd: eliminate CoR callback X-Git-Tag: v0.93~197^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=7c7f072aab38e4df504fc8478f901fd3b1d8397e;p=ceph.git librbd: eliminate CoR callback When issuing the CoR copyup request, there is no need to keep initialize the librados callback. Treat the CoR as a fire-and- forget operation. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/AioRequest.cc b/src/librbd/AioRequest.cc index 9a486fc5ecf4..8cb627ac879a 100644 --- a/src/librbd/AioRequest.cc +++ b/src/librbd/AioRequest.cc @@ -33,9 +33,7 @@ namespace librbd { m_object_off(off), m_object_len(len), m_snap_id(snap_id), m_completion(completion), m_parent_completion(NULL), m_hide_enoent(hide_enoent) { - for (std::vector::const_iterator it = snapc.snaps.begin(); - it != snapc.snaps.end(); ++it) - m_snaps.push_back(it->val); + m_snaps.insert(m_snaps.end(), snapc.snaps.begin(), snapc.snaps.end()); } AioRequest::~AioRequest() { @@ -137,31 +135,8 @@ namespace librbd { uint64_t object_overlap = m_ictx->prune_parent_extents(image_extents, image_overlap); if (object_overlap) { m_tried_parent = true; - - if (is_copy_on_read(m_ictx, m_snap_id)) { - // If there is a valid object being coyping up, directly extract - // content and finish up. - Mutex::Locker l3(m_ictx->copyup_list_lock); - - map::iterator it = - m_ictx->copyup_list.find(m_object_no); - if (it != m_ictx->copyup_list.end()) { - Mutex::Locker l4(it->second->get_lock()); - if (it->second->is_ready()) { - ceph::bufferlist ©up_data = it->second->get_copyup_data(); - m_read_data.substr_of(copyup_data, m_object_off, m_object_len); - ldout(m_ictx->cct, 20) << __func__ << " extract content from copyup_list, obj-" - << m_object_no << dendl; - finished = true; - break; - } - } - } - if (is_copy_on_read(m_ictx, m_snap_id)) { m_state = LIBRBD_AIO_READ_COPYUP; - } else { - m_state = LIBRBD_AIO_READ_GUARD; } read_from_parent(image_extents); @@ -180,17 +155,14 @@ namespace librbd { // If read entire object from parent success and CoR is possible, kick // off a asynchronous copyup. This approach minimizes the latency // impact. - m_ictx->copyup_list_lock.Lock(); + Mutex::Locker copyup_locker(m_ictx->copyup_list_lock); map::iterator it = m_ictx->copyup_list.find(m_object_no); if (it == m_ictx->copyup_list.end()) { RWLock::RLocker l(m_ictx->snap_lock); RWLock::RLocker l2(m_ictx->parent_lock); - if (m_ictx->parent == NULL) { ldout(m_ictx->cct, 20) << "parent is gone; do nothing" << dendl; - m_state = LIBRBD_AIO_READ_FLAT; - finished = true; break; } @@ -202,21 +174,10 @@ namespace librbd { CopyupRequest *new_req = new CopyupRequest(m_ictx, m_oid, m_object_no, true); m_ictx->copyup_list[m_object_no] = new_req; - m_ictx->copyup_list_lock.Unlock(); - new_req->queue_read_from_parent(m_image_extents); } - } else { - m_ictx->copyup_list_lock.Unlock(); } - - finished = true; } - - if (r < 0) { - ldout(m_ictx->cct, 20) << "error checking for object existence" << dendl; - } - break; case LIBRBD_AIO_READ_FLAT: ldout(m_ictx->cct, 20) << "should_complete " << this << " READ_FLAT" << dendl; @@ -294,7 +255,6 @@ namespace librbd { ldout(m_ictx->cct, 20) << "WRITE_CHECK_GUARD" << dendl; if (r == -ENOENT) { - RWLock::RLocker l(m_ictx->snap_lock); RWLock::RLocker l2(m_ictx->parent_lock); @@ -329,7 +289,7 @@ namespace librbd { m_state = LIBRBD_AIO_WRITE_COPYUP; if (is_copy_on_read(m_ictx, m_snap_id)) { - m_ictx->copyup_list_lock.Lock(); + m_ictx->copyup_list_lock.Lock(); it = m_ictx->copyup_list.find(m_object_no); if (it == m_ictx->copyup_list.end()) { // If it is not in the list, create a CopyupRequest and wait for it. @@ -338,31 +298,18 @@ namespace librbd { // make sure to wait on this CopyupRequest new_req->append_request(this); m_ictx->copyup_list[m_object_no] = new_req; - m_ictx->copyup_list_lock.Unlock(); m_entire_object = &(new_req->get_copyup_data()); ldout(m_ictx->cct, 20) << __func__ << " creating new Copyup for AioWrite, obj-" << m_object_no << dendl; + m_ictx->copyup_list_lock.Unlock(); new_req->read_from_parent(m_object_image_extents); ldout(m_ictx->cct, 20) << __func__ << " issuing read_from_parent" << dendl; } else { - Mutex::Locker l3(it->second->get_lock()); - if (it->second->is_ready()) { - ldout(m_ictx->cct, 20) << __func__ << " extracting contect from copyup_list, obj-" - << m_object_no << " length=" << it->second->get_copyup_data().length() - << dendl; - m_read_data.append(it->second->get_copyup_data()); - ldout(m_ictx->cct, 20) << __func__ << " extracted contect from copyup_list, obj-" - << m_object_no << " length=" << m_read_data.length() - << dendl; - m_ictx->copyup_list_lock.Unlock(); - return should_complete(1); - } else { - ldout(m_ictx->cct, 20) << __func__ << " someone is reading back from parent" << dendl; - it->second->append_request(this); - m_entire_object = &(m_ictx->copyup_list[m_object_no]->get_copyup_data()); - m_ictx->copyup_list_lock.Unlock(); - } + ldout(m_ictx->cct, 20) << __func__ << " someone is reading back from parent" << dendl; + it->second->append_request(this); + m_entire_object = &it->second->get_copyup_data(); + m_ictx->copyup_list_lock.Unlock(); } } else { read_from_parent(m_object_image_extents); @@ -400,7 +347,6 @@ namespace librbd { << m_object_no << dendl; it = m_ictx->copyup_list.find(m_object_no); assert(it != m_ictx->copyup_list.end()); - assert(it->second->is_ready()); assert(m_entire_object); m_read_data.append(*m_entire_object); } diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc index d3790a96fa48..690d149d4fd1 100644 --- a/src/librbd/CopyupRequest.cc +++ b/src/librbd/CopyupRequest.cc @@ -19,60 +19,38 @@ namespace librbd { CopyupRequest::CopyupRequest(ImageCtx *ictx, const std::string &oid, uint64_t objectno, bool send_copyup) - : m_ictx(ictx), m_oid(oid), m_object_no(objectno), - m_lock("librbd::CopyupRequest::m_lock"), m_ready(false), - m_send_copyup(send_copyup), m_parent_completion(NULL), - m_copyup_completion(NULL) {} + : m_ictx(ictx), m_oid(oid), m_object_no(objectno), m_send_copyup(send_copyup) + { + } CopyupRequest::~CopyupRequest() { + assert(m_ictx->copyup_list_lock.is_locked()); assert(m_pending_requests.empty()); - m_ictx->copyup_list_lock.Lock(); ldout(m_ictx->cct, 20) << __func__ << " removing the slot " << dendl; map::iterator it = m_ictx->copyup_list.find(m_object_no); assert(it != m_ictx->copyup_list.end()); - it->second = NULL; m_ictx->copyup_list.erase(it); - if (m_ictx->copyup_list.empty()) + if (m_ictx->copyup_list.empty()) { m_ictx->copyup_list_cond.Signal(); + } ldout(m_ictx->cct, 20) << __func__ << " remove the slot " << m_object_no << " in copyup_list, size = " << m_ictx->copyup_list.size() << dendl; - - m_ictx->copyup_list_lock.Unlock(); - } - - void CopyupRequest::set_ready() { - m_ready = true; - } - - bool CopyupRequest::is_ready() { - return m_ready; - } - - bool CopyupRequest::should_send_copyup() { - return m_send_copyup; } ceph::bufferlist& CopyupRequest::get_copyup_data() { return m_copyup_data; } - Mutex& CopyupRequest::get_lock() { - return m_lock; - } - void CopyupRequest::append_request(AioRequest *req) { - assert(!m_ready); m_pending_requests.push_back(req); } void CopyupRequest::complete_all(int r) { - assert(m_ready); - while (!m_pending_requests.empty()) { vector::iterator it = m_pending_requests.begin(); AioRequest *req = *it; @@ -89,28 +67,32 @@ namespace librbd { m_ictx->snap_lock.put_read(); std::vector snaps; - for (std::vector::const_iterator it = snapc.snaps.begin(); - it != snapc.snaps.end(); ++it) { - snaps.push_back(it->val); - } + snaps.insert(snaps.end(), snapc.snaps.begin(), snapc.snaps.end()); librados::ObjectWriteOperation copyup_op; copyup_op.exec("rbd", "copyup", m_copyup_data); - m_copyup_completion = librados::Rados::aio_create_completion(this, NULL, rbd_copyup_cb); - m_ictx->md_ctx.aio_operate(m_oid, m_copyup_completion, ©up_op, - snapc.seq.val, snaps); - m_copyup_completion->release(); + librados::AioCompletion *comp = + librados::Rados::aio_create_completion(NULL, NULL, NULL); + m_ictx->md_ctx.aio_operate(m_oid, comp, ©up_op, snapc.seq.val, snaps); + comp->release(); } void CopyupRequest::read_from_parent(vector >& image_extents) { - m_parent_completion = aio_create_completion_internal(this, rbd_read_from_parent_cb); + AioCompletion *comp = aio_create_completion_internal( + this, &CopyupRequest::read_from_parent_cb); ldout(m_ictx->cct, 20) << __func__ << " this = " << this - << " parent completion " << m_parent_completion + << " parent completion " << comp << " extents " << image_extents << dendl; - aio_read(m_ictx->parent, image_extents, NULL, &m_copyup_data, m_parent_completion, 0); + + int r = aio_read(m_ictx->parent, image_extents, NULL, &m_copyup_data, + comp, 0); + if (r < 0) { + comp->release(); + delete this; + } } void CopyupRequest::queue_read_from_parent(vector >& image_extents) @@ -121,34 +103,24 @@ namespace librbd { m_ictx->copyup_finisher->queue(ctx); } - void CopyupRequest::rbd_read_from_parent_cb(completion_t cb, void *arg) + void CopyupRequest::read_from_parent_cb(completion_t cb, void *arg) { CopyupRequest *req = reinterpret_cast(arg); AioCompletion *comp = reinterpret_cast(cb); ldout(req->m_ictx->cct, 20) << __func__ << dendl; - - req->m_lock.Lock(); - req->set_ready(); req->complete_all(comp->get_return_value()); - req->m_lock.Unlock(); // If this entry is created by a read request, then copyup operation will // be performed asynchronously. Perform cleaning up from copyup callback. // If this entry is created by a write request, then copyup operation will // be performed synchronously by AioWrite. After extracting data, perform // cleaning up here - if (req->should_send_copyup()) + if (req->m_send_copyup) { req->send_copyup(comp->get_return_value()); - else - delete req; - } - - void CopyupRequest::rbd_copyup_cb(rados_completion_t c, void *arg) - { - CopyupRequest *req = reinterpret_cast(arg); + } - ldout(req->m_ictx->cct, 20) << __func__ << dendl; + Mutex::Locker l(req->m_ictx->copyup_list_lock); delete req; } } diff --git a/src/librbd/CopyupRequest.h b/src/librbd/CopyupRequest.h index c5572cfad927..d251fa8ee607 100644 --- a/src/librbd/CopyupRequest.h +++ b/src/librbd/CopyupRequest.h @@ -19,23 +19,12 @@ namespace librbd { bool send_copyup); ~CopyupRequest(); - void set_ready(); - bool is_ready(); - bool should_send_copyup(); ceph::bufferlist& get_copyup_data(); - Mutex& get_lock(); void append_request(AioRequest *req); - void complete_all(int r); - void send_copyup(int r); void read_from_parent(vector >& image_extents); void queue_read_from_parent(vector >& image_extents); - static void rbd_read_from_parent_cb(completion_t cb, void *arg); - static void rbd_copyup_cb(completion_t aio_completion_impl, void *arg); - private: - ImageCtx *m_ictx; - class C_ReadFromParent : public Context { public: C_ReadFromParent(CopyupRequest *c, vector > i) @@ -50,19 +39,18 @@ namespace librbd { vector > m_image_extents; }; + ImageCtx *m_ictx; std::string m_oid; uint64_t m_object_no; - Mutex m_lock; - bool m_ready; bool m_send_copyup; - AioCompletion *m_parent_completion; - librados::AioCompletion *m_copyup_completion; ceph::bufferlist m_copyup_data; vector m_pending_requests; - }; - void rbd_read_from_parent_cb(completion_t cb, void *arg); - void rbd_copyup_cb(completion_t aio_completion_impl, void *arg); + void complete_all(int r); + void send_copyup(int r); + static void read_from_parent_cb(completion_t cb, void *arg); + + }; } #endif