Re: [PATCH v2 1/6] block: don't acquire AioContext lock in bdrv_drain_all()

2023-03-12 Thread Wilfred Mallawa
On Thu, 2023-03-09 at 14:08 -0500, Stefan Hajnoczi wrote:
> There is no need for the AioContext lock in bdrv_drain_all() because
> nothing in AIO_WAIT_WHILE() needs the lock and the condition is
> atomic.
> 
> AIO_WAIT_WHILE_UNLOCKED() has no use for the AioContext parameter
> other
> than performing a check that is nowadays already done by the
> GLOBAL_STATE_CODE()/IO_CODE() macros. Set the ctx argument to NULL
> here
> to help us keep track of all converted callers. Eventually all
> callers
> will have been converted and then the argument can be dropped
> entirely.
> 
> Reviewed-by: Kevin Wolf 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  block/block-backend.c | 8 +---
>  1 file changed, 1 insertion(+), 7 deletions(-)
Reviewed-by: Wilfred Mallawa 
> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 278b04ce69..d2b6b3652d 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1835,14 +1835,8 @@ void blk_drain_all(void)
>  bdrv_drain_all_begin();
>  
>  while ((blk = blk_all_next(blk)) != NULL) {
> -    AioContext *ctx = blk_get_aio_context(blk);
> -
> -    aio_context_acquire(ctx);
> -
>  /* We may have -ENOMEDIUM completions in flight */
> -    AIO_WAIT_WHILE(ctx, qatomic_mb_read(&blk->in_flight) > 0);
> -
> -    aio_context_release(ctx);
> +    AIO_WAIT_WHILE_UNLOCKED(NULL, qatomic_mb_read(&blk-
> >in_flight) > 0);
>  }
>  
>  bdrv_drain_all_end();



[PATCH v2 1/6] block: don't acquire AioContext lock in bdrv_drain_all()

2023-03-09 Thread Stefan Hajnoczi
There is no need for the AioContext lock in bdrv_drain_all() because
nothing in AIO_WAIT_WHILE() needs the lock and the condition is atomic.

AIO_WAIT_WHILE_UNLOCKED() has no use for the AioContext parameter other
than performing a check that is nowadays already done by the
GLOBAL_STATE_CODE()/IO_CODE() macros. Set the ctx argument to NULL here
to help us keep track of all converted callers. Eventually all callers
will have been converted and then the argument can be dropped entirely.

Reviewed-by: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
---
 block/block-backend.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 278b04ce69..d2b6b3652d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1835,14 +1835,8 @@ void blk_drain_all(void)
 bdrv_drain_all_begin();
 
 while ((blk = blk_all_next(blk)) != NULL) {
-AioContext *ctx = blk_get_aio_context(blk);
-
-aio_context_acquire(ctx);
-
 /* We may have -ENOMEDIUM completions in flight */
-AIO_WAIT_WHILE(ctx, qatomic_mb_read(&blk->in_flight) > 0);
-
-aio_context_release(ctx);
+AIO_WAIT_WHILE_UNLOCKED(NULL, qatomic_mb_read(&blk->in_flight) > 0);
 }
 
 bdrv_drain_all_end();
-- 
2.39.2