Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1477 bytes --]

On Mon, Apr 04, 2022 at 11:41:04AM +0200, Paolo Bonzini wrote:
> On Mon, Apr 4, 2022 at 11:25 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > - 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.

Good point!

> - 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. 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.

Interesting idea. If is_external is a block layer-specific feature that
nothing else in QEMU uses then I like the idea because it's cleaner and
more obvious than is_external.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]