[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 00/12] Removal of Aiocontext lock through drains: protect bdr
From: |
Emanuele Giuseppe Esposito |
Subject: |
Re: [PATCH 00/12] Removal of Aiocontext lock through drains: protect bdrv_replace_child_noperm. |
Date: |
Fri, 28 Jan 2022 13:20:28 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
On 27/01/2022 14:46, Paolo Bonzini wrote:
> On 1/26/22 12:29, Stefan Hajnoczi wrote:
>> Still, I'm a bit surprised I didn't notice any
>> aio_context_acquire/release() removals in this patch series because I
>> thought callers need that before they switch to
>> BDRV_POLL_WHILE_UNLOCKED()?
>
> I think the callers are new and were not calling
> bdrv_subtree_drained_begin() (and thus BDRV_POLL_WHILE) at all?
>
> Emanuele, enlighten us. :)
Yes, the callers were not calling any kind of drains (or at least most
of them) *and* no context lock was acquired before calling them.
The current logic (or at least how I see it) when draining is:
"ok, we need to use bdrv_drain or bdrv_subtree_drain, but these function
call BDRV_POLL_WHILE(), which in turns calls AIO_WAIT_WHILE and thus
performs aio_context_release(lock); [...] aio_context_acquire(lock);
*Therefore* we need to acquire the lock". The lock is taken as a
consequence of the drain implementation.
This makes the lock usage useless, because we are just blindly acquiring
it for the purpose of making BDRV_POLL_WHILE happy.
On the other side, here no lock was acquired before, and
BDRV_POLL_WHILE_UNLOCKED is not releasing anything, thus no lock is needed.
This seems to hold and kinda proves my logic above.
Thank you,
Emanuele
- [PATCH 02/12] block/io.c: make bdrv_do_drained_begin_quiesce static and introduce bdrv_drained_begin_no_poll, (continued)
- [PATCH 02/12] block/io.c: make bdrv_do_drained_begin_quiesce static and introduce bdrv_drained_begin_no_poll, Emanuele Giuseppe Esposito, 2022/01/18
- [PATCH 04/12] block.c: bdrv_replace_child_noperm: first call ->attach(), and then add child, Emanuele Giuseppe Esposito, 2022/01/18
- [PATCH 07/12] block/io.c: introduce bdrv_subtree_drained_{begin/end}_unlocked, Emanuele Giuseppe Esposito, 2022/01/18
- [PATCH 11/12] block/io.c: fully enable assert_bdrv_graph_writable, Emanuele Giuseppe Esposito, 2022/01/18
- Re: [PATCH 00/12] Removal of Aiocontext lock through drains: protect bdrv_replace_child_noperm., Paolo Bonzini, 2022/01/19
- Re: [PATCH 00/12] Removal of Aiocontext lock through drains: protect bdrv_replace_child_noperm., Stefan Hajnoczi, 2022/01/26