Skip to content

Commit

Permalink
block: Remove subtree drains
Browse files Browse the repository at this point in the history 
Subtree drains are not used any more. Remove them.

After this, BdrvChildClass.attach/detach() don't poll any more.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20221118174110.55183-11-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
@kevmw
kevmw committed Dec 15, 2022
1 parent 92140b9 commit 299403a
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 389 deletions.
20 changes: 5 additions & 15 deletions block.c
View file
Expand Up @@ -1232,7 +1232,7 @@ static void bdrv_child_cb_drained_begin(BdrvChild *child)
static bool bdrv_child_cb_drained_poll(BdrvChild *child)
{
BlockDriverState *bs = child->opaque;
return bdrv_drain_poll(bs, false, NULL, false);
return bdrv_drain_poll(bs, NULL, false);
}

static void bdrv_child_cb_drained_end(BdrvChild *child)
Expand Down Expand Up @@ -1482,8 +1482,6 @@ static void bdrv_child_cb_attach(BdrvChild *child)
assert(!bs->file);
bs->file = child;
}

bdrv_apply_subtree_drain(child, bs);
}

static void bdrv_child_cb_detach(BdrvChild *child)
Expand All @@ -1494,8 +1492,6 @@ static void bdrv_child_cb_detach(BdrvChild *child)
bdrv_backing_detach(child);
}

bdrv_unapply_subtree_drain(child, bs);

assert_bdrv_graph_writable(bs);
QLIST_REMOVE(child, next);
if (child == bs->backing) {
Expand Down Expand Up @@ -2882,9 +2878,6 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
}

if (old_bs) {
/* Detach first so that the recursive drain sections coming from @child
* are already gone and we only end the drain sections that came from
* elsewhere. */
if (child->klass->detach) {
child->klass->detach(child);
}
Expand All @@ -2899,17 +2892,14 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent);

/*
* Detaching the old node may have led to the new node's
* quiesce_counter having been decreased. Not a problem, we
* just need to recognize this here and then invoke
* drained_end appropriately more often.
* Polling in bdrv_parent_drained_begin_single() may have led to the new
* node's quiesce_counter having been decreased. Not a problem, we just
* need to recognize this here and then invoke drained_end appropriately
* more often.
*/
assert(new_bs->quiesce_counter <= new_bs_quiesce_counter);
drain_saldo += new_bs->quiesce_counter - new_bs_quiesce_counter;

/* Attach only after starting new drained sections, so that recursive
* drain sections coming from @child don't get an extra .drained_begin
* callback. */
if (child->klass->attach) {
child->klass->attach(child);
}
Expand Down
121 changes: 26 additions & 95 deletions block/io.c
View file
Expand Up @@ -236,17 +236,15 @@ typedef struct {
BlockDriverState *bs;
bool done;
bool begin;
bool recursive;
bool poll;
BdrvChild *parent;
bool ignore_bds_parents;
} BdrvCoDrainData;

/* Returns true if BDRV_POLL_WHILE() should go into a blocking aio_poll() */
bool bdrv_drain_poll(BlockDriverState *bs, bool recursive,
BdrvChild *ignore_parent, bool ignore_bds_parents)
bool bdrv_drain_poll(BlockDriverState *bs, BdrvChild *ignore_parent,
bool ignore_bds_parents)
{
BdrvChild *child, *next;
IO_OR_GS_CODE();

if (bdrv_parent_drained_poll(bs, ignore_parent, ignore_bds_parents)) {
Expand All @@ -257,29 +255,19 @@ bool bdrv_drain_poll(BlockDriverState *bs, bool recursive,
return true;
}

if (recursive) {
assert(!ignore_bds_parents);
QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
if (bdrv_drain_poll(child->bs, recursive, child, false)) {
return true;
}
}
}

return false;
}

static bool bdrv_drain_poll_top_level(BlockDriverState *bs, bool recursive,
static bool bdrv_drain_poll_top_level(BlockDriverState *bs,
BdrvChild *ignore_parent)
{
return bdrv_drain_poll(bs, recursive, ignore_parent, false);
return bdrv_drain_poll(bs, ignore_parent, false);
}

static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive,
BdrvChild *parent, bool ignore_bds_parents,
bool poll);
static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive,
BdrvChild *parent, bool ignore_bds_parents);
static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent,
bool ignore_bds_parents, bool poll);
static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent,
bool ignore_bds_parents);

static void bdrv_co_drain_bh_cb(void *opaque)
{
Expand All @@ -292,12 +280,11 @@ static void bdrv_co_drain_bh_cb(void *opaque)
aio_context_acquire(ctx);
bdrv_dec_in_flight(bs);
if (data->begin) {
bdrv_do_drained_begin(bs, data->recursive, data->parent,
data->ignore_bds_parents, data->poll);
bdrv_do_drained_begin(bs, data->parent, data->ignore_bds_parents,
data->poll);
} else {
assert(!data->poll);
bdrv_do_drained_end(bs, data->recursive, data->parent,
data->ignore_bds_parents);
bdrv_do_drained_end(bs, data->parent, data->ignore_bds_parents);
}
aio_context_release(ctx);
} else {
Expand All @@ -310,7 +297,7 @@ static void bdrv_co_drain_bh_cb(void *opaque)
}

static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
bool begin, bool recursive,
bool begin,
BdrvChild *parent,
bool ignore_bds_parents,
bool poll)
Expand All @@ -329,7 +316,6 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
.bs = bs,
.done = false,
.begin = begin,
.recursive = recursive,
.parent = parent,
.ignore_bds_parents = ignore_bds_parents,
.poll = poll,
Expand Down Expand Up @@ -380,29 +366,16 @@ void bdrv_do_drained_begin_quiesce(BlockDriverState *bs,
}
}

static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive,
BdrvChild *parent, bool ignore_bds_parents,
bool poll)
static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent,
bool ignore_bds_parents, bool poll)
{
BdrvChild *child, *next;

if (qemu_in_coroutine()) {
bdrv_co_yield_to_drain(bs, true, recursive, parent, ignore_bds_parents,
poll);
bdrv_co_yield_to_drain(bs, true, parent, ignore_bds_parents, poll);
return;
}

bdrv_do_drained_begin_quiesce(bs, parent, ignore_bds_parents);

if (recursive) {
assert(!ignore_bds_parents);
bs->recursive_quiesce_counter++;
QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
bdrv_do_drained_begin(child->bs, true, child, ignore_bds_parents,
false);
}
}

/*
* Wait for drained requests to finish.
*
Expand All @@ -414,35 +387,27 @@ static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive,
*/
if (poll) {
assert(!ignore_bds_parents);
BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, recursive, parent));
BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, parent));
}
}

void bdrv_drained_begin(BlockDriverState *bs)
{
IO_OR_GS_CODE();
bdrv_do_drained_begin(bs, false, NULL, false, true);
}

void bdrv_subtree_drained_begin(BlockDriverState *bs)
{
IO_OR_GS_CODE();
bdrv_do_drained_begin(bs, true, NULL, false, true);
bdrv_do_drained_begin(bs, NULL, false, true);
}

/**
* This function does not poll, nor must any of its recursively called
* functions.
*/
static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive,
BdrvChild *parent, bool ignore_bds_parents)
static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent,
bool ignore_bds_parents)
{
BdrvChild *child;
int old_quiesce_counter;

if (qemu_in_coroutine()) {
bdrv_co_yield_to_drain(bs, false, recursive, parent, ignore_bds_parents,
false);
bdrv_co_yield_to_drain(bs, false, parent, ignore_bds_parents, false);
return;
}
assert(bs->quiesce_counter > 0);
Expand All @@ -457,46 +422,12 @@ static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive,
if (old_quiesce_counter == 1) {
aio_enable_external(bdrv_get_aio_context(bs));
}

if (recursive) {
assert(!ignore_bds_parents);
bs->recursive_quiesce_counter--;
QLIST_FOREACH(child, &bs->children, next) {
bdrv_do_drained_end(child->bs, true, child, ignore_bds_parents);
}
}
}

void bdrv_drained_end(BlockDriverState *bs)
{
IO_OR_GS_CODE();
bdrv_do_drained_end(bs, false, NULL, false);
}

void bdrv_subtree_drained_end(BlockDriverState *bs)
{
IO_OR_GS_CODE();
bdrv_do_drained_end(bs, true, NULL, false);
}

void bdrv_apply_subtree_drain(BdrvChild *child, BlockDriverState *new_parent)
{
int i;
IO_OR_GS_CODE();

for (i = 0; i < new_parent->recursive_quiesce_counter; i++) {
bdrv_do_drained_begin(child->bs, true, child, false, true);
}
}

void bdrv_unapply_subtree_drain(BdrvChild *child, BlockDriverState *old_parent)
{
int i;
IO_OR_GS_CODE();

for (i = 0; i < old_parent->recursive_quiesce_counter; i++) {
bdrv_do_drained_end(child->bs, true, child, false);
}
bdrv_do_drained_end(bs, NULL, false);
}

void bdrv_drain(BlockDriverState *bs)
Expand Down Expand Up @@ -529,7 +460,7 @@ static bool bdrv_drain_all_poll(void)
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, false, NULL, true);
result |= bdrv_drain_poll(bs, NULL, true);
aio_context_release(aio_context);
}

Expand All @@ -554,7 +485,7 @@ void bdrv_drain_all_begin(void)
GLOBAL_STATE_CODE();

if (qemu_in_coroutine()) {
bdrv_co_yield_to_drain(NULL, true, false, NULL, true, true);
bdrv_co_yield_to_drain(NULL, true, NULL, true, true);
return;
}

Expand All @@ -579,7 +510,7 @@ void bdrv_drain_all_begin(void)
AioContext *aio_context = bdrv_get_aio_context(bs);

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

Expand All @@ -599,7 +530,7 @@ void bdrv_drain_all_end_quiesce(BlockDriverState *bs)
g_assert(!bs->refcnt);

while (bs->quiesce_counter) {
bdrv_do_drained_end(bs, false, NULL, true);
bdrv_do_drained_end(bs, NULL, true);
}
}

Expand All @@ -621,7 +552,7 @@ void bdrv_drain_all_end(void)
AioContext *aio_context = bdrv_get_aio_context(bs);

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

Expand Down
18 changes: 3 additions & 15 deletions include/block/block-io.h
View file
Expand Up @@ -302,17 +302,16 @@ void bdrv_parent_drained_end_single(BdrvChild *c);
/**
* bdrv_drain_poll:
*
* Poll for pending requests in @bs, its parents (except for @ignore_parent),
* and if @recursive is true its children as well (used for subtree drain).
* Poll for pending requests in @bs and its parents (except for @ignore_parent).
*
* If @ignore_bds_parents is true, parents that are BlockDriverStates must
* ignore the drain request because they will be drained separately (used for
* drain_all).
*
* This is part of bdrv_drained_begin.
*/
bool bdrv_drain_poll(BlockDriverState *bs, bool recursive,
BdrvChild *ignore_parent, bool ignore_bds_parents);
bool bdrv_drain_poll(BlockDriverState *bs, BdrvChild *ignore_parent,
bool ignore_bds_parents);

/**
* bdrv_drained_begin:
Expand All @@ -333,22 +332,11 @@ void bdrv_drained_begin(BlockDriverState *bs);
void bdrv_do_drained_begin_quiesce(BlockDriverState *bs,
BdrvChild *parent, bool ignore_bds_parents);

/**
* Like bdrv_drained_begin, but recursively begins a quiesced section for
* exclusive access to all child nodes as well.
*/
void bdrv_subtree_drained_begin(BlockDriverState *bs);

/**
* bdrv_drained_end:
*
* End a quiescent section started by bdrv_drained_begin().
*/
void bdrv_drained_end(BlockDriverState *bs);

/**
* End a quiescent section started by bdrv_subtree_drained_begin().
*/
void bdrv_subtree_drained_end(BlockDriverState *bs);

#endif /* BLOCK_IO_H */
1 change: 0 additions & 1 deletion include/block/block_int-common.h
View file
Expand Up @@ -1184,7 +1184,6 @@ struct BlockDriverState {

/* Accessed with atomic ops. */
int quiesce_counter;
int recursive_quiesce_counter;

unsigned int write_gen; /* Current data generation */

Expand Down
12 changes: 0 additions & 12 deletions include/block/block_int-io.h
View file
Expand Up @@ -179,16 +179,4 @@ void bdrv_bsc_invalidate_range(BlockDriverState *bs,
*/
void bdrv_bsc_fill(BlockDriverState *bs, int64_t offset, int64_t bytes);


/*
* "I/O or GS" API functions. These functions can run without
* the BQL, but only in one specific iothread/main loop.
*
* See include/block/block-io.h for more information about
* the "I/O or GS" API.
*/

void bdrv_apply_subtree_drain(BdrvChild *child, BlockDriverState *new_parent);
void bdrv_unapply_subtree_drain(BdrvChild *child, BlockDriverState *old_parent);

#endif /* BLOCK_INT_IO_H */

0 comments on commit 299403a

Please sign in to comment.