Re: [Qemu-devel] [PATCH v2 0/7] threadpool: support multiple ThreadPools

[Paolo Bonzini <pbonzini@redhat.com>]

Il 07/03/2013 13:41, Stefan Hajnoczi ha scritto:
> This patch series changes the global thread pool to a one ThreadPool per
> AioContext model.  We still only use the main loop AioContext so in practice
> there is just one ThreadPool.  But this opens the door to refactoring the block
> layer (which depends on ThreadPool) so block devices can be accessed outside
> the global mutex in the future.
> 
> ThreadPool is tightly bound to an AioContext because it uses an EventNotifier
> to signal work completion.  Completed work items are reaped and their callback
> functions are invoked from the EventNotifier read handler (executing under
> AioContext).
> 
> It might be possible to record the AioContext for the completion callback on a
> per-request basis and continuing to use a global pool of worker threads.  After
> discussing thread pool models with Paolo I have been convinced that it is
> simpler and more scalable to have one ThreadPool per AioContext instead.
> Therefore this series implements the 1:1 approach.  For details on previous
> thread pool model discussion, see:
> http://lists.gnu.org/archive/html/qemu-devel/2013-02/msg03987.html
> 
> The final patch was previously separate but I have included it because it
> depends on qemu_get_aio_context().  It is unrelated to ThreadPool but the patch
> reviewers are the same in both instances, so I combined the series.
> 
> At the end of this series block/raw-posix.c and block/raw-win32.c are aware of
> the ThreadPool they submit work to.  The next step after this series is to
> associate BlockDriverState with an AioContext so that the block layer can run
> outside the global main loop.
> 
> v2:
>  * Always find AioContext, don't split if (ctx) cases [Paolo]
>  * Introduce bdrv_get_aio_context() [Paolo]
>  * Add CoQueue AioContext patch since it depends on qemu_get_aio_context()
> 
> Stefan Hajnoczi (7):
>   main-loop: add qemu_get_aio_context()
>   threadpool: move globals into struct ThreadPool
>   threadpool: add thread_pool_new() and thread_pool_free()
>   aio: add a ThreadPool instance to AioContext
>   block: add bdrv_get_aio_context()
>   threadpool: drop global thread pool
>   coroutine: use AioContext for CoQueue BH
> 
>  async.c                     |  11 ++
>  block.c                     |   6 ++
>  block/raw-posix.c           |   8 +-
>  block/raw-win32.c           |   4 +-
>  include/block/aio.h         |   6 ++
>  include/block/block_int.h   |   7 ++
>  include/block/coroutine.h   |   1 +
>  include/block/thread-pool.h |  15 ++-
>  include/qemu/main-loop.h    |   5 +
>  main-loop.c                 |   5 +
>  qemu-coroutine-lock.c       |  55 ++++++----
>  tests/test-thread-pool.c    |  44 ++++----
>  thread-pool.c               | 243 ++++++++++++++++++++++++++++----------------
>  trace-events                |   4 +-
>  14 files changed, 276 insertions(+), 138 deletions(-)
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>