Am 04.11.2013 um 11:28 hat Matthias Brugger geschrieben: > This patch introduces function pointers for the thread pool, so that > it's implementation can be set at run-time. > > Signed-off-by: Matthias Brugger <matthias....@gmail.com>
Like Stefan said, this can really only be given a meaningful review in context with an actual user of the infrastructure, but I'll mention some minor points in this series anyway. > include/block/thread-pool.h | 11 +++++++++++ > thread-pool.c | 33 +++++++++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > > diff --git a/include/block/thread-pool.h b/include/block/thread-pool.h > index 32afcdd..1f73712 100644 > --- a/include/block/thread-pool.h > +++ b/include/block/thread-pool.h > @@ -38,4 +38,15 @@ int coroutine_fn thread_pool_submit_co(ThreadPool *pool, > ThreadPoolFunc *func, void *arg); > void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg); > > +ThreadPoolFuncArr *thread_pool_probe(void); > +void thread_pool_delete(ThreadPoolFuncArr *tpf); > + > +struct ThreadPoolFuncArr { If "Arr" is supposed to mean "array", this isn't really one. Similar structs containing function pointers have names ending in Info (AIOCBInfo, NetClientInfo) or Driver (BlockDriver). > + BlockDriverAIOCB *(*thread_pool_submit_aio)(ThreadPool *pool, > + ThreadPoolFunc *func, void *arg, BlockDriverCompletionFunc *cb, > + void *opaque); > + ThreadPool *(*thread_pool_new)(AioContext *ctx); > +}; > + > + > #endif > diff --git a/thread-pool.c b/thread-pool.c > index 3735fd3..53294a9 100644 > --- a/thread-pool.c > +++ b/thread-pool.c > @@ -26,6 +26,7 @@ > #include "qemu/main-loop.h" > > static void do_spawn_thread(ThreadPool *pool); > +static void thread_pool_aio_free(ThreadPool *pool); > > typedef struct ThreadPoolElement ThreadPoolElement; > > @@ -77,6 +78,7 @@ struct ThreadPool { > int pending_threads; /* threads created but not running yet */ > int pending_cancellations; /* whether we need a cond_broadcast */ > bool stopping; > + void (*thread_pool_free)(ThreadPool *pool); > }; > > static void *worker_thread(void *opaque) > @@ -300,6 +302,7 @@ static void thread_pool_init_one(ThreadPool *pool, > AioContext *ctx) > qemu_sem_init(&pool->sem, 0); > pool->max_threads = 64; > pool->new_thread_bh = aio_bh_new(ctx, spawn_thread_bh_fn, pool); > + pool->thread_pool_free = &thread_pool_aio_free; > > QLIST_INIT(&pool->head); > QTAILQ_INIT(&pool->request_list); > @@ -316,6 +319,11 @@ ThreadPool *thread_pool_new(AioContext *ctx) > > void thread_pool_free(ThreadPool *pool) > { > + pool->thread_pool_free(pool); > +} > + > +void thread_pool_aio_free(ThreadPool *pool) > +{ > if (!pool) { > return; > } > @@ -346,3 +354,28 @@ void thread_pool_free(ThreadPool *pool) > event_notifier_cleanup(&pool->notifier); > g_free(pool); > } > + > +ThreadPoolFuncArr *thread_pool_probe(void) What is probed here? Isn't it just a function that creates a thread pool? > +{ > + ThreadPoolFuncArr *tpf_pool = NULL; > + > + if (tpf_pool) { > + return tpf_pool; > + } > + > + tpf_pool = g_new(ThreadPoolFuncArr, 1); > + if (!tpf_pool) { > + printf("error allocating thread pool\n"); > + return NULL; > + } g_new() doesn't fail. > + > + tpf_pool->thread_pool_submit_aio = thread_pool_submit_aio; > + tpf_pool->thread_pool_new = thread_pool_new; > + > + return tpf_pool; > +} > + > +void thread_pool_delete(ThreadPoolFuncArr *tpf) > +{ > + g_free(tpf); > +} Kevin