Merge pull request #10432 from dillaman/wip-16708

rbd-mirror: potential IO stall when using asok flush request

Reviewed-by: Mykola Golub <mgolub@mirantis.com>
This commit is contained in:
Mykola Golub 2016-07-30 19:59:49 +03:00 committed by GitHub
commit 77ec14a95b
12 changed files with 70 additions and 50 deletions

View File

@ -50,7 +50,7 @@ namespace librbd {
* context or via a thread pool context for cache read hits).
*/
struct AioCompletion {
Mutex lock;
mutable Mutex lock;
Cond cond;
aio_state_t state;
ssize_t rval;
@ -100,6 +100,15 @@ namespace librbd {
return comp;
}
template <typename T, void (T::*MF)(int) = &T::complete>
static AioCompletion *create_and_start(T *obj, ImageCtx *image_ctx,
aio_type_t type) {
AioCompletion *comp = create<T, MF>(obj);
comp->init_time(image_ctx, type);
comp->start_op();
return comp;
}
AioCompletion() : lock("AioCompletion::lock", true, false),
state(STATE_PENDING), rval(0), complete_cb(NULL),
complete_arg(NULL), rbd_comp(NULL),
@ -117,6 +126,15 @@ namespace librbd {
void finalize(ssize_t rval);
inline bool is_initialized(aio_type_t type) const {
Mutex::Locker locker(lock);
return ((ictx != nullptr) && (aio_type == type));
}
inline bool is_started() const {
Mutex::Locker locker(lock);
return async_op.started();
}
void init_time(ImageCtx *i, aio_type_t t);
void start_op(bool ignore_type = false);
void fail(int r);

View File

@ -79,10 +79,7 @@ void AioImageRequest<I>::aio_read(
I *ictx, AioCompletion *c,
const std::vector<std::pair<uint64_t,uint64_t> > &extents,
char *buf, bufferlist *pbl, int op_flags) {
c->init_time(ictx, librbd::AIO_TYPE_READ);
AioImageRead req(*ictx, c, extents, buf, pbl, op_flags);
req.start_op();
req.send();
}
@ -90,10 +87,7 @@ template <typename I>
void AioImageRequest<I>::aio_read(I *ictx, AioCompletion *c,
uint64_t off, size_t len, char *buf,
bufferlist *pbl, int op_flags) {
c->init_time(ictx, librbd::AIO_TYPE_READ);
AioImageRead req(*ictx, c, off, len, buf, pbl, op_flags);
req.start_op();
req.send();
}
@ -101,35 +95,29 @@ template <typename I>
void AioImageRequest<I>::aio_write(I *ictx, AioCompletion *c,
uint64_t off, size_t len, const char *buf,
int op_flags) {
c->init_time(ictx, librbd::AIO_TYPE_WRITE);
AioImageWrite req(*ictx, c, off, len, buf, op_flags);
req.start_op();
req.send();
}
template <typename I>
void AioImageRequest<I>::aio_discard(I *ictx, AioCompletion *c,
uint64_t off, uint64_t len) {
c->init_time(ictx, librbd::AIO_TYPE_DISCARD);
AioImageDiscard req(*ictx, c, off, len);
req.start_op();
req.send();
}
template <typename I>
void AioImageRequest<I>::aio_flush(I *ictx, AioCompletion *c) {
c->init_time(ictx, librbd::AIO_TYPE_FLUSH);
assert(c->is_initialized(AIO_TYPE_FLUSH));
AioImageFlush req(*ictx, c);
req.start_op();
req.send();
}
template <typename I>
void AioImageRequest<I>::send() {
assert(m_image_ctx.owner_lock.is_locked());
assert(m_aio_comp->is_initialized(get_aio_type()));
assert(m_aio_comp->is_started() ^ (get_aio_type() == AIO_TYPE_FLUSH));
CephContext *cct = m_image_ctx.cct;
ldout(cct, 20) << get_request_type() << ": ictx=" << &m_image_ctx << ", "

View File

@ -58,6 +58,7 @@ protected:
: m_image_ctx(image_ctx), m_aio_comp(aio_comp) {}
virtual void send_request() = 0;
virtual aio_type_t get_aio_type() const = 0;
virtual const char *get_request_type() const = 0;
};
@ -79,6 +80,9 @@ public:
protected:
virtual void send_request();
virtual aio_type_t get_aio_type() const {
return AIO_TYPE_READ;
}
virtual const char *get_request_type() const {
return "aio_read";
}
@ -145,6 +149,9 @@ public:
}
protected:
virtual aio_type_t get_aio_type() const {
return AIO_TYPE_WRITE;
}
virtual const char *get_request_type() const {
return "aio_write";
}
@ -177,6 +184,9 @@ public:
}
protected:
virtual aio_type_t get_aio_type() const {
return AIO_TYPE_DISCARD;
}
virtual const char *get_request_type() const {
return "aio_discard";
}
@ -207,6 +217,9 @@ public:
protected:
virtual void send_request();
virtual aio_type_t get_aio_type() const {
return AIO_TYPE_FLUSH;
}
virtual const char *get_request_type() const {
return "aio_flush";
}

View File

@ -122,6 +122,7 @@ void AioImageRequestWQ::aio_read(AioCompletion *c, uint64_t off, uint64_t len,
lock_required) {
queue(new AioImageRead(m_image_ctx, c, off, len, buf, pbl, op_flags));
} else {
c->start_op();
AioImageRequest<>::aio_read(&m_image_ctx, c, off, len, buf, pbl, op_flags);
finish_in_flight_op();
}
@ -148,6 +149,7 @@ void AioImageRequestWQ::aio_write(AioCompletion *c, uint64_t off, uint64_t len,
if (m_image_ctx.non_blocking_aio || writes_blocked()) {
queue(new AioImageWrite(m_image_ctx, c, off, len, buf, op_flags));
} else {
c->start_op();
AioImageRequest<>::aio_write(&m_image_ctx, c, off, len, buf, op_flags);
finish_in_flight_op();
}
@ -173,6 +175,7 @@ void AioImageRequestWQ::aio_discard(AioCompletion *c, uint64_t off,
if (m_image_ctx.non_blocking_aio || writes_blocked()) {
queue(new AioImageDiscard(m_image_ctx, c, off, len));
} else {
c->start_op();
AioImageRequest<>::aio_discard(&m_image_ctx, c, off, len);
finish_in_flight_op();
}

View File

@ -268,7 +268,8 @@ namespace librbd {
void AioObjectRead::read_from_parent(const vector<pair<uint64_t,uint64_t> >& parent_extents)
{
assert(!m_parent_completion);
m_parent_completion = AioCompletion::create<AioObjectRequest>(this);
m_parent_completion = AioCompletion::create_and_start<AioObjectRequest>(
this, m_ictx, AIO_TYPE_READ);
// prevent the parent image from being deleted while this
// request is still in-progress

View File

@ -181,7 +181,8 @@ private:
void CopyupRequest::send()
{
m_state = STATE_READ_FROM_PARENT;
AioCompletion *comp = AioCompletion::create(this);
AioCompletion *comp = AioCompletion::create_and_start(
this, m_ictx, AIO_TYPE_READ);
ldout(m_ictx->cct, 20) << __func__ << " " << this
<< ": completion " << comp

View File

@ -154,6 +154,10 @@ Context *create_async_context_callback(I &image_ctx, Context *on_finish) {
image_ctx.op_work_queue, on_finish);
}
inline ImageCtx *get_image_ctx(ImageCtx *image_ctx) {
return image_ctx;
}
} // namespace util
} // namespace librbd

View File

@ -2581,7 +2581,8 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
uint64_t len = min(period, src_size - offset);
bufferlist *bl = new bufferlist();
Context *ctx = new C_CopyRead(&throttle, dest, offset, bl);
AioCompletion *comp = AioCompletion::create(ctx);
AioCompletion *comp = AioCompletion::create_and_start(ctx, src,
AIO_TYPE_READ);
AioImageRequest<>::aio_read(src, comp, offset, len, NULL, bl,
fadvise_flags);
prog_ctx.update_progress(offset, src_size);
@ -2806,7 +2807,8 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
bufferlist bl;
C_SaferCond ctx;
AioCompletion *c = AioCompletion::create(&ctx);
AioCompletion *c = AioCompletion::create_and_start(&ctx, ictx,
AIO_TYPE_READ);
AioImageRequest<>::aio_read(ictx, c, off, read_len, NULL, &bl, 0);
int ret = ctx.wait();

View File

@ -297,6 +297,7 @@ void Replay<I>::handle_event(const journal::AioDiscardEvent &event,
bool flush_required;
AioCompletion *aio_comp = create_aio_modify_completion(on_ready, on_safe,
AIO_TYPE_DISCARD,
&flush_required);
AioImageRequest<I>::aio_discard(&m_image_ctx, aio_comp, event.offset,
event.length);
@ -318,6 +319,7 @@ void Replay<I>::handle_event(const journal::AioWriteEvent &event,
bufferlist data = event.data;
bool flush_required;
AioCompletion *aio_comp = create_aio_modify_completion(on_ready, on_safe,
AIO_TYPE_WRITE,
&flush_required);
AioImageRequest<I>::aio_write(&m_image_ctx, aio_comp, event.offset,
event.length, data.c_str(), 0);
@ -837,6 +839,7 @@ void Replay<I>::handle_op_complete(uint64_t op_tid, int r) {
template <typename I>
AioCompletion *Replay<I>::create_aio_modify_completion(Context *on_ready,
Context *on_safe,
aio_type_t aio_type,
bool *flush_required) {
Mutex::Locker locker(m_lock);
CephContext *cct = m_image_ctx.cct;
@ -871,8 +874,9 @@ AioCompletion *Replay<I>::create_aio_modify_completion(Context *on_ready,
// when the modification is ACKed by librbd, we can process the next
// event. when flushed, the completion of the next flush will fire the
// on_safe callback
AioCompletion *aio_comp = AioCompletion::create<Context>(
new C_AioModifyComplete(this, on_ready, on_safe));
AioCompletion *aio_comp = AioCompletion::create_and_start<Context>(
new C_AioModifyComplete(this, on_ready, on_safe),
util::get_image_ctx(&m_image_ctx), aio_type);
return aio_comp;
}
@ -883,9 +887,10 @@ AioCompletion *Replay<I>::create_aio_flush_completion(Context *on_safe) {
++m_in_flight_aio_flush;
// associate all prior write/discard ops to this flush request
AioCompletion *aio_comp = AioCompletion::create<Context>(
AioCompletion *aio_comp = AioCompletion::create_and_start<Context>(
new C_AioFlushComplete(this, on_safe,
std::move(m_aio_modify_unsafe_contexts)));
std::move(m_aio_modify_unsafe_contexts)),
util::get_image_ctx(&m_image_ctx), AIO_TYPE_FLUSH);
m_aio_modify_unsafe_contexts.clear();
return aio_comp;
}

View File

@ -8,6 +8,7 @@
#include "include/buffer_fwd.h"
#include "include/Context.h"
#include "common/Mutex.h"
#include "librbd/AioCompletion.h"
#include "librbd/journal/Types.h"
#include <boost/variant.hpp>
#include <list>
@ -171,6 +172,7 @@ private:
AioCompletion *create_aio_modify_completion(Context *on_ready,
Context *on_safe,
aio_type_t aio_type,
bool *flush_required);
AioCompletion *create_aio_flush_completion(Context *on_safe);
void handle_aio_completion(AioCompletion *aio_comp);

View File

@ -255,37 +255,12 @@ TEST_F(TestJournalReplay, AioFlushEvent) {
// inject a flush operation into the journal
inject_into_journal(ictx, librbd::journal::AioFlushEvent());
// start an AIO write op
librbd::Journal<> *journal = ictx->journal;
ictx->journal = NULL;
std::string payload(m_image_size, '1');
librbd::AioCompletion *aio_comp = new librbd::AioCompletion();
{
RWLock::RLocker owner_lock(ictx->owner_lock);
librbd::AioImageRequest<>::aio_write(ictx, aio_comp, 0, payload.size(),
payload.c_str(), 0);
}
ictx->journal = journal;
close_image(ictx);
// re-open the journal so that it replays the new entry
ASSERT_EQ(0, open_image(m_image_name, &ictx));
ASSERT_EQ(0, when_acquired_lock(ictx));
ASSERT_TRUE(aio_comp->is_complete());
ASSERT_EQ(0, aio_comp->wait_for_complete());
aio_comp->release();
std::string read_payload(m_image_size, '\0');
aio_comp = new librbd::AioCompletion();
ictx->aio_work_queue->aio_read(aio_comp, 0, read_payload.size(),
&read_payload[0], NULL, 0);
ASSERT_EQ(0, aio_comp->wait_for_complete());
aio_comp->release();
ASSERT_EQ(payload, read_payload);
// check the commit position is properly updated
int64_t current_tag;
int64_t current_entry;
@ -305,7 +280,7 @@ TEST_F(TestJournalReplay, AioFlushEvent) {
ASSERT_EQ(1, current_entry);
// verify lock ordering constraints
aio_comp = new librbd::AioCompletion();
librbd::AioCompletion *aio_comp = new librbd::AioCompletion();
ictx->aio_work_queue->aio_flush(aio_comp);
ASSERT_EQ(0, aio_comp->wait_for_complete());
aio_comp->release();

View File

@ -54,8 +54,16 @@ struct AioImageRequest<MockReplayImageCtx> {
AioImageRequest<MockReplayImageCtx> *AioImageRequest<MockReplayImageCtx>::s_instance = nullptr;
namespace util {
inline ImageCtx *get_image_ctx(librbd::MockReplayImageCtx *image_ctx) {
return image_ctx->image_ctx;
}
} // namespace util
} // namespace librbd
// template definitions
#include "librbd/journal/Replay.cc"
template class librbd::journal::Replay<librbd::MockReplayImageCtx>;