Re: [Qemu-devel] [PATCH v2 2/3] aio: use g_slice_alloc() for AIOCB pooling

[Paolo Bonzini]

Il 31/10/2012 16:34, Stefan Hajnoczi ha scritto:
> AIO control blocks are frequently acquired and released because each aio
> request involves at least one AIOCB.  Therefore, we pool them to avoid
> heap allocation overhead.
> 
> The problem with the freelist approach in AIOPool is thread-safety.  If
> we want BlockDriverStates to associate with AioContexts that execute in
> multiple threads, then a global freelist becomes a problem.
> 
> This patch drops the freelist and instead uses g_slice_alloc() which is
> tuned for per-thread fixed-size object pools.  qemu_aio_get() and
> qemu_aio_release() are now thread-safe.
> 
> Note that the change from g_malloc0() to g_slice_alloc() should be safe
> since the freelist reuse case doesn't zero the AIOCB either.
> 
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  block.c    | 15 ++++-----------
>  qemu-aio.h |  2 --
>  2 files changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/block.c b/block.c
> index da1fdca..ea0f7d8 100644
> --- a/block.c
> +++ b/block.c
> @@ -3909,13 +3909,8 @@ void *qemu_aio_get(AIOPool *pool, BlockDriverState *bs,
>  {
>      BlockDriverAIOCB *acb;
>  
> -    if (pool->free_aiocb) {
> -        acb = pool->free_aiocb;
> -        pool->free_aiocb = acb->next;
> -    } else {
> -        acb = g_malloc0(pool->aiocb_size);
> -        acb->pool = pool;
> -    }
> +    acb = g_slice_alloc(pool->aiocb_size);
> +    acb->pool = pool;
>      acb->bs = bs;
>      acb->cb = cb;
>      acb->opaque = opaque;
> @@ -3924,10 +3919,8 @@ void *qemu_aio_get(AIOPool *pool, BlockDriverState *bs,
>  
>  void qemu_aio_release(void *p)
>  {
> -    BlockDriverAIOCB *acb = (BlockDriverAIOCB *)p;
> -    AIOPool *pool = acb->pool;
> -    acb->next = pool->free_aiocb;
> -    pool->free_aiocb = acb;
> +    BlockDriverAIOCB *acb = p;
> +    g_slice_free1(acb->pool->aiocb_size, acb);
>  }
>  
>  /**************************************************************/
> diff --git a/qemu-aio.h b/qemu-aio.h
> index 111b0b3..b29c509 100644
> --- a/qemu-aio.h
> +++ b/qemu-aio.h
> @@ -24,7 +24,6 @@ typedef void BlockDriverCompletionFunc(void *opaque, int 
> ret);
>  typedef struct AIOPool {
>      void (*cancel)(BlockDriverAIOCB *acb);
>      size_t aiocb_size;
> -    BlockDriverAIOCB *free_aiocb;
>  } AIOPool;
>  
>  struct BlockDriverAIOCB {
> @@ -32,7 +31,6 @@ struct BlockDriverAIOCB {
>      BlockDriverState *bs;
>      BlockDriverCompletionFunc *cb;
>      void *opaque;
> -    BlockDriverAIOCB *next;
>  };
>  
>  void *qemu_aio_get(AIOPool *pool, BlockDriverState *bs,
> 

Reviewed-by: Paolo Bonzini <address@hidden>