diff mbox series

[01/12] introduce BDRV_POLL_WHILE_UNLOCKED

Message ID 20220118162738.1366281-2-eesposit@redhat.com
State New
Headers show
Series Removal of Aiocontext lock through drains: protect bdrv_replace_child_noperm. | expand

Commit Message

Emanuele Giuseppe Esposito Jan. 18, 2022, 4:27 p.m. UTC
Same as BDRV_POLL_WHILE, but uses AIO_WAIT_WHILE_UNLOCKED.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 include/block/block-global-state.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Stefan Hajnoczi Jan. 26, 2022, 10:49 a.m. UTC | #1
On Tue, Jan 18, 2022 at 11:27:27AM -0500, Emanuele Giuseppe Esposito wrote:
> Same as BDRV_POLL_WHILE, but uses AIO_WAIT_WHILE_UNLOCKED.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>  include/block/block-global-state.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
> index 419fe8427f..7ad9496f56 100644
> --- a/include/block/block-global-state.h
> +++ b/include/block/block-global-state.h
> @@ -158,6 +158,11 @@ void bdrv_drain_all(void);
>      AIO_WAIT_WHILE(bdrv_get_aio_context(bs_),              \
>                     cond); })
>  
> +#define BDRV_POLL_WHILE_UNLOCKED(bs, cond) ({              \
> +    BlockDriverState *bs_ = (bs);                          \
> +    AIO_WAIT_WHILE_UNLOCKED(bdrv_get_aio_context(bs_),     \
> +                            cond); })

No doc comments? When and why is this API useful? Are there any
preconditions or assumptions (e.g. cond must be thread-safe)?

Stefan
Emanuele Giuseppe Esposito Feb. 3, 2022, 1:57 p.m. UTC | #2
On 26/01/2022 11:49, Stefan Hajnoczi wrote:
> On Tue, Jan 18, 2022 at 11:27:27AM -0500, Emanuele Giuseppe Esposito wrote:
>> Same as BDRV_POLL_WHILE, but uses AIO_WAIT_WHILE_UNLOCKED.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> ---
>>  include/block/block-global-state.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
>> index 419fe8427f..7ad9496f56 100644
>> --- a/include/block/block-global-state.h
>> +++ b/include/block/block-global-state.h
>> @@ -158,6 +158,11 @@ void bdrv_drain_all(void);
>>      AIO_WAIT_WHILE(bdrv_get_aio_context(bs_),              \
>>                     cond); })
>>  
>> +#define BDRV_POLL_WHILE_UNLOCKED(bs, cond) ({              \
>> +    BlockDriverState *bs_ = (bs);                          \
>> +    AIO_WAIT_WHILE_UNLOCKED(bdrv_get_aio_context(bs_),     \
>> +                            cond); })
> 
> No doc comments? When and why is this API useful? Are there any
> preconditions or assumptions (e.g. cond must be thread-safe)?
> 
I am planning to add the following comment above the macro:

/*
 * Unlocked counterpart of BDRV_POLL_WHILE. Uses AIO_WAIT_WHILE_UNLOCKED,
 * so it does not release+acquire the AioContext lock if we are in the
 * main loop, therefore the caller does not need to take it.
 * This function avoids taking the AioContext lock unnecessarly, so use
 * it only when sure that the lock is not taken already, otherwise it
 * might cause deadlocks.
 *
 * @cond must be thread-safe.
 */

Emanuele
Paolo Bonzini Feb. 4, 2022, 12:13 p.m. UTC | #3
On 2/3/22 14:57, Emanuele Giuseppe Esposito wrote:
>   * This function avoids taking the AioContext lock unnecessarly, so use
>   * it only when sure that the lock is not taken already, otherwise it
>   * might cause deadlocks.

"so use" should be "but use". :)

Paolo

>   *
>   * @cond must be thread-safe.
>   */
diff mbox series

Patch

diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index 419fe8427f..7ad9496f56 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -158,6 +158,11 @@  void bdrv_drain_all(void);
     AIO_WAIT_WHILE(bdrv_get_aio_context(bs_),              \
                    cond); })
 
+#define BDRV_POLL_WHILE_UNLOCKED(bs, cond) ({              \
+    BlockDriverState *bs_ = (bs);                          \
+    AIO_WAIT_WHILE_UNLOCKED(bdrv_get_aio_context(bs_),     \
+                            cond); })
+
 int bdrv_has_zero_init_1(BlockDriverState *bs);
 int bdrv_has_zero_init(BlockDriverState *bs);
 int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only,