From 75e26b23c335c728b547e3d60a808640c8422017 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 8 Jul 2015 11:26:57 -0400 Subject: [PATCH] librbd: ImageWatcher no longer maintains pending AIO op list Previously the ImageWatcher stored delayed ops that were waiting on the image exclusive lock. This management has been moved to the AioImageRequestWQ to ensure requests are processed in-order. Signed-off-by: Jason Dillaman --- src/librbd/AioImageRequest.cc | 22 +-- src/librbd/AioImageRequestWQ.cc | 82 +++++++++--- src/librbd/AioImageRequestWQ.h | 24 ++-- src/librbd/ImageWatcher.cc | 231 +++++++++++++++----------------- src/librbd/ImageWatcher.h | 22 ++- src/librbd/internal.cc | 10 ++ 6 files changed, 206 insertions(+), 185 deletions(-) diff --git a/src/librbd/AioImageRequest.cc b/src/librbd/AioImageRequest.cc index 379dd535d8538..9f0a5d78ed8b5 100644 --- a/src/librbd/AioImageRequest.cc +++ b/src/librbd/AioImageRequest.cc @@ -7,8 +7,6 @@ #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/internal.h" -#include -#include "include/assert.h" #define dout_subsys ceph_subsys_rbd #undef dout_prefix @@ -169,14 +167,8 @@ void AioImageWrite::execute_request() { m_aio_comp->start_op(&m_image_ctx, AIO_TYPE_WRITE); } - if (m_image_ctx.image_watcher->is_lock_supported() && - !m_image_ctx.image_watcher->is_lock_owner()) { - m_image_ctx.image_watcher->request_lock( - boost::bind(&AioImageRequest::write, &m_image_ctx, _1, m_off, m_len, - m_buf, m_op_flags), m_aio_comp); - m_aio_comp->put(); - return; - } + assert(!m_image_ctx.image_watcher->is_lock_supported() || + m_image_ctx.image_watcher->is_lock_owner()); // map vector extents; @@ -244,14 +236,8 @@ void AioImageDiscard::execute_request() { m_aio_comp->start_op(&m_image_ctx, AIO_TYPE_DISCARD); } - if (m_image_ctx.image_watcher->is_lock_supported() && - !m_image_ctx.image_watcher->is_lock_owner()) { - m_image_ctx.image_watcher->request_lock( - boost::bind(&AioImageRequest::discard, &m_image_ctx, _1, m_off, m_len), - m_aio_comp); - m_aio_comp->put(); - return; - } + assert(!m_image_ctx.image_watcher->is_lock_supported() || + m_image_ctx.image_watcher->is_lock_owner()); // map vector extents; diff --git a/src/librbd/AioImageRequestWQ.cc b/src/librbd/AioImageRequestWQ.cc index d8f6e7778af0c..bef86c0933e6b 100644 --- a/src/librbd/AioImageRequestWQ.cc +++ b/src/librbd/AioImageRequestWQ.cc @@ -17,8 +17,8 @@ namespace librbd { ssize_t AioImageRequestWQ::read(uint64_t off, size_t len, char *buf, int op_flags) { CephContext *cct = m_image_ctx.cct; - ldout(cct, 20) << "read " << &m_image_ctx << " off = " << off << " len = " - << len << dendl; + ldout(cct, 20) << "read: ictx=" << &m_image_ctx << ", off=" << off << ", " + << "len = " << len << dendl; std::vector > image_extents; image_extents.push_back(make_pair(off, len)); @@ -32,8 +32,8 @@ ssize_t AioImageRequestWQ::read(uint64_t off, size_t len, char *buf, ssize_t AioImageRequestWQ::write(uint64_t off, size_t len, const char *buf, int op_flags) { CephContext *cct = m_image_ctx.cct; - ldout(cct, 20) << "write " << &m_image_ctx << " off = " << off << " len = " - << len << dendl; + ldout(cct, 20) << "write: ictx=" << &m_image_ctx << ", off=" << off << ", " + << "len = " << len << dendl; m_image_ctx.snap_lock.get_read(); int r = clip_io(&m_image_ctx, off, &len); @@ -55,8 +55,8 @@ ssize_t AioImageRequestWQ::write(uint64_t off, size_t len, const char *buf, int AioImageRequestWQ::discard(uint64_t off, uint64_t len) { CephContext *cct = m_image_ctx.cct; - ldout(cct, 20) << "discard " << &m_image_ctx << " off = " << off << " len = " - << len << dendl; + ldout(cct, 20) << "discard: ictx=" << &m_image_ctx << ", off=" << off << ", " + << "len = " << len << dendl; m_image_ctx.snap_lock.get_read(); int r = clip_io(&m_image_ctx, off, &len); @@ -81,12 +81,13 @@ void AioImageRequestWQ::aio_read(AioCompletion *c, uint64_t off, size_t len, c->init_time(&m_image_ctx, librbd::AIO_TYPE_READ); CephContext *cct = m_image_ctx.cct; ldout(cct, 20) << "aio_read: ictx=" << &m_image_ctx << ", " - << "completion=" << c << ", off=" << off << ", len=" << len - << "flags=" << op_flags << dendl; + << "completion=" << c << ", off=" << off << ", " + << "len=" << len << ", " << "flags=" << op_flags << dendl; RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.non_blocking_aio) { - queue(new AioImageRead(m_image_ctx, c, off, len, buf, pbl, op_flags)); + queue(new AioImageRead(m_image_ctx, c, off, len, buf, pbl, op_flags), + false); } else { AioImageRequest::read(&m_image_ctx, c, off, len, buf, pbl, op_flags); } @@ -97,13 +98,14 @@ void AioImageRequestWQ::aio_write(AioCompletion *c, uint64_t off, size_t len, c->init_time(&m_image_ctx, librbd::AIO_TYPE_WRITE); CephContext *cct = m_image_ctx.cct; ldout(cct, 20) << "aio_write: ictx=" << &m_image_ctx << ", " - << "completion=" << c << ", off=" << off << ", len=" << len - << "flags=" << op_flags << dendl; + << "completion=" << c << ", off=" << off << ", " + << "len=" << len << ", flags=" << op_flags << dendl; RWLock::RLocker owner_locker(m_image_ctx.owner_lock); bool lock_required = is_lock_required(); if (m_image_ctx.non_blocking_aio || lock_required) { - queue(new AioImageWrite(m_image_ctx, c, off, len, buf, op_flags)); + queue(new AioImageWrite(m_image_ctx, c, off, len, buf, op_flags), + lock_required); } else { AioImageRequest::write(&m_image_ctx, c, off, len, buf, op_flags); } @@ -120,7 +122,8 @@ void AioImageRequestWQ::aio_discard(AioCompletion *c, uint64_t off, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); bool lock_required = is_lock_required(); if (m_image_ctx.non_blocking_aio || lock_required) { - queue(new AioImageDiscard(m_image_ctx, c, off, len)); + queue(new AioImageDiscard(m_image_ctx, c, off, len), + lock_required); } else { AioImageRequest::discard(&m_image_ctx, c, off, len); } @@ -133,16 +136,33 @@ void AioImageRequestWQ::aio_flush(AioCompletion *c) { << "completion=" << c << dendl; RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - if (m_image_ctx.non_blocking_aio) { - queue(new AioImageFlush(m_image_ctx, c)); + if (m_image_ctx.non_blocking_aio || !writes_empty()) { + queue(new AioImageFlush(m_image_ctx, c), false); } else { AioImageRequest::flush(&m_image_ctx, c); } } -bool AioImageRequestWQ::writes_empty() const { +void AioImageRequestWQ::suspend_writes() { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 5) << __func__ << ": " << &m_image_ctx << dendl; + Mutex::Locker locker(m_lock); - return (m_queued_writes > 0); + m_writes_suspended = true; + while (m_in_progress_writes > 0) { + m_cond.Wait(m_lock); + } +} + +void AioImageRequestWQ::resume_writes() { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 5) << __func__ << ": " << &m_image_ctx << dendl; + + { + Mutex::Locker locker(m_lock); + m_writes_suspended = false; + } + signal(); } void *AioImageRequestWQ::_void_dequeue() { @@ -168,6 +188,10 @@ void *AioImageRequestWQ::_void_dequeue() { } void AioImageRequestWQ::process(AioImageRequest *req) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 20) << __func__ << ": ictx=" << &m_image_ctx << ", " + << "req=" << req << dendl; + { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); req->send(); @@ -177,7 +201,9 @@ void AioImageRequestWQ::process(AioImageRequest *req) { Mutex::Locker locker(m_lock); if (req->is_write_op()) { assert(m_queued_writes > 0); - --m_queued_writes; + if (--m_queued_writes == 0) { + m_image_ctx.image_watcher->clear_aio_ops_pending(); + } assert(m_in_progress_writes > 0); if (--m_in_progress_writes == 0) { @@ -197,14 +223,30 @@ bool AioImageRequestWQ::is_lock_required() { !m_image_ctx.image_watcher->is_lock_owner()); } -void AioImageRequestWQ::queue(AioImageRequest *req) { +void AioImageRequestWQ::queue(AioImageRequest *req, bool lock_required) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 20) << __func__ << ": ictx=" << &m_image_ctx << ", " + << "req=" << req << ", lock_req=" << lock_required << dendl; + + assert(m_image_ctx.owner_lock.is_locked()); + + bool first_write_op = false; { Mutex::Locker locker(m_lock); if (req->is_write_op()) { - ++m_queued_writes; + if (++m_queued_writes == 1) { + first_write_op = true; + } } } ThreadPool::PointerWQ::queue(req); + + if (first_write_op) { + m_image_ctx.image_watcher->flag_aio_ops_pending(); + if (lock_required) { + m_image_ctx.image_watcher->request_lock(); + } + } } } // namespace librbd diff --git a/src/librbd/AioImageRequestWQ.h b/src/librbd/AioImageRequestWQ.h index 034026c82223a..c57715fc2393e 100644 --- a/src/librbd/AioImageRequestWQ.h +++ b/src/librbd/AioImageRequestWQ.h @@ -35,29 +35,23 @@ public: using ThreadPool::PointerWQ::drain; - bool writes_empty() const; - inline bool writes_suspended() const { + inline bool writes_empty() const { Mutex::Locker locker(m_lock); - return m_writes_suspended; + return (m_queued_writes == 0); } - void suspend_writes() { + inline bool writes_suspended() const { Mutex::Locker locker(m_lock); - while (m_in_progress_writes > 0) { - m_cond.Wait(m_lock); - } + return m_writes_suspended; } - void resume_writes() { - { - Mutex::Locker locker(m_lock); - m_writes_suspended = false; - } - signal(); - } + void suspend_writes(); + void resume_writes(); + protected: virtual void *_void_dequeue(); virtual void process(AioImageRequest *req); + private: ImageCtx &m_image_ctx; mutable Mutex m_lock; @@ -67,7 +61,7 @@ private: uint32_t m_queued_writes; bool is_lock_required(); - void queue(AioImageRequest *req); + void queue(AioImageRequest *req, bool lock_required); }; } // namespace librbd diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index ba2765695eb15..026432cd5f595 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -2,6 +2,7 @@ // vim: ts=8 sw=2 smarttab #include "librbd/ImageWatcher.h" #include "librbd/AioCompletion.h" +#include "librbd/AioImageRequestWQ.h" #include "librbd/ImageCtx.h" #include "librbd/internal.h" #include "librbd/ObjectMap.h" @@ -34,11 +35,10 @@ ImageWatcher::ImageWatcher(ImageCtx &image_ctx) : m_image_ctx(image_ctx), m_watch_lock(unique_lock_name("librbd::ImageWatcher::m_watch_lock", this)), m_watch_ctx(*this), m_watch_handle(0), - m_watch_state(WATCH_STATE_UNREGISTERED), + m_watch_state(WATCH_STATE_UNREGISTERED), m_aio_ops_pending(false), m_lock_owner_state(LOCK_OWNER_STATE_NOT_LOCKED), m_task_finisher(new TaskFinisher(*m_image_ctx.cct)), m_async_request_lock(unique_lock_name("librbd::ImageWatcher::m_async_request_lock", this)), - m_aio_request_lock(unique_lock_name("librbd::ImageWatcher::m_aio_request_lock", this)), m_owner_client_id_lock(unique_lock_name("librbd::ImageWatcher::m_owner_client_id_lock", this)) { } @@ -93,11 +93,6 @@ int ImageWatcher::register_watch() { int ImageWatcher::unregister_watch() { ldout(m_image_ctx.cct, 10) << this << " unregistering image watcher" << dendl; - { - Mutex::Locker l(m_aio_request_lock); - assert(m_aio_requests.empty()); - } - cancel_async_requests(); m_task_finisher->cancel_all(); @@ -115,6 +110,16 @@ int ImageWatcher::unregister_watch() { return r; } +void ImageWatcher::refresh() { + assert(m_image_ctx.owner_lock.is_locked()); + + if (is_lock_supported() && !is_lock_owner()) { + m_image_ctx.aio_work_queue->suspend_writes(); + } else if (!is_lock_supported()) { + m_image_ctx.aio_work_queue->resume_writes(); + } +} + int ImageWatcher::try_lock() { assert(m_image_ctx.owner_lock.is_wlocked()); assert(m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED); @@ -182,33 +187,12 @@ int ImageWatcher::try_lock() { return 0; } -void ImageWatcher::request_lock( - const boost::function& restart_op, AioCompletion* c) { - assert(m_image_ctx.owner_lock.is_locked()); - assert(m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED); - +void ImageWatcher::request_lock() { { - Mutex::Locker l(m_aio_request_lock); - bool request_pending = !m_aio_requests.empty(); - ldout(m_image_ctx.cct, 15) << this << " queuing aio request: " << c - << dendl; - - c->get(); - m_aio_requests.push_back(std::make_pair(restart_op, c)); - if (request_pending) { - return; - } - } - - RWLock::RLocker l(m_watch_lock); - if (m_watch_state == WATCH_STATE_REGISTERED) { - ldout(m_image_ctx.cct, 15) << this << " requesting exclusive lock" << dendl; - - // run notify request in finisher to avoid blocking aio path - FunctionContext *ctx = new FunctionContext( - boost::bind(&ImageWatcher::notify_request_lock, this)); - m_task_finisher->queue(TASK_CODE_REQUEST_LOCK, ctx); + RWLock::WLocker watch_locker(m_watch_lock); + m_aio_ops_pending = true; } + schedule_request_lock(false); } bool ImageWatcher::try_request_lock() { @@ -321,6 +305,8 @@ int ImageWatcher::lock() { bufferlist bl; ::encode(NotifyMessage(AcquiredLockPayload(get_client_id())), bl); + m_image_ctx.aio_work_queue->resume_writes(); + // send the notification when we aren't holding locks FunctionContext *ctx = new FunctionContext( boost::bind(&IoCtx::notify2, &m_image_ctx.md_ctx, m_image_ctx.header_oid, @@ -364,6 +350,10 @@ int ImageWatcher::unlock() m_image_ctx.object_map.unlock(); } + if (is_lock_supported()) { + m_image_ctx.aio_work_queue->suspend_writes(); + } + Mutex::Locker l(m_owner_client_id_lock); set_owner_client_id(ClientId()); @@ -383,8 +373,10 @@ bool ImageWatcher::release_lock() } prepare_unlock(); m_image_ctx.owner_lock.put_write(); + m_image_ctx.cancel_async_requests(); m_image_ctx.flush_async_operations(); + m_image_ctx.aio_work_queue->suspend_writes(); { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); @@ -401,6 +393,22 @@ bool ImageWatcher::release_lock() return true; } +void ImageWatcher::flag_aio_ops_pending() { + RWLock::WLocker watch_locker(m_watch_lock); + if (!m_aio_ops_pending) { + ldout(m_image_ctx.cct, 20) << this << " pending AIO ops" << dendl; + m_aio_ops_pending = true; + } +} + +void ImageWatcher::clear_aio_ops_pending() { + RWLock::WLocker watch_locker(m_watch_lock); + if (m_aio_ops_pending) { + ldout(m_image_ctx.cct, 20) << this << " no pending AIO ops" << dendl; + m_aio_ops_pending = false; + } +} + void ImageWatcher::assert_header_locked(librados::ObjectWriteOperation *op) { rados::cls::lock::assert_locked(op, RBD_LOCK_NAME, LOCK_EXCLUSIVE, encode_lock_cookie(), WATCHER_LOCK_TAG); @@ -556,39 +564,6 @@ bool ImageWatcher::decode_lock_cookie(const std::string &tag, return true; } -void ImageWatcher::schedule_retry_aio_requests(bool use_timer) { - m_task_finisher->cancel(TASK_CODE_REQUEST_LOCK); - Context *ctx = new FunctionContext(boost::bind( - &ImageWatcher::retry_aio_requests, this)); - if (use_timer) { - m_task_finisher->add_event_after(TASK_CODE_RETRY_AIO_REQUESTS, - RETRY_DELAY_SECONDS, ctx); - } else { - m_task_finisher->queue(TASK_CODE_RETRY_AIO_REQUESTS, ctx); - } -} - -void ImageWatcher::retry_aio_requests() { - m_task_finisher->cancel(TASK_CODE_RETRY_AIO_REQUESTS); - - std::vector lock_request_restarts; - { - Mutex::Locker l(m_aio_request_lock); - lock_request_restarts.swap(m_aio_requests); - } - - ldout(m_image_ctx.cct, 15) << this << " retrying pending aio requests" - << dendl; - RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - for (std::vector::iterator iter = lock_request_restarts.begin(); - iter != lock_request_restarts.end(); ++iter) { - ldout(m_image_ctx.cct, 20) << this << " retrying aio request: " - << iter->second << dendl; - iter->first(iter->second); - iter->second->put(); - } -} - void ImageWatcher::schedule_cancel_async_requests() { FunctionContext *ctx = new FunctionContext( boost::bind(&ImageWatcher::cancel_async_requests, this)); @@ -629,14 +604,33 @@ void ImageWatcher::notify_released_lock() { m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, NOTIFY_TIMEOUT, NULL); } +void ImageWatcher::schedule_request_lock(bool use_timer, int timer_delay) { + assert(m_image_ctx.owner_lock.is_locked()); + assert(m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED); + + RWLock::RLocker watch_locker(m_watch_lock); + if (m_watch_state == WATCH_STATE_REGISTERED && m_aio_ops_pending) { + ldout(m_image_ctx.cct, 15) << this << " requesting exclusive lock" << dendl; + + FunctionContext *ctx = new FunctionContext( + boost::bind(&ImageWatcher::notify_request_lock, this)); + if (use_timer) { + if (timer_delay < 0) { + timer_delay = RETRY_DELAY_SECONDS; + } + m_task_finisher->add_event_after(TASK_CODE_REQUEST_LOCK, timer_delay, + ctx); + } else { + m_task_finisher->queue(TASK_CODE_REQUEST_LOCK, ctx); + } + } +} + void ImageWatcher::notify_request_lock() { ldout(m_image_ctx.cct, 10) << this << " notify request lock" << dendl; - m_task_finisher->cancel(TASK_CODE_RETRY_AIO_REQUESTS); - m_image_ctx.owner_lock.get_read(); + RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (try_request_lock()) { - m_image_ctx.owner_lock.put_read(); - retry_aio_requests(); return; } @@ -644,23 +638,20 @@ void ImageWatcher::notify_request_lock() { ::encode(NotifyMessage(RequestLockPayload(get_client_id())), bl); int r = notify_lock_owner(bl); - m_image_ctx.owner_lock.put_read(); - if (r == -ETIMEDOUT) { - ldout(m_image_ctx.cct, 5) << this << "timed out requesting lock: retrying" + ldout(m_image_ctx.cct, 5) << this << " timed out requesting lock: retrying" << dendl; - retry_aio_requests(); + schedule_request_lock(false); } else if (r < 0) { lderr(m_image_ctx.cct) << this << " error requesting lock: " << cpp_strerror(r) << dendl; - schedule_retry_aio_requests(true); + schedule_request_lock(true); } else { // lock owner acked -- but resend if we don't see them release the lock int retry_timeout = m_image_ctx.cct->_conf->client_notify_timeout; - FunctionContext *ctx = new FunctionContext( - boost::bind(&ImageWatcher::notify_request_lock, this)); - m_task_finisher->add_event_after(TASK_CODE_REQUEST_LOCK, - retry_timeout, ctx); + ldout(m_image_ctx.cct, 15) << this << " will retry in " << retry_timeout + << " seconds" << dendl; + schedule_request_lock(true, retry_timeout); } } @@ -824,10 +815,10 @@ void ImageWatcher::handle_payload(const AcquiredLockPayload &payload, set_owner_client_id(payload.client_id); } - RWLock::RLocker l(m_image_ctx.owner_lock); + RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED) { schedule_cancel_async_requests(); - schedule_retry_aio_requests(false); + schedule_request_lock(false); } } @@ -845,10 +836,10 @@ void ImageWatcher::handle_payload(const ReleasedLockPayload &payload, set_owner_client_id(ClientId()); } - RWLock::RLocker l(m_image_ctx.owner_lock); + RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED) { schedule_cancel_async_requests(); - schedule_retry_aio_requests(false); + schedule_request_lock(false); } } @@ -1080,53 +1071,53 @@ void ImageWatcher::acknowledge_notify(uint64_t notify_id, uint64_t handle, void ImageWatcher::reregister_watch() { ldout(m_image_ctx.cct, 10) << this << " re-registering image watch" << dendl; + RWLock::WLocker l(m_image_ctx.owner_lock); + bool was_lock_owner = false; + if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + // ensure all async requests are canceled and IO is flushed + was_lock_owner = release_lock(); + } + + int r; { - RWLock::WLocker l(m_image_ctx.owner_lock); - bool was_lock_owner = false; - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { - // ensure all async requests are canceled and IO is flushed - was_lock_owner = release_lock(); + RWLock::WLocker l(m_watch_lock); + if (m_watch_state != WATCH_STATE_ERROR) { + return; } - int r; - { - RWLock::WLocker l(m_watch_lock); - if (m_watch_state != WATCH_STATE_ERROR) { - return; - } - - r = m_image_ctx.md_ctx.watch2(m_image_ctx.header_oid, - &m_watch_handle, &m_watch_ctx); - if (r < 0) { - lderr(m_image_ctx.cct) << this << " failed to re-register image watch: " - << cpp_strerror(r) << dendl; - if (r != -ESHUTDOWN) { - FunctionContext *ctx = new FunctionContext(boost::bind( - &ImageWatcher::reregister_watch, this)); - m_task_finisher->add_event_after(TASK_CODE_REREGISTER_WATCH, - RETRY_DELAY_SECONDS, ctx); - } - return; + r = m_image_ctx.md_ctx.watch2(m_image_ctx.header_oid, + &m_watch_handle, &m_watch_ctx); + if (r < 0) { + lderr(m_image_ctx.cct) << this << " failed to re-register image watch: " + << cpp_strerror(r) << dendl; + if (r != -ESHUTDOWN) { + FunctionContext *ctx = new FunctionContext(boost::bind( + &ImageWatcher::reregister_watch, this)); + m_task_finisher->add_event_after(TASK_CODE_REREGISTER_WATCH, + RETRY_DELAY_SECONDS, ctx); } - - m_watch_state = WATCH_STATE_REGISTERED; + return; } - handle_payload(HeaderUpdatePayload(), NULL); - if (was_lock_owner) { - r = try_lock(); - if (r == -EBUSY) { - ldout(m_image_ctx.cct, 5) << this << "lost image lock while " - << "re-registering image watch" << dendl; - } else if (r < 0) { - lderr(m_image_ctx.cct) << this - << "failed to lock image while re-registering " - << "image watch" << cpp_strerror(r) << dendl; - } + m_watch_state = WATCH_STATE_REGISTERED; + } + handle_payload(HeaderUpdatePayload(), NULL); + + if (was_lock_owner) { + r = try_lock(); + if (r == -EBUSY) { + ldout(m_image_ctx.cct, 5) << this << "lost image lock while " + << "re-registering image watch" << dendl; + } else if (r < 0) { + lderr(m_image_ctx.cct) << this + << "failed to lock image while re-registering " + << "image watch" << cpp_strerror(r) << dendl; } } - retry_aio_requests(); + if (m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED) { + schedule_request_lock(false); + } } void ImageWatcher::WatchCtx::handle_notify(uint64_t notify_id, diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h index 9099e1685d028..110c221516204 100644 --- a/src/librbd/ImageWatcher.h +++ b/src/librbd/ImageWatcher.h @@ -20,7 +20,6 @@ class entity_name_t; namespace librbd { - class AioCompletion; class ImageCtx; template class TaskFinisher; @@ -37,13 +36,17 @@ namespace librbd { int register_watch(); int unregister_watch(); + void refresh(); + int try_lock(); - void request_lock(const boost::function& restart_op, - AioCompletion* c); + void request_lock(); void prepare_unlock(); void cancel_unlock(); int unlock(); + void flag_aio_ops_pending(); + void clear_aio_ops_pending(); + void assert_header_locked(librados::ObjectWriteOperation *op); int notify_flatten(uint64_t request_id, ProgressContext &prog_ctx); @@ -77,7 +80,6 @@ namespace librbd { TASK_CODE_REQUEST_LOCK, TASK_CODE_RELEASING_LOCK, TASK_CODE_RELEASED_LOCK, - TASK_CODE_RETRY_AIO_REQUESTS, TASK_CODE_CANCEL_ASYNC_REQUESTS, TASK_CODE_REREGISTER_WATCH, TASK_CODE_ASYNC_REQUEST, @@ -85,8 +87,6 @@ namespace librbd { }; typedef std::pair AsyncRequest; - typedef std::pair, - AioCompletion *> AioRequest; class Task { public: @@ -193,6 +193,7 @@ namespace librbd { WatchCtx m_watch_ctx; uint64_t m_watch_handle; WatchState m_watch_state; + bool m_aio_ops_pending; LockOwnerState m_lock_owner_state; @@ -202,9 +203,6 @@ namespace librbd { std::map m_async_requests; std::set m_async_pending; - Mutex m_aio_request_lock; - std::vector m_aio_requests; - Mutex m_owner_client_id_lock; WatchNotify::ClientId m_owner_client_id; @@ -217,9 +215,6 @@ namespace librbd { bool release_lock(); bool try_request_lock(); - void schedule_retry_aio_requests(bool use_timer); - void retry_aio_requests(); - void schedule_cancel_async_requests(); void cancel_async_requests(); @@ -228,7 +223,10 @@ namespace librbd { void notify_release_lock(); void notify_released_lock(); + + void schedule_request_lock(bool use_timer, int timer_delay = -1); void notify_request_lock(); + int notify_lock_owner(bufferlist &bl); void schedule_async_request_timed_out(const WatchNotify::AsyncRequestId &id); diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 8b8b25b2cc391..794102b62b636 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -2694,6 +2694,10 @@ reprotect_and_return_err: ictx->data_ctx.selfmanaged_snap_set_write_ctx(ictx->snapc.seq, ictx->snaps); } // release snap_lock and cache_lock + if (ictx->image_watcher != NULL) { + ictx->image_watcher->refresh(); + } + if (new_snap) { _flush(ictx); } @@ -3021,6 +3025,7 @@ reprotect_and_return_err: << dendl; } } + ictx->image_watcher->refresh(); } return r; } @@ -3055,6 +3060,11 @@ reprotect_and_return_err: if ((r = _snap_set(ictx, ictx->snap_name.c_str())) < 0) goto err_close; + if (ictx->image_watcher != NULL) { + RWLock::RLocker owner_locker(ictx->owner_lock); + ictx->image_watcher->refresh(); + } + return 0; err_close: -- 2.39.5