[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->childr
From: |
Emanuele Giuseppe Esposito |
Subject: |
Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept |
Date: |
Mon, 4 Apr 2022 11:51:51 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
Am 04/04/2022 um 11:41 schrieb Paolo Bonzini:
>
>
> On Mon, Apr 4, 2022 at 11:25 AM Stefan Hajnoczi <stefanha@redhat.com
> <mailto:stefanha@redhat.com>> wrote:
>
> - The new API doesn't stop more I/O requests from being submitted, it
> just blocks the current coroutine so request processing is deferred.
>
>
> New I/O requests would not complete until the write-side critical
> section ends. However they would still be accepted: from the point of
> view of the guest, the "consumed" index of the virtio ring would move
> forward, unlike bdrv_drained_begin/end().
>
> - In other words, is_external is a flow control API whereas the new API
> queues up request coroutines without notifying the caller.
>
>
> Yes, I think this is the same I wrote above.
>
> - The new API still needs to be combined with bdrv_drained_begin/end()
> to ensure in-flight requests are done.
>
>
> I don't think so, because in-flight requests would take the lock for
> reading. The write side would not start until those in-flight requests
> release the lock.
>
> - It's not obvious to me whether the new API obsoletes is_external.
> I think it probably doesn't.
>
>
> I agree that it doesn't. This new lock is only protecting ->parents and
> ->children.
Side note: it will also be used to protect other fields, like
.aio_context I think. I haven't checked if there is something else we
might want to protect that is currently protected by AioContext lock.
At least, I think we are going to use the same lock, right?
Emanuele
bdrv_drained_begin()/end() remains necessary, for example,
> when you need to send a request during the drained section. An example
> is block_resize.
>
> In addition, bdrv_drained_begin()/end() ensures that the callback of
> blk_aio_*() functions has been invoked (see commit 46aaf2a566,
> "block-backend: Decrease in_flight only after callback", 2018-09-25).
> This new lock would not ensure that.
>
> As an aside, instead of is_external, QEMU could remove/add the ioeventfd
> handler in the blk->dev_ops->drained_begin and blk->dev_ops->drained_end
> callbacks respectively. But that's just a code cleanup.
>
> Paolo
- Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept, Emanuele Giuseppe Esposito, 2022/04/01
- Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept, Emanuele Giuseppe Esposito, 2022/04/13
- Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept, Kevin Wolf, 2022/04/13
- Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept, Emanuele Giuseppe Esposito, 2022/04/13
- Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept, Emanuele Giuseppe Esposito, 2022/04/13
- Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept, Kevin Wolf, 2022/04/13
- Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept, Paolo Bonzini, 2022/04/13
- Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept, Paolo Bonzini, 2022/04/13