On Tue, Nov 08, 2022 at 01:37:25PM +0100, Kevin Wolf wrote: > I'm aware that exactly nobody has been looking forward to a series with > this title, but it has to be. The way drain works means that we need to > poll in bdrv_replace_child_noperm() and that makes things rather messy > with Emanuele's multiqueue work because you must not poll while you hold > the graph lock. > > The other reason why it has to be is that drain is way too complex and > there are too many different cases. Some simplification like this will > hopefully make it considerably more maintainable. The diffstat probably > tells something, too. > > There are roughly speaking three parts in this series: > > 1. Make BlockDriver.bdrv_drained_begin/end() non-coroutine_fn again, > which allows us to not poll on bdrv_drained_end() any more. > > 2. Remove subtree drains. They are a considerable complication in the > whole drain machinery (in particular, they require polling in the > BdrvChildClass.attach/detach() callbacks that are called during > bdrv_replace_child_noperm()) and none of their users actually has a > good reason to use them. > > 3. Finally get rid of polling in bdrv_replace_child_noperm() by > requiring that the child is already drained by the caller and calling > callbacks only once and not again for every nested drain section. > > If necessary, a prefix of this series can be merged that covers only the > first or the first two parts and it would still make sense. > > Kevin Wolf (13): > qed: Don't yield in bdrv_qed_co_drain_begin() > test-bdrv-drain: Don't yield in .bdrv_co_drained_begin/end() > block: Revert .bdrv_drained_begin/end to non-coroutine_fn > block: Remove drained_end_counter > block: Inline bdrv_drain_invoke() > block: Drain invidual nodes during reopen > block: Don't use subtree drains in bdrv_drop_intermediate() > stream: Replace subtree drain with a single node drain > block: Remove subtree drains > block: Call drain callbacks only once > block: Remove ignore_bds_parents parameter from drain functions > block: Don't poll in bdrv_replace_child_noperm() > block: Remove poll parameter from bdrv_parent_drained_begin_single() > > include/block/block-global-state.h | 3 + > include/block/block-io.h | 52 +--- > include/block/block_int-common.h | 17 +- > include/block/block_int-io.h | 12 - > block.c | 132 ++++++----- > block/block-backend.c | 4 +- > block/io.c | 281 ++++------------------ > block/qed.c | 24 +- > block/replication.c | 6 - > block/stream.c | 20 +- > block/throttle.c | 6 +- > blockdev.c | 13 - > blockjob.c | 2 +- > tests/unit/test-bdrv-drain.c | 369 +++++++---------------------- > 14 files changed, 270 insertions(+), 671 deletions(-)
I have looked through all patches but don't understand the code well enough to give an opinion or spot bugs. Removing subtree drains and aio_poll() in bdrv_replace_child_noperm() are nice. Acked-by: Stefan Hajnoczi <stefa...@redhat.com>
signature.asc
Description: PGP signature