block: only call aio_poll on the current thread's AioContext

aio_poll is not thread safe; for example bdrv_drain can hang if
the last in-flight I/O operation is completed in the I/O thread after
the main thread has checked bs->in_flight.

The bug remains latent as long as all of it is called within
aio_context_acquire/aio_context_release, but this will change soon.

To fix this, if bdrv_drain is called from outside the I/O thread,
signal the main AioContext through a dummy bottom half.  The event
loop then only runs in the I/O thread.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <1477565348-5458-18-git-send-email-pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
This commit is contained in:
Paolo Bonzini 2016-10-27 12:49:05 +02:00 committed by Fam Zheng
parent 9e944cb474
commit c9d1a56174
8 changed files with 60 additions and 6 deletions

View File

@ -61,6 +61,7 @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
smp_wmb(); smp_wmb();
ctx->first_bh = bh; ctx->first_bh = bh;
qemu_mutex_unlock(&ctx->bh_lock); qemu_mutex_unlock(&ctx->bh_lock);
aio_notify(ctx);
} }
QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque) QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)

View File

@ -2090,7 +2090,9 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er
assert(bs_queue != NULL); assert(bs_queue != NULL);
aio_context_release(ctx);
bdrv_drain_all(); bdrv_drain_all();
aio_context_acquire(ctx);
QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) { QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, &local_err)) { if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, &local_err)) {

View File

@ -474,9 +474,21 @@ void bdrv_inc_in_flight(BlockDriverState *bs)
atomic_inc(&bs->in_flight); atomic_inc(&bs->in_flight);
} }
static void dummy_bh_cb(void *opaque)
{
}
void bdrv_wakeup(BlockDriverState *bs)
{
if (bs->wakeup) {
aio_bh_schedule_oneshot(qemu_get_aio_context(), dummy_bh_cb, NULL);
}
}
void bdrv_dec_in_flight(BlockDriverState *bs) void bdrv_dec_in_flight(BlockDriverState *bs)
{ {
atomic_dec(&bs->in_flight); atomic_dec(&bs->in_flight);
bdrv_wakeup(bs);
} }
static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self) static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)

View File

@ -505,6 +505,7 @@ nfs_get_allocated_file_size_cb(int ret, struct nfs_context *nfs, void *data,
error_report("NFS Error: %s", nfs_get_error(nfs)); error_report("NFS Error: %s", nfs_get_error(nfs));
} }
task->complete = 1; task->complete = 1;
bdrv_wakeup(task->bs);
} }
static int64_t nfs_get_allocated_file_size(BlockDriverState *bs) static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)

View File

@ -702,6 +702,9 @@ out:
srco->ret = ret; srco->ret = ret;
srco->finished = true; srco->finished = true;
if (srco->bs) {
bdrv_wakeup(srco->bs);
}
} }
/* /*

View File

@ -189,13 +189,11 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
assert(s->ctx == iothread_get_aio_context(vs->conf.iothread)); assert(s->ctx == iothread_get_aio_context(vs->conf.iothread));
aio_context_acquire(s->ctx); aio_context_acquire(s->ctx);
virtio_scsi_clear_aio(s); virtio_scsi_clear_aio(s);
aio_context_release(s->ctx);
blk_drain_all(); /* ensure there are no in-flight requests */ blk_drain_all(); /* ensure there are no in-flight requests */
aio_context_release(s->ctx);
for (i = 0; i < vs->conf.num_queues + 2; i++) { for (i = 0; i < vs->conf.num_queues + 2; i++) {
virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false); virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
} }

View File

@ -337,10 +337,30 @@ void bdrv_drain_all(void);
#define BDRV_POLL_WHILE(bs, cond) ({ \ #define BDRV_POLL_WHILE(bs, cond) ({ \
bool waited_ = false; \ bool waited_ = false; \
BlockDriverState *bs_ = (bs); \ BlockDriverState *bs_ = (bs); \
AioContext *ctx_ = bdrv_get_aio_context(bs_); \
if (aio_context_in_iothread(ctx_)) { \
while ((cond)) { \ while ((cond)) { \
aio_poll(bdrv_get_aio_context(bs_), true); \ aio_poll(ctx_, true); \
waited_ = true; \ waited_ = true; \
} \ } \
} else { \
assert(qemu_get_current_aio_context() == \
qemu_get_aio_context()); \
/* Ask bdrv_dec_in_flight to wake up the main \
* QEMU AioContext. Extra I/O threads never take \
* other I/O threads' AioContexts (see for example \
* block_job_defer_to_main_loop for how to do it). \
*/ \
assert(!bs_->wakeup); \
bs_->wakeup = true; \
while ((cond)) { \
aio_context_release(ctx_); \
aio_poll(qemu_get_aio_context(), true); \
aio_context_acquire(ctx_); \
waited_ = true; \
} \
bs_->wakeup = false; \
} \
waited_; }) waited_; })
int bdrv_pdiscard(BlockDriverState *bs, int64_t offset, int count); int bdrv_pdiscard(BlockDriverState *bs, int64_t offset, int count);

View File

@ -475,6 +475,8 @@ struct BlockDriverState {
unsigned int in_flight; unsigned int in_flight;
unsigned int serialising_in_flight; unsigned int serialising_in_flight;
bool wakeup;
/* Offset after the highest byte written to */ /* Offset after the highest byte written to */
uint64_t wr_highest_offset; uint64_t wr_highest_offset;
@ -633,6 +635,21 @@ void bdrv_remove_aio_context_notifier(BlockDriverState *bs,
void (*aio_context_detached)(void *), void (*aio_context_detached)(void *),
void *opaque); void *opaque);
/**
* bdrv_wakeup:
* @bs: The BlockDriverState for which an I/O operation has been completed.
*
* Wake up the main thread if it is waiting on BDRV_POLL_WHILE. During
* synchronous I/O on a BlockDriverState that is attached to another
* I/O thread, the main thread lets the I/O thread's event loop run,
* waiting for the I/O operation to complete. A bdrv_wakeup will wake
* up the main thread if necessary.
*
* Manual calls to bdrv_wakeup are rarely necessary, because
* bdrv_dec_in_flight already calls it.
*/
void bdrv_wakeup(BlockDriverState *bs);
#ifdef _WIN32 #ifdef _WIN32
int is_windows_drive(const char *filename); int is_windows_drive(const char *filename);
#endif #endif