Skip to content

Commit

Permalink
librbd: clean up pre-release lock handling
Browse files Browse the repository at this point in the history 
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
  • Loading branch information
Jason Dillaman committed Jan 18, 2017
1 parent a086b2c commit 58ed9a4
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 79 deletions.
28 changes: 3 additions & 25 deletions src/librbd/ExclusiveLock.cc
View file
Expand Up @@ -30,8 +30,7 @@ ExclusiveLock<I>::ExclusiveLock(I &image_ctx)
image_ctx.image_watcher, managed_lock::EXCLUSIVE,
image_ctx.blacklist_on_break_lock,
image_ctx.blacklist_expire_seconds),
m_image_ctx(image_ctx), m_pre_post_callback(nullptr),
m_shutting_down(false) {
m_image_ctx(image_ctx) {
Mutex::Locker locker(ML<I>::m_lock);
ML<I>::set_state_uninitialized();
}
Expand Down Expand Up @@ -249,34 +248,13 @@ void ExclusiveLock<I>::pre_release_lock_handler(bool shutting_down,
ldout(m_image_ctx.cct, 10) << dendl;
Mutex::Locker locker(ML<I>::m_lock);

m_shutting_down = shutting_down;

using EL = ExclusiveLock<I>;
PreReleaseRequest<I> *req = PreReleaseRequest<I>::create(m_image_ctx,
util::create_context_callback<EL, &EL::handle_pre_releasing_lock>(this),
on_finish, shutting_down);

PreReleaseRequest<I> *req = PreReleaseRequest<I>::create(
m_image_ctx, shutting_down, on_finish);
m_image_ctx.op_work_queue->queue(new FunctionContext([req](int r) {
req->send();
}));
}

template <typename I>
void ExclusiveLock<I>::handle_pre_releasing_lock(int r) {
ldout(m_image_ctx.cct, 10) << dendl;

Mutex::Locker locker(ML<I>::m_lock);

assert(r == 0);

// all IO and ops should be blocked/canceled by this point
if (!m_shutting_down) {
ML<I>::set_state_releasing();
} else {
ML<I>::set_state_shutting_down();
}
}

template <typename I>
void ExclusiveLock<I>::post_release_lock_handler(bool shutting_down, int r,
Context *on_finish) {
Expand Down
4 changes: 1 addition & 3 deletions src/librbd/ExclusiveLock.h
View file
Expand Up @@ -92,8 +92,7 @@ class ExclusiveLock : public ManagedLock<ImageCtxT> {
};

ImageCtxT& m_image_ctx;
Context *m_pre_post_callback;
bool m_shutting_down;
Context *m_pre_post_callback = nullptr;

uint32_t m_request_blocked_count = 0;
int m_request_blocked_ret_val = 0;
Expand All @@ -103,7 +102,6 @@ class ExclusiveLock : public ManagedLock<ImageCtxT> {
void handle_init_complete();
void handle_post_acquiring_lock(int r);
void handle_post_acquired_lock(int r);
void handle_pre_releasing_lock(int r);
};

} // namespace librbd
Expand Down
13 changes: 9 additions & 4 deletions src/librbd/ManagedLock.cc
View file
Expand Up @@ -240,10 +240,6 @@ void ManagedLock<I>::post_acquire_lock_handler(int r, Context *on_finish) {
template <typename I>
void ManagedLock<I>::pre_release_lock_handler(bool shutting_down,
Context *on_finish) {
{
Mutex::Locker locker(m_lock);
m_state = shutting_down ? STATE_SHUTTING_DOWN : STATE_RELEASING;
}
on_finish->complete(0);
}

Expand Down Expand Up @@ -565,6 +561,12 @@ template <typename I>
void ManagedLock<I>::handle_pre_release_lock(int r) {
ldout(m_cct, 10) << ": r=" << r << dendl;

{
Mutex::Locker locker(m_lock);
assert(m_state == STATE_PRE_RELEASING);
m_state = STATE_RELEASING;
}

if (r < 0) {
handle_release_lock(r);
return;
Expand Down Expand Up @@ -654,6 +656,9 @@ void ManagedLock<I>::handle_shutdown_pre_release(int r) {
{
Mutex::Locker locker(m_lock);
cookie = m_cookie;

assert(m_state == STATE_PRE_SHUTTING_DOWN);
m_state = STATE_SHUTTING_DOWN;
}

using managed_lock::ReleaseRequest;
Expand Down
10 changes: 0 additions & 10 deletions src/librbd/ManagedLock.h
View file
Expand Up @@ -89,16 +89,6 @@ class ManagedLock {
assert(m_state == STATE_ACQUIRING);
m_state = STATE_POST_ACQUIRING;
}
inline void set_state_releasing() {
assert(m_lock.is_locked());
assert(m_state == STATE_PRE_RELEASING);
m_state = STATE_RELEASING;
}
inline void set_state_shutting_down() {
assert(m_lock.is_locked());
assert(m_state == STATE_PRE_SHUTTING_DOWN);
m_state = STATE_SHUTTING_DOWN;
}

bool is_state_shutdown() const;
inline bool is_state_acquiring() const {
Expand Down
21 changes: 6 additions & 15 deletions src/librbd/exclusive_lock/PreReleaseRequest.cc
View file
Expand Up @@ -24,17 +24,15 @@ using util::create_context_callback;

template <typename I>
PreReleaseRequest<I>* PreReleaseRequest<I>::create(I &image_ctx,
Context *on_releasing,
Context *on_finish,
bool shutting_down) {
return new PreReleaseRequest(image_ctx, on_releasing, on_finish,
shutting_down);
bool shutting_down,
Context *on_finish) {
return new PreReleaseRequest(image_ctx, shutting_down, on_finish);
}

template <typename I>
PreReleaseRequest<I>::PreReleaseRequest(I &image_ctx, Context *on_releasing,
Context *on_finish, bool shutting_down)
: m_image_ctx(image_ctx), m_on_releasing(on_releasing),
PreReleaseRequest<I>::PreReleaseRequest(I &image_ctx, bool shutting_down,
Context *on_finish)
: m_image_ctx(image_ctx),
m_on_finish(create_async_context_callback(image_ctx, on_finish)),
m_shutting_down(shutting_down), m_error_result(0), m_object_map(nullptr),
m_journal(nullptr) {
Expand All @@ -45,7 +43,6 @@ PreReleaseRequest<I>::~PreReleaseRequest() {
if (!m_shutting_down) {
m_image_ctx.state->handle_prepare_lock_complete();
}
delete m_on_releasing;
}

template <typename I>
Expand Down Expand Up @@ -273,12 +270,6 @@ void PreReleaseRequest<I>::send_unlock() {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << dendl;

if (m_on_releasing != nullptr) {
// alert caller that we no longer own the exclusive lock
m_on_releasing->complete(0);
m_on_releasing = nullptr;
}

finish();
}

Expand Down
9 changes: 4 additions & 5 deletions src/librbd/exclusive_lock/PreReleaseRequest.h
View file
Expand Up @@ -18,8 +18,8 @@ namespace exclusive_lock {
template <typename ImageCtxT = ImageCtx>
class PreReleaseRequest {
public:
static PreReleaseRequest* create(ImageCtxT &image_ctx, Context *on_releasing,
Context *on_finish, bool shutting_down);
static PreReleaseRequest* create(ImageCtxT &image_ctx, bool shutting_down,
Context *on_finish);

~PreReleaseRequest();
void send();
Expand Down Expand Up @@ -57,11 +57,10 @@ class PreReleaseRequest {
* @endverbatim
*/

PreReleaseRequest(ImageCtxT &image_ctx, Context *on_releasing,
Context *on_finish, bool shutting_down);
PreReleaseRequest(ImageCtxT &image_ctx, bool shutting_down,
Context *on_finish);

ImageCtxT &m_image_ctx;
Context *m_on_releasing;
Context *m_on_finish;
bool m_shutting_down;

Expand Down
19 changes: 6 additions & 13 deletions src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc
View file
Expand Up @@ -145,13 +145,11 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, Success) {
mock_image_ctx.object_map = mock_object_map;
expect_close_object_map(mock_image_ctx, *mock_object_map);

MockContext mock_releasing_ctx;
expect_complete_context(mock_releasing_ctx, 0);
expect_handle_prepare_lock_complete(mock_image_ctx);

C_SaferCond ctx;
MockPreReleaseRequest *req = MockPreReleaseRequest::create(
mock_image_ctx, &mock_releasing_ctx, &ctx, false);
mock_image_ctx, false, &ctx);
req->send();
ASSERT_EQ(0, ctx.wait());
}
Expand Down Expand Up @@ -179,12 +177,10 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, SuccessJournalDisabled) {

expect_handle_prepare_lock_complete(mock_image_ctx);

C_SaferCond release_ctx;
C_SaferCond ctx;
MockPreReleaseRequest *req = MockPreReleaseRequest::create(
mock_image_ctx, &release_ctx, &ctx, false);
mock_image_ctx, false, &ctx);
req->send();
ASSERT_EQ(0, release_ctx.wait());
ASSERT_EQ(0, ctx.wait());
}

Expand All @@ -207,9 +203,8 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, SuccessObjectMapDisabled) {
C_SaferCond release_ctx;
C_SaferCond ctx;
MockPreReleaseRequest *req = MockPreReleaseRequest::create(
mock_image_ctx, &release_ctx, &ctx, true);
mock_image_ctx, true, &ctx);
req->send();
ASSERT_EQ(0, release_ctx.wait());
ASSERT_EQ(0, ctx.wait());
}

Expand Down Expand Up @@ -240,13 +235,11 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, Blacklisted) {
mock_image_ctx.object_map = mock_object_map;
expect_close_object_map(mock_image_ctx, *mock_object_map);

MockContext mock_releasing_ctx;
expect_complete_context(mock_releasing_ctx, 0);
expect_handle_prepare_lock_complete(mock_image_ctx);

C_SaferCond ctx;
MockPreReleaseRequest *req = MockPreReleaseRequest::create(
mock_image_ctx, &mock_releasing_ctx, &ctx, false);
mock_image_ctx, false, &ctx);
req->send();
ASSERT_EQ(0, ctx.wait());
}
Expand All @@ -268,7 +261,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, BlockWritesError) {

C_SaferCond ctx;
MockPreReleaseRequest *req = MockPreReleaseRequest::create(
mock_image_ctx, nullptr, &ctx, true);
mock_image_ctx, true, &ctx);
req->send();
ASSERT_EQ(-EINVAL, ctx.wait());
}
Expand All @@ -291,7 +284,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, UnlockError) {

C_SaferCond ctx;
MockPreReleaseRequest *req = MockPreReleaseRequest::create(
mock_image_ctx, nullptr, &ctx, true);
mock_image_ctx, true, &ctx);
req->send();
ASSERT_EQ(0, ctx.wait());
}
Expand Down
10 changes: 6 additions & 4 deletions src/test/librbd/test_mock_ExclusiveLock.cc
View file
Expand Up @@ -63,8 +63,6 @@ struct ManagedLock<MockExclusiveLockImageCtx> {
MOCK_METHOD0(set_state_unlocked, void());
MOCK_METHOD0(set_state_waiting_for_lock, void());
MOCK_METHOD0(set_state_post_acquiring, void());
MOCK_METHOD0(set_state_releasing, void());
MOCK_METHOD0(set_state_shutting_down, void());

MOCK_CONST_METHOD0(is_state_shutdown, bool());
MOCK_CONST_METHOD0(is_state_acquiring, bool());
Expand All @@ -90,8 +88,7 @@ struct BaseRequest {
Context *on_finish = nullptr;

static T* create(MockExclusiveLockImageCtx &image_ctx,
Context *on_lock_unlock, Context *on_finish,
bool shutting_down = false) {
Context *on_lock_unlock, Context *on_finish) {
assert(!s_requests.empty());
T* req = s_requests.front();
req->on_lock_unlock = on_lock_unlock;
Expand Down Expand Up @@ -124,6 +121,11 @@ struct PostAcquireRequest<MockExclusiveLockImageCtx> : public BaseRequest<PostAc

template <>
struct PreReleaseRequest<MockExclusiveLockImageCtx> : public BaseRequest<PreReleaseRequest<MockExclusiveLockImageCtx> > {
static PreReleaseRequest<MockExclusiveLockImageCtx> *create(
MockExclusiveLockImageCtx &image_ctx, bool shutting_down,
Context *on_finish) {
return BaseRequest::create(image_ctx, nullptr, on_finish);
}
MOCK_METHOD0(send, void());
};

Expand Down

0 comments on commit 58ed9a4

Please sign in to comment.