Am 20.09.2011 18:53, schrieb Jan Kiszka: > Although there is nothing to wrap for non-POSIX here, redirecting thread > and synchronization services to our core simplifies managements jobs > like scheduling parameter adjustment. It also frees compat AIO from some > duplicate code (/wrt qemu-thread). > > CC: Kevin Wolf <kw...@redhat.com> > Signed-off-by: Jan Kiszka <jan.kis...@siemens.com> > --- > posix-aio-compat.c | 115 ++++++++++++++------------------------------------- > 1 files changed, 32 insertions(+), 83 deletions(-) > > diff --git a/posix-aio-compat.c b/posix-aio-compat.c > index d3c1174..0715aba 100644 > --- a/posix-aio-compat.c > +++ b/posix-aio-compat.c > @@ -13,7 +13,6 @@ > > #include <sys/ioctl.h> > #include <sys/types.h> > -#include <pthread.h> > #include <unistd.h> > #include <errno.h> > #include <time.h> > @@ -27,9 +26,12 @@ > #include "qemu-common.h" > #include "trace.h" > #include "block_int.h" > +#include "qemu-thread.h" > > #include "block/raw-posix-aio.h" > > +#define AIO_THREAD_IDLE_TIMEOUT 10000 /* 10 s */ > + > static void do_spawn_thread(void); > > struct qemu_paiocb { > @@ -57,10 +59,9 @@ typedef struct PosixAioState { > } PosixAioState; > > > -static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; > -static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; > -static pthread_t thread_id; > -static pthread_attr_t attr; > +static QemuMutex lock; > +static QemuCond cond; > +static QemuThread thread; > static int max_threads = 64; > static int cur_threads = 0; > static int idle_threads = 0; > @@ -86,39 +87,6 @@ static void die(const char *what) > die2(errno, what); > } > > -static void mutex_lock(pthread_mutex_t *mutex) > -{ > - int ret = pthread_mutex_lock(mutex); > - if (ret) die2(ret, "pthread_mutex_lock"); > -} > - > -static void mutex_unlock(pthread_mutex_t *mutex) > -{ > - int ret = pthread_mutex_unlock(mutex); > - if (ret) die2(ret, "pthread_mutex_unlock"); > -} > - > -static int cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex, > - struct timespec *ts) > -{ > - int ret = pthread_cond_timedwait(cond, mutex, ts); > - if (ret && ret != ETIMEDOUT) die2(ret, "pthread_cond_timedwait"); > - return ret; > -} > - > -static void cond_signal(pthread_cond_t *cond) > -{ > - int ret = pthread_cond_signal(cond); > - if (ret) die2(ret, "pthread_cond_signal"); > -} > - > -static void thread_create(pthread_t *thread, pthread_attr_t *attr, > - void *(*start_routine)(void*), void *arg) > -{ > - int ret = pthread_create(thread, attr, start_routine, arg); > - if (ret) die2(ret, "pthread_create"); > -} > - > static ssize_t handle_aiocb_ioctl(struct qemu_paiocb *aiocb) > { > int ret; > @@ -311,27 +279,22 @@ static void posix_aio_notify_event(void); > > static void *aio_thread(void *unused) > { > - mutex_lock(&lock); > + qemu_mutex_lock(&lock); > pending_threads--; > - mutex_unlock(&lock); > + qemu_mutex_unlock(&lock); > do_spawn_thread(); > > while (1) { > struct qemu_paiocb *aiocb; > - ssize_t ret = 0; > - qemu_timeval tv; > - struct timespec ts; > - > - qemu_gettimeofday(&tv); > - ts.tv_sec = tv.tv_sec + 10; > - ts.tv_nsec = 0; > + bool timed_out = false; > + ssize_t ret; > > - mutex_lock(&lock); > + qemu_mutex_lock(&lock); > > - while (QTAILQ_EMPTY(&request_list) && > - !(ret == ETIMEDOUT)) { > + while (QTAILQ_EMPTY(&request_list) && !timed_out) { > idle_threads++; > - ret = cond_timedwait(&cond, &lock, &ts); > + timed_out = qemu_cond_timedwait(&cond, &lock, > + AIO_THREAD_IDLE_TIMEOUT) != 0;
Maybe I'm confused by too many negations, but isn't this the wrong way round? + err = pthread_cond_timedwait(&cond->cond, &mutex->lock, &ts); + if (err && err != ETIMEDOUT) { + error_exit(err, __func__); + } + return err == 0; So if there was an timeout, qemu_cond_timedwait returns 0 (should it return a bool? Also documenting the return value wouldn't hurt) and timed_out becomes false (0 != 0). Kevin