Re: [Qemu-devel] [PATCH v2 1/3] Make thread pool implementation modular

2013-11-11 Thread Kevin Wolf
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



[Qemu-devel] [PATCH v2 1/3] Make thread pool implementation modular

2013-11-04 Thread Matthias Brugger
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
---
 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 {
+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)
+{
+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;
+}
+
+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);
+}
-- 
1.8.1.2