Skip to content

Drain fixes and cleanups, part 3

Kevin Wolf qemu-devel 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest20 patches 2 repliesIn review
  2. v119 patches 31 repliesSuperseded

Changes in v2 from the cover letter

- Rebased on top of current master (e.g. including Job infrastructure)

- Avoid unnecessary parent callbacks for .drained_begin/poll/end:
* subtree drains: Don't propagate the drain to the parent that we came from recursively
* drain_all: Don't propagate the drain to BDS parents (which are
already separately drained), but only to non-BDS parents like BBs or Jobs

- Separate bdrv_drain_poll_top_level() function instead of having a top_level parameter for bdrv_drain_poll().

- A few commit message and comment improvements

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/20test-bdrv-drain: bdrv_drain() works with cross-AioContext eventsblock/io.c tests/test-bdrv-drain.c+186 -50 replies
2/20block: Use bdrv_do_drain_begin/end in bdrv_drain_all()block/io.c tests/test-bdrv-drain.c+6 -180 replies
3/20block: Remove 'recursive' parameter from bdrv_drain_invoke()block/io.c+3 -100 replies
4/20block: Don't manually poll in bdrv_drain_all()block/io.c+12 -290 replies
5/20tests/test-bdrv-drain: bdrv_drain_all() works in coroutines nowtests/test-bdrv-drain.c+14 -20 replies
6/20block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE()block/io.c include/block/aio-wait.h+18 -150 replies
7/20block: Really pause block jobs on drainblock.c block/io.c block/mirror.c +5 more+107 -140 replies
8/20block: Remove bdrv_drain_recurse()block/io.c+3 -330 replies
9/20test-bdrv-drain: Add test for node deletiontests/test-bdrv-drain.c+169 -00 replies
10/20block: Drain recursively with a single BDRV_POLL_WHILE()block.c block/io.c include/block/block.h+52 -220 replies
11/20test-bdrv-drain: Test node deletion in subtree recursiontests/test-bdrv-drain.c+29 -90 replies
12/20block: Don't poll in parent drain callbacksblock.c block/io.c include/block/block.h+26 -90 replies
13/20test-bdrv-drain: Graph change through parent callbacktests/test-bdrv-drain.c+130 -00 replies
14/20block: Defer .bdrv_drain_begin callback to polling phaseblock/io.c+23 -50 replies
15/20test-bdrv-drain: Test that bdrv_drain_invoke() doesn't polltests/test-bdrv-drain.c+88 -140 replies
16/20block: Allow AIO_WAIT_WHILE with NULL ctxinclude/block/aio-wait.h+9 -40 replies
17/20block: Move bdrv_drain_all_begin() out of coroutine contextblock/io.c+17 -50 replies
18/20block: ignore_bds_parents parameter for drain functionsblock.c block/io.c block/vvfat.c +2 more+78 -440 replies
19/20block: Allow graph changes in bdrv_drain_all_begin/end sectionsblock.c block/io.c include/block/block.h +1 more+79 -170 replies
20/20test-bdrv-drain: Test graph changes in drain_all sectiontests/test-bdrv-drain.c+73 -20 replies

Reviews trailers per patch

6 of 20 patches have a Reviewed-by or Acked-by.

PatchStefan Hajnoczi
1/20
2/20R-b
3/20R-b
4/20R-b
5/20R-b
6/20R-b
7/20
8/20R-b
9/20
10/20
11/20
12/20
13/20
14/20
15/20
16/20
17/20
18/20
19/20
20/20

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v2 thread

Cover letter [Qemu-devel] [PATCH v2 00/20] Drain fixes and cleanups, part 3

This is the third and hopefully for now last part of my work to fix drain. The main goal of this series is to make drain robust against graph changes that happen in any callbacks of in-flight requests while we drain a block node.

The individual patches describe the details, but the rough plan is to change all three drain types (single node, subtree and all) to work like this:

1. First call all the necessary callbacks to quiesce external sources for new requests. This includes the block driver callbacks, the child node callbacks and disabling external AioContext events. This is done recursively.

Much of the trouble we had with drain resulted from the fact that the graph changed while we were traversing the graph recursively. None of the callbacks called in this phase may change the graph.

2. Then do a single AIO_WAIT_WHILE() to drain the requests of all affected nodes. The aio_poll() called by it is where graph changes can happen and we need to be careful.

However, while evaluating the loop condition, the graph can't change, so we can safely call all necessary callbacks, if needed recursively, to determine whether there are still pending requests in any affected nodes. We just need to make sure that we don't rely on the set of nodes being the same between any two evaluation of the condition.

There are a few more smaller, mostly self-contained changes needed before we're actually safe, but this is the main mechanism that will help you understand what we're working towards during the series.

v2:

- Rebased on top of current master (e.g. including Job infrastructure)

- Avoid unnecessary parent callbacks for .drained_begin/poll/end:
* subtree drains: Don't propagate the drain to the parent that we came from recursively
* drain_all: Don't propagate the drain to BDS parents (which are
already separately drained), but only to non-BDS parents like BBs or Jobs

- Separate bdrv_drain_poll_top_level() function instead of having a top_level parameter for bdrv_drain_poll().

- A few commit message and comment improvements

Kevin Wolf (19):
test-bdrv-drain: bdrv_drain() works with cross-AioContext events
block: Use bdrv_do_drain_begin/end in bdrv_drain_all()
block: Remove 'recursive' parameter from bdrv_drain_invoke()
block: Don't manually poll in bdrv_drain_all() tests/test-bdrv-drain: bdrv_drain_all() works in coroutines now
block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE()
block: Really pause block jobs on drain
block: Remove bdrv_drain_recurse()
block: Drain recursively with a single BDRV_POLL_WHILE()
test-bdrv-drain: Test node deletion in subtree recursion
block: Don't poll in parent drain callbacks
test-bdrv-drain: Graph change through parent callback
block: Defer .bdrv_drain_begin callback to polling phase
test-bdrv-drain: Test that bdrv_drain_invoke() doesn't poll
block: Allow AIO_WAIT_WHILE with NULL ctx
block: Move bdrv_drain_all_begin() out of coroutine context
block: ignore_bds_parents parameter for drain functions
block: Allow graph changes in bdrv_drain_all_begin/end sections
test-bdrv-drain: Test graph changes in drain_all section

Max Reitz (1):
test-bdrv-drain: Add test for node deletion

 include/block/aio-wait.h     |  25 +-
 include/block/block.h        |  31 +-
 include/block/block_int.h    |  14 +
 include/block/blockjob_int.h |   8 +
 block.c                      |  52 +++-
 block/io.c                   | 332 ++++++++++++--------
 block/mirror.c               |   8 +
 block/vvfat.c                |   1 +
 blockjob.c                   |  23 ++
 tests/test-bdrv-drain.c      | 705 +++++++++++++++++++++++++++++++++++++++++--
 10 files changed, 1032 insertions(+), 167 deletions(-)
Signature · 1 line
2.13.6