From: Jason Dillaman Date: Tue, 16 Feb 2016 14:07:57 +0000 (-0500) Subject: librbd: journal replay should execute ops in clean context X-Git-Tag: v10.1.0~357^2~6 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=e01b406762d1ac27a2912a5d5da663633d3512ec;p=ceph.git librbd: journal replay should execute ops in clean context lockdep will complain about loop cycles that won't cause an issue in reality as replay and record are two different journal states. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index 0db4bb3da6c..bd6e081d4df 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -8,6 +8,8 @@ #include "librbd/AioCompletion.h" #include "librbd/AioImageRequest.h" #include "librbd/ImageCtx.h" +#include "librbd/ImageState.h" +#include "librbd/ImageWatcher.h" #include "librbd/internal.h" #include "librbd/Operations.h" #include "librbd/Utils.h" @@ -26,6 +28,32 @@ static const uint64_t IN_FLIGHT_IO_HIGH_WATER_MARK(64); static NoOpProgressContext no_op_progress_callback; +template +struct ExecuteOp : public Context { + I &image_ctx; + E event; + Context *on_op_complete; + + ExecuteOp(I &image_ctx, const E &event, Context *on_op_complete) + : image_ctx(image_ctx), event(event), on_op_complete(on_op_complete) { + } + + void execute(const journal::SnapCreateEvent &_) { + image_ctx.operations->snap_create(event.snap_name.c_str(), on_op_complete, + event.op_tid); + } + + void execute(const journal::ResizeEvent &_) { + image_ctx.operations->resize(event.size, no_op_progress_callback, + on_op_complete, event.op_tid); + } + + virtual void finish(int r) override { + RWLock::RLocker owner_locker(image_ctx.owner_lock); + execute(event); + } +}; + } // anonymous namespace template @@ -254,8 +282,10 @@ void Replay::handle_event(const journal::SnapCreateEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); - m_image_ctx.operations->snap_create(event.snap_name.c_str(), on_op_complete, - event.op_tid); + // avoid lock cycles + m_image_ctx.op_work_queue->queue( + new ExecuteOp(m_image_ctx, event, + on_op_complete), 0); // do not process more events until the state machine is ready // since it will affect IO @@ -391,8 +421,10 @@ void Replay::handle_event(const journal::ResizeEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); - m_image_ctx.operations->resize(event.size, no_op_progress_callback, - on_op_complete, event.op_tid); + // avoid lock cycles + m_image_ctx.op_work_queue->queue( + new ExecuteOp(m_image_ctx, event, + on_op_complete), 0); // do not process more events until the state machine is ready // since it will affect IO diff --git a/src/librbd/operation/Request.cc b/src/librbd/operation/Request.cc index d3e8bf6ed89..216da1a983a 100644 --- a/src/librbd/operation/Request.cc +++ b/src/librbd/operation/Request.cc @@ -4,6 +4,7 @@ #include "librbd/operation/Request.h" #include "common/dout.h" #include "common/errno.h" +#include "common/WorkQueue.h" #include "librbd/ImageCtx.h" #include "librbd/Journal.h" #include "librbd/Utils.h" @@ -83,6 +84,18 @@ void Request::commit_op_event(int r) { } } +template +void Request::replay_op_ready(Context *on_safe) { + I &image_ctx = this->m_image_ctx; + assert(image_ctx.owner_lock.is_locked()); + assert(image_ctx.snap_lock.is_locked()); + assert(m_op_tid != 0); + + m_appended_op_event = true; + image_ctx.journal->replay_op_ready( + m_op_tid, util::create_async_context_callback(image_ctx, on_safe)); +} + template void Request::append_op_event(Context *on_safe) { I &image_ctx = this->m_image_ctx; diff --git a/src/librbd/operation/Request.h b/src/librbd/operation/Request.h index 44ff5e2eab2..be4d174be5d 100644 --- a/src/librbd/operation/Request.h +++ b/src/librbd/operation/Request.h @@ -44,9 +44,7 @@ protected: if (image_ctx.journal != NULL) { Context *ctx = util::create_context_callback(request); if (image_ctx.journal->is_journal_replaying()) { - assert(m_op_tid != 0); - m_appended_op_event = true; - image_ctx.journal->replay_op_ready(m_op_tid, ctx); + replay_op_ready(ctx); } else { append_op_event(ctx); } @@ -83,6 +81,7 @@ private: bool m_appended_op_event = false; bool m_committed_op_event = false; + void replay_op_ready(Context *on_safe); void append_op_event(Context *on_safe); void handle_op_event_safe(int r); diff --git a/src/test/librbd/journal/test_Replay.cc b/src/test/librbd/journal/test_Replay.cc index be121cff90a..93aeddcbd43 100644 --- a/src/test/librbd/journal/test_Replay.cc +++ b/src/test/librbd/journal/test_Replay.cc @@ -12,7 +12,9 @@ #include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" +#include "librbd/internal.h" #include "librbd/Journal.h" +#include "librbd/Operations.h" #include "librbd/journal/Types.h" void register_test_journal_replay() { @@ -153,6 +155,12 @@ TEST_F(TestJournalReplay, AioDiscardEvent) { ASSERT_EQ(0, when_acquired_lock(ictx)); get_journal_commit_position(ictx, ¤t); ASSERT_EQ(current, initial + 3); + + // verify lock ordering constraints + aio_comp = new librbd::AioCompletion(); + ictx->aio_work_queue->aio_discard(aio_comp, 0, read_payload.size()); + ASSERT_EQ(0, aio_comp->wait_for_complete()); + aio_comp->release(); } TEST_F(TestJournalReplay, AioWriteEvent) { @@ -199,6 +207,13 @@ TEST_F(TestJournalReplay, AioWriteEvent) { ASSERT_EQ(0, when_acquired_lock(ictx)); get_journal_commit_position(ictx, ¤t); ASSERT_EQ(current, initial + 3); + + // verify lock ordering constraints + aio_comp = new librbd::AioCompletion(); + ictx->aio_work_queue->aio_write(aio_comp, 0, payload.size(), payload.c_str(), + 0); + ASSERT_EQ(0, aio_comp->wait_for_complete()); + aio_comp->release(); } TEST_F(TestJournalReplay, AioFlushEvent) { @@ -257,6 +272,331 @@ TEST_F(TestJournalReplay, AioFlushEvent) { ASSERT_EQ(0, when_acquired_lock(ictx)); get_journal_commit_position(ictx, ¤t); ASSERT_EQ(current, initial + 3); + + // verify lock ordering constraints + aio_comp = new librbd::AioCompletion(); + ictx->aio_work_queue->aio_flush(aio_comp); + ASSERT_EQ(0, aio_comp->wait_for_complete()); + aio_comp->release(); +} + +TEST_F(TestJournalReplay, SnapCreate) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapCreateEvent(1, "snap")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + { + RWLock::RLocker snap_locker(ictx->snap_lock); + ASSERT_NE(CEPH_NOSNAP, ictx->get_snap_id("snap")); + } + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->snap_create("snap2")); +} + +TEST_F(TestJournalReplay, SnapProtect) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapProtectEvent(1, "snap")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + bool is_protected; + ASSERT_EQ(0, librbd::snap_is_protected(ictx, "snap", &is_protected)); + ASSERT_TRUE(is_protected); + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->snap_create("snap2")); + ASSERT_EQ(0, ictx->operations->snap_protect("snap2")); +} + +TEST_F(TestJournalReplay, SnapUnprotect) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + uint64_t snap_id; + { + RWLock::RLocker snap_locker(ictx->snap_lock); + snap_id = ictx->get_snap_id("snap"); + ASSERT_NE(CEPH_NOSNAP, snap_id); + } + ASSERT_EQ(0, ictx->operations->snap_protect("snap")); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapUnprotectEvent(1, "snap")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + bool is_protected; + ASSERT_EQ(0, librbd::snap_is_protected(ictx, "snap", &is_protected)); + ASSERT_FALSE(is_protected); + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->snap_create("snap2")); + ASSERT_EQ(0, ictx->operations->snap_protect("snap2")); + ASSERT_EQ(0, ictx->operations->snap_unprotect("snap2")); +} + +TEST_F(TestJournalReplay, SnapRename) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + uint64_t snap_id; + { + RWLock::RLocker snap_locker(ictx->snap_lock); + snap_id = ictx->get_snap_id("snap"); + ASSERT_NE(CEPH_NOSNAP, snap_id); + } + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapRenameEvent(1, snap_id, "snap2")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + { + RWLock::RLocker snap_locker(ictx->snap_lock); + snap_id = ictx->get_snap_id("snap2"); + ASSERT_NE(CEPH_NOSNAP, snap_id); + } + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->snap_rename("snap2", "snap3")); +} + +TEST_F(TestJournalReplay, SnapRollback) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapRollbackEvent(1, "snap")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + // verify lock ordering constraints + librbd::NoOpProgressContext no_op_progress; + ASSERT_EQ(0, ictx->operations->snap_rollback("snap", no_op_progress)); +} + +TEST_F(TestJournalReplay, SnapRemove) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapRemoveEvent(1, "snap")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + { + RWLock::RLocker snap_locker(ictx->snap_lock); + uint64_t snap_id = ictx->get_snap_id("snap"); + ASSERT_EQ(CEPH_NOSNAP, snap_id); + } + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + ASSERT_EQ(0, ictx->operations->snap_remove("snap")); +} + +TEST_F(TestJournalReplay, Rename) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + std::string new_image_name(get_temp_image_name()); + inject_into_journal(ictx, librbd::journal::RenameEvent(1, new_image_name)); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + // verify lock ordering constraints + librbd::RBD rbd; + ASSERT_EQ(0, rbd.rename(m_ioctx, new_image_name.c_str(), m_image_name.c_str())); +} + +TEST_F(TestJournalReplay, Resize) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::ResizeEvent(1, 16)); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + // verify lock ordering constraints + librbd::NoOpProgressContext no_op_progress; + ASSERT_EQ(0, ictx->operations->resize(0, no_op_progress)); +} + +TEST_F(TestJournalReplay, Flatten) { + REQUIRE_FEATURE(RBD_FEATURE_LAYERING | RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + ASSERT_EQ(0, ictx->operations->snap_protect("snap")); + + std::string clone_name = get_temp_image_name(); + int order = ictx->order; + ASSERT_EQ(0, librbd::clone(m_ioctx, m_image_name.c_str(), "snap", m_ioctx, + clone_name.c_str(), ictx->features, &order, 0, 0)); + + librbd::ImageCtx *ictx2; + ASSERT_EQ(0, open_image(clone_name, &ictx2)); + ASSERT_EQ(0, when_acquired_lock(ictx2)); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx2, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx2, librbd::journal::FlattenEvent(1)); + inject_into_journal(ictx2, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(clone_name, &ictx2)); + ASSERT_EQ(0, when_acquired_lock(ictx2)); + + int64_t current; + get_journal_commit_position(ictx2, ¤t); + ASSERT_EQ(current, initial + 2); + ASSERT_EQ(0, ictx->operations->snap_unprotect("snap")); + + // verify lock ordering constraints + librbd::NoOpProgressContext no_op; + ASSERT_EQ(-EINVAL, ictx2->operations->flatten(no_op)); } TEST_F(TestJournalReplay, EntryPosition) {