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 <stefanha@redhat.com>