Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
block: remove AioContext locking 
This is the big patch that removes
aio_context_acquire()/aio_context_release() from the block layer and
affected block layer users.

There isn't a clean way to split this patch and the reviewers are likely
the same group of people, so I decided to do it in one patch.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Message-ID: <20231205182011.1976568-7-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
Stefan Hajnoczi authored and Kevin Wolf committed Dec 21, 2023
1 parent 382a6d7 commit 82e290d
Show file tree
Hide file tree
Showing 41 changed files with 104 additions and 1,169 deletions.
234 changes: 8 additions & 226 deletions block.c
View file

Large diffs are not rendered by default.

14 changes: 0 additions & 14 deletions block/block-backend.c
View file
Expand Up @@ -429,7 +429,6 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
{
BlockBackend *blk;
BlockDriverState *bs;
AioContext *ctx;
uint64_t perm = 0;
uint64_t shared = BLK_PERM_ALL;

Expand Down Expand Up @@ -459,23 +458,18 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
shared = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
}

aio_context_acquire(qemu_get_aio_context());
bs = bdrv_open(filename, reference, options, flags, errp);
aio_context_release(qemu_get_aio_context());
if (!bs) {
return NULL;
}

/* bdrv_open() could have moved bs to a different AioContext */
ctx = bdrv_get_aio_context(bs);
blk = blk_new(bdrv_get_aio_context(bs), perm, shared);
blk->perm = perm;
blk->shared_perm = shared;

aio_context_acquire(ctx);
blk_insert_bs(blk, bs, errp);
bdrv_unref(bs);
aio_context_release(ctx);

if (!blk->root) {
blk_unref(blk);
Expand Down Expand Up @@ -577,13 +571,9 @@ void blk_remove_all_bs(void)
GLOBAL_STATE_CODE();

while ((blk = blk_all_next(blk)) != NULL) {
AioContext *ctx = blk_get_aio_context(blk);

aio_context_acquire(ctx);
if (blk->root) {
blk_remove_bs(blk);
}
aio_context_release(ctx);
}
}

Expand Down Expand Up @@ -2736,20 +2726,16 @@ int blk_commit_all(void)
GRAPH_RDLOCK_GUARD_MAINLOOP();

while ((blk = blk_all_next(blk)) != NULL) {
AioContext *aio_context = blk_get_aio_context(blk);
BlockDriverState *unfiltered_bs = bdrv_skip_filters(blk_bs(blk));

aio_context_acquire(aio_context);
if (blk_is_inserted(blk) && bdrv_cow_child(unfiltered_bs)) {
int ret;

ret = bdrv_commit(unfiltered_bs);
if (ret < 0) {
aio_context_release(aio_context);
return ret;
}
}
aio_context_release(aio_context);
}
return 0;
}
Expand Down
22 changes: 5 additions & 17 deletions block/copy-before-write.c
View file
Expand Up @@ -412,7 +412,6 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
int64_t cluster_size;
g_autoptr(BlockdevOptions) full_opts = NULL;
BlockdevOptionsCbw *opts;
AioContext *ctx;
int ret;

full_opts = cbw_parse_options(options, errp);
Expand All @@ -435,15 +434,11 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,

GRAPH_RDLOCK_GUARD_MAINLOOP();

ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);

if (opts->bitmap) {
bitmap = block_dirty_bitmap_lookup(opts->bitmap->node,
opts->bitmap->name, NULL, errp);
if (!bitmap) {
ret = -EINVAL;
goto out;
return -EINVAL;
}
}
s->on_cbw_error = opts->has_on_cbw_error ? opts->on_cbw_error :
Expand All @@ -461,24 +456,21 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
s->bcs = block_copy_state_new(bs->file, s->target, bitmap, errp);
if (!s->bcs) {
error_prepend(errp, "Cannot create block-copy-state: ");
ret = -EINVAL;
goto out;
return -EINVAL;
}

cluster_size = block_copy_cluster_size(s->bcs);

s->done_bitmap = bdrv_create_dirty_bitmap(bs, cluster_size, NULL, errp);
if (!s->done_bitmap) {
ret = -EINVAL;
goto out;
return -EINVAL;
}
bdrv_disable_dirty_bitmap(s->done_bitmap);

/* s->access_bitmap starts equal to bcs bitmap */
s->access_bitmap = bdrv_create_dirty_bitmap(bs, cluster_size, NULL, errp);
if (!s->access_bitmap) {
ret = -EINVAL;
goto out;
return -EINVAL;
}
bdrv_disable_dirty_bitmap(s->access_bitmap);
bdrv_dirty_bitmap_merge_internal(s->access_bitmap,
Expand All @@ -487,11 +479,7 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,

qemu_co_mutex_init(&s->lock);
QLIST_INIT(&s->frozen_read_reqs);

ret = 0;
out:
aio_context_release(ctx);
return ret;
return 0;
}

static void cbw_close(BlockDriverState *bs)
Expand Down
22 changes: 1 addition & 21 deletions block/export/export.c
View file
Expand Up @@ -114,7 +114,6 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
}

ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);

if (export->iothread) {
IOThread *iothread;
Expand All @@ -133,8 +132,6 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
set_context_errp = fixed_iothread ? errp : NULL;
ret = bdrv_try_change_aio_context(bs, new_ctx, NULL, set_context_errp);
if (ret == 0) {
aio_context_release(ctx);
aio_context_acquire(new_ctx);
ctx = new_ctx;
} else if (fixed_iothread) {
goto fail;
Expand Down Expand Up @@ -191,16 +188,13 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
assert(exp->blk != NULL);

QLIST_INSERT_HEAD(&block_exports, exp, next);

aio_context_release(ctx);
return exp;

fail:
if (blk) {
blk_set_dev_ops(blk, NULL, NULL);
blk_unref(blk);
}
aio_context_release(ctx);
if (exp) {
g_free(exp->id);
g_free(exp);
Expand All @@ -218,9 +212,6 @@ void blk_exp_ref(BlockExport *exp)
static void blk_exp_delete_bh(void *opaque)
{
BlockExport *exp = opaque;
AioContext *aio_context = exp->ctx;

aio_context_acquire(aio_context);

assert(exp->refcount == 0);
QLIST_REMOVE(exp, next);
Expand All @@ -230,8 +221,6 @@ static void blk_exp_delete_bh(void *opaque)
qapi_event_send_block_export_deleted(exp->id);
g_free(exp->id);
g_free(exp);

aio_context_release(aio_context);
}

void blk_exp_unref(BlockExport *exp)
Expand All @@ -249,32 +238,23 @@ void blk_exp_unref(BlockExport *exp)
* connections and other internally held references start to shut down. When
* the function returns, there may still be active references while the export
* is in the process of shutting down.
*
* Acquires exp->ctx internally. Callers must *not* hold the lock.
*/
void blk_exp_request_shutdown(BlockExport *exp)
{
AioContext *aio_context = exp->ctx;

aio_context_acquire(aio_context);

/*
* If the user doesn't own the export any more, it is already shutting
* down. We must not call .request_shutdown and decrease the refcount a
* second time.
*/
if (!exp->user_owned) {
goto out;
return;
}

exp->drv->request_shutdown(exp);

assert(exp->user_owned);
exp->user_owned = false;
blk_exp_unref(exp);

out:
aio_context_release(aio_context);
}

/*
Expand Down
45 changes: 5 additions & 40 deletions block/io.c
View file
Expand Up @@ -294,16 +294,13 @@ static void bdrv_co_drain_bh_cb(void *opaque)
BlockDriverState *bs = data->bs;

if (bs) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
bdrv_dec_in_flight(bs);
if (data->begin) {
bdrv_do_drained_begin(bs, data->parent, data->poll);
} else {
assert(!data->poll);
bdrv_do_drained_end(bs, data->parent);
}
aio_context_release(ctx);
} else {
assert(data->begin);
bdrv_drain_all_begin();
Expand All @@ -320,8 +317,6 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
{
BdrvCoDrainData data;
Coroutine *self = qemu_coroutine_self();
AioContext *ctx = bdrv_get_aio_context(bs);
AioContext *co_ctx = qemu_coroutine_get_aio_context(self);

/* Calling bdrv_drain() from a BH ensures the current coroutine yields and
* other coroutines run if they were queued by aio_co_enter(). */
Expand All @@ -340,29 +335,13 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
bdrv_inc_in_flight(bs);
}

/*
* Temporarily drop the lock across yield or we would get deadlocks.
* bdrv_co_drain_bh_cb() reaquires the lock as needed.
*
* When we yield below, the lock for the current context will be
* released, so if this is actually the lock that protects bs, don't drop
* it a second time.
*/
if (ctx != co_ctx) {
aio_context_release(ctx);
}
replay_bh_schedule_oneshot_event(qemu_get_aio_context(),
bdrv_co_drain_bh_cb, &data);

qemu_coroutine_yield();
/* If we are resumed from some other event (such as an aio completion or a
* timer callback), it is a bug in the caller that should be fixed. */
assert(data.done);

/* Reacquire the AioContext of bs if we dropped it */
if (ctx != co_ctx) {
aio_context_acquire(ctx);
}
}

static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent,
Expand Down Expand Up @@ -478,13 +457,12 @@ static bool bdrv_drain_all_poll(void)
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();

/* bdrv_drain_poll() can't make changes to the graph and we are holding the
* main AioContext lock, so iterating bdrv_next_all_states() is safe. */
/*
* bdrv_drain_poll() can't make changes to the graph and we hold the BQL,
* so iterating bdrv_next_all_states() is safe.
*/
while ((bs = bdrv_next_all_states(bs))) {
AioContext *aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
result |= bdrv_drain_poll(bs, NULL, true);
aio_context_release(aio_context);
}

return result;
Expand Down Expand Up @@ -525,11 +503,7 @@ void bdrv_drain_all_begin_nopoll(void)
/* Quiesce all nodes, without polling in-flight requests yet. The graph
* cannot change during this loop. */
while ((bs = bdrv_next_all_states(bs))) {
AioContext *aio_context = bdrv_get_aio_context(bs);

aio_context_acquire(aio_context);
bdrv_do_drained_begin(bs, NULL, false);
aio_context_release(aio_context);
}
}

Expand Down Expand Up @@ -588,11 +562,7 @@ void bdrv_drain_all_end(void)
}

while ((bs = bdrv_next_all_states(bs))) {
AioContext *aio_context = bdrv_get_aio_context(bs);

aio_context_acquire(aio_context);
bdrv_do_drained_end(bs, NULL);
aio_context_release(aio_context);
}

assert(qemu_get_current_aio_context() == qemu_get_aio_context());
Expand Down Expand Up @@ -2368,15 +2338,10 @@ int bdrv_flush_all(void)
}

for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *aio_context = bdrv_get_aio_context(bs);
int ret;

aio_context_acquire(aio_context);
ret = bdrv_flush(bs);
int ret = bdrv_flush(bs);
if (ret < 0 && !result) {
result = ret;
}
aio_context_release(aio_context);
}

return result;
Expand Down

0 comments on commit 82e290d

Please sign in to comment.