From: Jason Dillaman Date: Thu, 28 Jan 2016 19:38:20 +0000 (-0500) Subject: librbd: ImageWatcher shouldn't block the notification thread X-Git-Tag: v9.2.1~2^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F7406%2Fhead;p=ceph.git librbd: ImageWatcher shouldn't block the notification thread Blocking the notification thread will also result in librados async callbacks becoming blocked (since they use the same thread). Signed-off-by: Jason Dillaman (cherry picked from commit 6f94bde44500cc4592ac9a842cbb150b8cabf96b) Conflicts: src/librbd/ImageWatcher.[cc|h]: fewer RPC messages and synchronous snapshot actions --- diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index 43644f8854cd..7d3ddd2a3e4d 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -790,24 +790,25 @@ void ImageWatcher::cleanup_async_request(const AsyncRequestId& async_request_id, m_async_pending.erase(async_request_id); } -void ImageWatcher::handle_payload(const HeaderUpdatePayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const HeaderUpdatePayload &payload, + C_NotifyAck *ack_ctx) { ldout(m_image_ctx.cct, 10) << this << " image header updated" << dendl; Mutex::Locker lictx(m_image_ctx.refresh_lock); ++m_image_ctx.refresh_seq; m_image_ctx.perfcounter->inc(l_librbd_notify); + return true; } -void ImageWatcher::handle_payload(const AcquiredLockPayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const AcquiredLockPayload &payload, + C_NotifyAck *ack_ctx) { ldout(m_image_ctx.cct, 10) << this << " image exclusively locked announcement" << dendl; if (payload.client_id.is_valid()) { Mutex::Locker l(m_owner_client_id_lock); if (payload.client_id == m_owner_client_id) { // we already know that the remote client is the owner - return; + return true; } set_owner_client_id(payload.client_id); } @@ -817,10 +818,11 @@ void ImageWatcher::handle_payload(const AcquiredLockPayload &payload, schedule_cancel_async_requests(); schedule_retry_aio_requests(false); } + return true; } -void ImageWatcher::handle_payload(const ReleasedLockPayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const ReleasedLockPayload &payload, + C_NotifyAck *ack_ctx) { ldout(m_image_ctx.cct, 10) << this << " exclusive lock released" << dendl; if (payload.client_id.is_valid()) { Mutex::Locker l(m_owner_client_id_lock); @@ -828,7 +830,7 @@ void ImageWatcher::handle_payload(const ReleasedLockPayload &payload, ldout(m_image_ctx.cct, 10) << this << " unexpected owner: " << payload.client_id << " != " << m_owner_client_id << dendl; - return; + return true; } set_owner_client_id(ClientId()); } @@ -838,24 +840,25 @@ void ImageWatcher::handle_payload(const ReleasedLockPayload &payload, schedule_cancel_async_requests(); schedule_retry_aio_requests(false); } + return true; } -void ImageWatcher::handle_payload(const RequestLockPayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const RequestLockPayload &payload, + C_NotifyAck *ack_ctx) { ldout(m_image_ctx.cct, 10) << this << " exclusive lock requested" << dendl; if (payload.client_id == get_client_id()) { - return; + return true; } RWLock::RLocker l(m_image_ctx.owner_lock); if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { // need to send something back so the client can detect a missing leader - ::encode(ResponseMessage(0), *out); + ::encode(ResponseMessage(0), ack_ctx->out); { Mutex::Locker l(m_owner_client_id_lock); if (!m_owner_client_id.is_valid()) { - return; + return true; } } @@ -865,10 +868,11 @@ void ImageWatcher::handle_payload(const RequestLockPayload &payload, boost::bind(&ImageWatcher::notify_release_lock, this)); m_task_finisher->queue(TASK_CODE_RELEASING_LOCK, ctx); } + return true; } -void ImageWatcher::handle_payload(const AsyncProgressPayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const AsyncProgressPayload &payload, + C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_async_request_lock); std::map::iterator req_it = m_async_requests.find(payload.async_request_id); @@ -880,10 +884,11 @@ void ImageWatcher::handle_payload(const AsyncProgressPayload &payload, schedule_async_request_timed_out(payload.async_request_id); req_it->second.second->update_progress(payload.offset, payload.total); } + return true; } -void ImageWatcher::handle_payload(const AsyncCompletePayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const AsyncCompletePayload &payload, + C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_async_request_lock); std::map::iterator req_it = m_async_requests.find(payload.async_request_id); @@ -893,10 +898,11 @@ void ImageWatcher::handle_payload(const AsyncCompletePayload &payload, << payload.result << dendl; req_it->second.first->complete(payload.result); } + return true; } -void ImageWatcher::handle_payload(const FlattenPayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const FlattenPayload &payload, + C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { @@ -916,12 +922,13 @@ void ImageWatcher::handle_payload(const FlattenPayload &payload, } } - ::encode(ResponseMessage(r), *out); + ::encode(ResponseMessage(r), ack_ctx->out); } + return true; } -void ImageWatcher::handle_payload(const ResizePayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const ResizePayload &payload, + C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { bool new_request; @@ -941,38 +948,43 @@ void ImageWatcher::handle_payload(const ResizePayload &payload, } } - ::encode(ResponseMessage(r), *out); + ::encode(ResponseMessage(r), ack_ctx->out); } + return true; } -void ImageWatcher::handle_payload(const SnapCreatePayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const SnapCreatePayload &payload, + C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { ldout(m_image_ctx.cct, 10) << this << " remote snap_create request: " << payload.snap_name << dendl; - int r = librbd::snap_create_helper(&m_image_ctx, NULL, - payload.snap_name.c_str()); - ::encode(ResponseMessage(r), *out); + // execute outside of librados AIO thread + m_image_ctx.op_work_queue->queue(new C_SnapCreateResponseMessage( + this, ack_ctx, payload.snap_name), 0); + return false; } + return true; } -void ImageWatcher::handle_payload(const SnapRemovePayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const SnapRemovePayload &payload, + C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { ldout(m_image_ctx.cct, 10) << this << " remote snap_remove request: " << payload.snap_name << dendl; - int r = librbd::snap_remove_helper(&m_image_ctx, NULL, - payload.snap_name.c_str()); - ::encode(ResponseMessage(r), *out); + // execute outside of librados AIO thread + m_image_ctx.op_work_queue->queue(new C_SnapRemoveResponseMessage( + this, ack_ctx, payload.snap_name), 0); + return false; } + return true; } -void ImageWatcher::handle_payload(const RebuildObjectMapPayload& payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const RebuildObjectMapPayload& payload, + C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { bool new_request; @@ -993,16 +1005,18 @@ void ImageWatcher::handle_payload(const RebuildObjectMapPayload& payload, } } - ::encode(ResponseMessage(0), *out); + ::encode(ResponseMessage(0), ack_ctx->out); } + return true; } -void ImageWatcher::handle_payload(const UnknownPayload &payload, - bufferlist *out) { +bool ImageWatcher::handle_payload(const UnknownPayload &payload, + C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); if (is_lock_owner()) { - ::encode(ResponseMessage(-EOPNOTSUPP), *out); + ::encode(ResponseMessage(-EOPNOTSUPP), ack_ctx->out); } + return true; } void ImageWatcher::handle_notify(uint64_t notify_id, uint64_t handle, @@ -1118,4 +1132,43 @@ void ImageWatcher::RemoteContext::finish(int r) { m_image_watcher.schedule_async_complete(m_async_request_id, r); } +ImageWatcher::C_NotifyAck::C_NotifyAck(ImageWatcher *image_watcher, + uint64_t notify_id, uint64_t handle) + : image_watcher(image_watcher), notify_id(notify_id), handle(handle) { + CephContext *cct = image_watcher->m_image_ctx.cct; + ldout(cct, 10) << this << " C_NotifyAck start: id=" << notify_id << ", " + << "handle=" << handle << dendl; +} + +void ImageWatcher::C_NotifyAck::finish(int r) { + assert(r == 0); + CephContext *cct = image_watcher->m_image_ctx.cct; + ldout(cct, 10) << this << " C_NotifyAck finish: id=" << notify_id << ", " + << "handle=" << handle << dendl; + + image_watcher->acknowledge_notify(notify_id, handle, out); } + +void ImageWatcher::C_ResponseMessage::finish(int r) { + CephContext *cct = notify_ack->image_watcher->m_image_ctx.cct; + ldout(cct, 10) << this << " C_ResponseMessage: r=" << r << dendl; + + RWLock::RLocker owner_locker(image_watcher->m_image_ctx.owner_lock); + if (image_watcher->m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + r = execute(); + ::encode(ResponseMessage(r), notify_ack->out); + } + notify_ack->complete(0); +} + +int ImageWatcher::C_SnapCreateResponseMessage::execute() { + return librbd::snap_create_helper(&image_watcher->m_image_ctx, NULL, + snap_name.c_str()); +} + +int ImageWatcher::C_SnapRemoveResponseMessage::execute() { + return librbd::snap_remove_helper(&image_watcher->m_image_ctx, NULL, + snap_name.c_str()); +} + +} // namespace librbd diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h index 6ebeb9dfcb84..8964464676d0 100644 --- a/src/librbd/ImageWatcher.h +++ b/src/librbd/ImageWatcher.h @@ -161,6 +161,52 @@ namespace librbd { ProgressContext *m_prog_ctx; }; + struct C_NotifyAck : public Context { + ImageWatcher *image_watcher; + uint64_t notify_id; + uint64_t handle; + bufferlist out; + + C_NotifyAck(ImageWatcher *image_watcher, uint64_t notify_id, + uint64_t handle); + virtual void finish(int r); + }; + + struct C_ResponseMessage : public Context { + ImageWatcher *image_watcher; + C_NotifyAck *notify_ack; + + C_ResponseMessage(ImageWatcher *image_watcher, C_NotifyAck *notify_ack) + : image_watcher(image_watcher), notify_ack(notify_ack) { + } + virtual void finish(int r); + virtual int execute() = 0; + }; + + struct C_SnapCreateResponseMessage : public C_ResponseMessage { + std::string snap_name; + + C_SnapCreateResponseMessage(ImageWatcher *image_watcher, + C_NotifyAck *notify_ack, + const std::string &snap_name) + : C_ResponseMessage(image_watcher, notify_ack), + snap_name(snap_name) { + } + virtual int execute(); + }; + + struct C_SnapRemoveResponseMessage : public C_ResponseMessage { + std::string snap_name; + + C_SnapRemoveResponseMessage(ImageWatcher *image_watcher, + C_NotifyAck *notify_ack, + const std::string &snap_name) + : C_ResponseMessage(image_watcher, notify_ack), + snap_name(snap_name) { + } + virtual int execute(); + }; + struct HandlePayloadVisitor : public boost::static_visitor { ImageWatcher *image_watcher; uint64_t notify_id; @@ -172,17 +218,12 @@ namespace librbd { { } - inline void operator()(const WatchNotify::HeaderUpdatePayload &payload) const { - bufferlist out; - image_watcher->handle_payload(payload, &out); - image_watcher->acknowledge_notify(notify_id, handle, out); - } - template inline void operator()(const Payload &payload) const { - bufferlist out; - image_watcher->handle_payload(payload, &out); - image_watcher->acknowledge_notify(notify_id, handle, out); + C_NotifyAck *ctx = new C_NotifyAck(image_watcher, notify_id, handle); + if (image_watcher->handle_payload(payload, ctx)) { + ctx->complete(0); + } } }; @@ -251,30 +292,30 @@ namespace librbd { void cleanup_async_request(const WatchNotify::AsyncRequestId& id, Context *ctx); - void handle_payload(const WatchNotify::HeaderUpdatePayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::AcquiredLockPayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::ReleasedLockPayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::RequestLockPayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::AsyncProgressPayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::AsyncCompletePayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::FlattenPayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::ResizePayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::SnapCreatePayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::SnapRemovePayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::RebuildObjectMapPayload& payload, - bufferlist *out); - void handle_payload(const WatchNotify::UnknownPayload& payload, - bufferlist *out); + bool handle_payload(const WatchNotify::HeaderUpdatePayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::AcquiredLockPayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::ReleasedLockPayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::RequestLockPayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::AsyncProgressPayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::AsyncCompletePayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::FlattenPayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::ResizePayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::SnapCreatePayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::SnapRemovePayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::RebuildObjectMapPayload& payload, + C_NotifyAck *ctx); + bool handle_payload(const WatchNotify::UnknownPayload& payload, + C_NotifyAck *ctx); void handle_notify(uint64_t notify_id, uint64_t handle, bufferlist &bl); void handle_error(uint64_t cookie, int err);