aboutsummaryrefslogtreecommitdiff
path: root/include
diff options
context:
space:
mode:
authorMax Reitz <mreitz@redhat.com>2019-07-19 11:26:14 +0200
committerKevin Wolf <kwolf@redhat.com>2019-07-19 13:19:16 +0200
commite037c09c78520cbdb6da7cfc6ad0256d5870b814 (patch)
treeee4b9ffaf87c06445fe091a6dbeed72246005d16 /include
parent1b285657687c7f08761759092fa05fa33578fc00 (diff)
downloadqemu-e037c09c78520cbdb6da7cfc6ad0256d5870b814.zip
qemu-e037c09c78520cbdb6da7cfc6ad0256d5870b814.tar.gz
qemu-e037c09c78520cbdb6da7cfc6ad0256d5870b814.tar.bz2
block: Do not poll in bdrv_do_drained_end()
We should never poll anywhere in bdrv_do_drained_end() (including its recursive callees like bdrv_drain_invoke()), because it does not cope well with graph changes. In fact, it has been written based on the postulation that no graph changes will happen in it. Instead, the callers that want to poll must poll, i.e. all currently globally available wrappers: bdrv_drained_end(), bdrv_subtree_drained_end(), bdrv_unapply_subtree_drain(), and bdrv_drain_all_end(). Graph changes there do not matter. They can poll simply by passing a pointer to a drained_end_counter and wait until it reaches 0. This patch also adds a non-polling global wrapper for bdrv_do_drained_end() that takes a drained_end_counter pointer. We need such a variant because now no function called anywhere from bdrv_do_drained_end() must poll. This includes BdrvChildRole.drained_end(), which already must not poll according to its interface documentation, but bdrv_child_cb_drained_end() just violates that by invoking bdrv_drained_end() (which does poll). Therefore, BdrvChildRole.drained_end() must take a *drained_end_counter parameter, which bdrv_child_cb_drained_end() can pass on to the new bdrv_drained_end_no_poll() function. Note that we now have a pattern of all drained_end-related functions either polling or receiving a *drained_end_counter to let the caller poll based on that. A problem with a single poll loop is that when the drained section in bdrv_set_aio_context_ignore() ends, some nodes in the subgraph may be in the old contexts, while others are in the new context already. To let the collective poll in bdrv_drained_end() work correctly, we must not hold a lock to the old context, so that the old context can make progress in case it is different from the current context. (In the process, remove the comment saying that the current context is always the old context, because it is wrong.) In all other places, all nodes in a subtree must be in the same context, so we can just poll that. The exception of course is bdrv_drain_all_end(), but that always runs in the main context, so we can just poll NULL (like bdrv_drain_all_begin() does). Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Diffstat (limited to 'include')
-rw-r--r--include/block/block.h25
-rw-r--r--include/block/block_int.h6
2 files changed, 30 insertions, 1 deletions
diff --git a/include/block/block.h b/include/block/block.h
index a81645e..60f0047 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -612,6 +612,9 @@ void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll);
* bdrv_parent_drained_end_single:
*
* End a quiesced section for the parent of @c.
+ *
+ * This polls @bs's AioContext until all scheduled sub-drained_ends
+ * have settled, which may result in graph changes.
*/
void bdrv_parent_drained_end_single(BdrvChild *c);
@@ -661,10 +664,32 @@ void bdrv_subtree_drained_begin(BlockDriverState *bs);
* bdrv_drained_end:
*
* End a quiescent section started by bdrv_drained_begin().
+ *
+ * This polls @bs's AioContext until all scheduled sub-drained_ends
+ * have settled. On one hand, that may result in graph changes. On
+ * the other, this requires that all involved nodes (@bs and all of
+ * its parents) are in the same AioContext, and that the caller has
+ * acquired it.
+ * If there are any nodes that are in different contexts from @bs,
+ * these contexts must not be acquired.
*/
void bdrv_drained_end(BlockDriverState *bs);
/**
+ * bdrv_drained_end_no_poll:
+ *
+ * Same as bdrv_drained_end(), but do not poll for the subgraph to
+ * actually become unquiesced. Therefore, no graph changes will occur
+ * with this function.
+ *
+ * *drained_end_counter is incremented for every background operation
+ * that is scheduled, and will be decremented for every operation once
+ * it settles. The caller must poll until it reaches 0. The counter
+ * should be accessed using atomic operations only.
+ */
+void bdrv_drained_end_no_poll(BlockDriverState *bs, int *drained_end_counter);
+
+/**
* End a quiescent section started by bdrv_subtree_drained_begin().
*/
void bdrv_subtree_drained_end(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f5b044f..3aa1e83 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -664,11 +664,15 @@ struct BdrvChildRole {
* These functions must not change the graph (and therefore also must not
* call aio_poll(), which could change the graph indirectly).
*
+ * If drained_end() schedules background operations, it must atomically
+ * increment *drained_end_counter for each such operation and atomically
+ * decrement it once the operation has settled.
+ *
* Note that this can be nested. If drained_begin() was called twice, new
* I/O is allowed only after drained_end() was called twice, too.
*/
void (*drained_begin)(BdrvChild *child);
- void (*drained_end)(BdrvChild *child);
+ void (*drained_end)(BdrvChild *child, int *drained_end_counter);
/*
* Returns whether the parent has pending requests for the child. This