[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: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain |
Date: |
Thu, 12 Apr 2018 15:27:35 +0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 12.04.2018 um 14:02 hat Paolo Bonzini geschrieben:
> On 12/04/2018 13:53, Kevin Wolf wrote:
> >> The problem I have is that there is a direction through which I/O flows
> >> (parent-to-child), so why can't draining follow that natural direction.
> >> Having to check for the parents' I/O, while draining the child, seems
> >> wrong. Perhaps we can't help it, but I cannot understand the reason.
> > I'm not sure what's there that could be not understood. You already
> > confirmed that we need to drain the parents, too, when we drain a node.
> > Drain really must propagate in the opposite direction of I/O, because
> > part of its job is to quiesce the origin of any I/O to the node that
> > should be drained. Opposite of I/O _is_ the natural direction for drain.
>
> Opposite of I/O is the natural direction for drain to propagate, yes.
>
> However, I/O direction is the natural direction for requests to stop.
> After quiescing X and calling X->drv->bdrv_drain(X), there can be
> pending requests only in X's children. So I don't understand why you
> need to keep checking in_flight over the whole subgraph, when there are
> roots that will conclude their request first, and then their children,
> and so on so forth.
Not sure I follow. Let's look at an example. Say, we have a block job
BlockBackend as the root (because that uses proper layering, unlike
devices which use aio_disable_external()), connected to a qcow2 node
over file.
1. The block job issues a request to the qcow2 node
2. In order to process that request, the qcow2 node internally issues
one or more requests to the file node
3. Someone calls bdrv_drained_begin(qcow2_node)
4. We call BlockDriver.bdrv_drained_begin() for qcow2_node and
file_node, and BdrvChildRole.drained_begin() for the block job.
Now the block nodes don't create any new original requests any more
(qcow2 and file don't do that anyway; qcow2 only creates requests to
its children in the context of parent requests). The job potentially
continues sending requests until it reaches the next pause point. The
previously issued requests are still in flight.
Is this step what you meant by X->drv->bdrv_drain(X)? I don't see why
pending requests can only be in X's children. Why can't the request
be pending in X itself, say waiting for a thread pool worker
decrypting a buffer?
Also, note that after this series, the driver callbacks are called
asynchronously, but I don't think it makes a big difference here.
5. The file_node request completes. file_node doesn't have any requests
in flight any more, but in theory it could still get new requests
from qcow2_node. Anyway, let's say this is the last request, so I
think we'd call its requests concluded?
6. qcow2 still has a request in flight, but doesn't need to access
file_node for it. It finishes the work and therefore concludes its
requests as well. Note that qcow2_node (the parent) concludes after
file_node (the child).
7. We'll keep the example simple, so after completion of its request,
the job reaches a pause point without sending a new request. Again,
this happens after qcow2_node has concluded.
8. Only when neither file_node nor qcow2_node have a request in flight
and the job has reached a pause point, bdrv_drained_begin() can
return.
So completing the last request and reaching an actually quiescent state
looks very much like a process in child-to-parent order to me?
Kevin
- Re: [Qemu-devel] [PATCH 06/19] block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE(), (continued)
- [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, 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 <=
- 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/13
- Re: [Qemu-devel] [Qemu-block] [PATCH 07/19] block: Really pause block jobs on drain, Paolo Bonzini, 2018/04/13
- Re: [Qemu-devel] [Qemu-block] [PATCH 07/19] block: Really pause block jobs on drain, Kevin Wolf, 2018/04/13
[Qemu-devel] [PATCH 02/19] block: Use bdrv_do_drain_begin/end in bdrv_drain_all(), Kevin Wolf, 2018/04/11
[Qemu-devel] [PATCH 01/19] test-bdrv-drain: bdrv_drain() works with cross-AioContext events, Kevin Wolf, 2018/04/11
[Qemu-devel] [PATCH 11/19] test-bdrv-drain: Test node deletion in subtree recursion, Kevin Wolf, 2018/04/11