[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain |
Date: |
Thu, 12 Apr 2018 10:37:51 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
On 11/04/2018 18:39, Kevin Wolf wrote:
> +bool bdrv_drain_poll(BlockDriverState *bs, bool top_level)
> {
> /* Execute pending BHs first and check everything else only after the BHs
> * have executed. */
> - while (aio_poll(bs->aio_context, false));
> + if (top_level) {
> + while (aio_poll(bs->aio_context, false));
> + }
> +
> + if (bdrv_parent_drained_poll(bs)) {
> + return true;
> + }
> +
> return atomic_read(&bs->in_flight);
> }
>
Up until now I liked very much this series, but I like this patch a bit
less for two reasons.
1) I think I would prefer to have the !top_level case in a separate
function---making the API easier to use in the BdrvChildRole callback
because there is no need to pass false. In addition, the callback is
not really polling anything, but rather returning whether the children
are quiescent. So a better name would be bdrv_children_drained and
c->role->drained.
2) Worse, the main idea behind the first drain restructuring was that
draining could proceed in topological order: first drain the roots' I/O,
then call bdrv_drain to send the last requests to their children, then
recurse. It is not clear to me why you need to introduce this recursive
step, which is also O(n^2) in the worst case.
Paolo
- [Qemu-devel] [PATCH 05/19] tests/test-bdrv-drain: bdrv_drain_all() works in coroutines now, (continued)
- [Qemu-devel] [PATCH 05/19] tests/test-bdrv-drain: bdrv_drain_all() works in coroutines now, Kevin Wolf, 2018/04/11
- [Qemu-devel] [PATCH 08/19] block: Remove bdrv_drain_recurse(), Kevin Wolf, 2018/04/11
- [Qemu-devel] [PATCH 04/19] block: Don't manually poll in bdrv_drain_all(), Kevin Wolf, 2018/04/11
- [Qemu-devel] [PATCH 06/19] block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE(), Kevin Wolf, 2018/04/11
- [Qemu-devel] [PATCH 09/19] test-bdrv-drain: Add test for node deletion, Kevin Wolf, 2018/04/11
- [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Kevin Wolf, 2018/04/11
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Kevin Wolf, 2018/04/12
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Paolo Bonzini, 2018/04/12
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Kevin Wolf, 2018/04/12
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Paolo Bonzini, 2018/04/12
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Kevin Wolf, 2018/04/12
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Paolo Bonzini, 2018/04/12
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Kevin Wolf, 2018/04/12
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Paolo Bonzini, 2018/04/12
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Kevin Wolf, 2018/04/12
- Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain, Paolo Bonzini, 2018/04/12