| Message ID | 1393348774-14663-2-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/25/2014 10:19 AM, Stefan Hajnoczi wrote: > Keep the thread ID around so we can report it via QMP. > > There's only one problem: qemu_get_thread_id() (gettid() wrapper on > Linux) must be called from the thread itself. There is no way to get > the thread ID outside the thread. > > This patch uses a condvar to wait for iothread_run() to populate the > thread_id inside the thread. > > + > + /* Wait for initialization to complete */ > + qemu_mutex_lock(&iothread->init_done_lock); > + qemu_cond_wait(&iothread->init_done_cond, > + &iothread->init_done_lock); > + qemu_mutex_unlock(&iothread->init_done_lock); Generally, cond_wait needs to be done in a loop, where you also check the condition (in this case, that thread_id was actually set) - a pthread implementation is allowed to do spurious wakeups even if no other thread has managed to change the condition that you were waiting for.
On Tue, Feb 25, 2014 at 10:33:49AM -0700, Eric Blake wrote: > On 02/25/2014 10:19 AM, Stefan Hajnoczi wrote: > > Keep the thread ID around so we can report it via QMP. > > > > There's only one problem: qemu_get_thread_id() (gettid() wrapper on > > Linux) must be called from the thread itself. There is no way to get > > the thread ID outside the thread. > > > > This patch uses a condvar to wait for iothread_run() to populate the > > thread_id inside the thread. > > > > > + > > + /* Wait for initialization to complete */ > > + qemu_mutex_lock(&iothread->init_done_lock); > > + qemu_cond_wait(&iothread->init_done_cond, > > + &iothread->init_done_lock); > > + qemu_mutex_unlock(&iothread->init_done_lock); > > Generally, cond_wait needs to be done in a loop, where you also check > the condition (in this case, that thread_id was actually set) - a > pthread implementation is allowed to do spurious wakeups even if no > other thread has managed to change the condition that you were waiting for. You are right, POSIX is explicit about this: "When using condition variables there is always a Boolean predicate involving shared variables associated with each condition wait that is true if the thread should proceed. Spurious wakeups from the pthread_cond_timedwait() or pthread_cond_wait() functions may occur. Since the return from pthread_cond_timedwait() or pthread_cond_wait() does not imply anything about the value of this predicate, the predicate should be re-evaluated upon such return." I figured no other thread issues pthread_cond_signal/broadcast so spurious wakeups cannot occur but I was wrong. Stefan
diff --git a/iothread.c b/iothread.c index 033de7f..36cb4cf 100644 --- a/iothread.c +++ b/iothread.c @@ -25,7 +25,10 @@ struct IOThread { Object parent; QemuThread thread; AioContext *ctx; + QemuMutex init_done_lock; + QemuCond init_done_cond; /* is thread initialization done? */ bool stopping; + int thread_id; }; #define IOTHREAD_GET_CLASS(obj) \ @@ -37,6 +40,13 @@ static void *iothread_run(void *opaque) { IOThread *iothread = opaque; + iothread->thread_id = qemu_get_thread_id(); + + /* Signal that initialization is done */ + qemu_mutex_lock(&iothread->init_done_lock); + qemu_cond_signal(&iothread->init_done_cond); + qemu_mutex_unlock(&iothread->init_done_lock); + while (!iothread->stopping) { aio_context_acquire(iothread->ctx); while (!iothread->stopping && aio_poll(iothread->ctx, true)) { @@ -54,6 +64,8 @@ static void iothread_instance_finalize(Object *obj) iothread->stopping = true; aio_notify(iothread->ctx); qemu_thread_join(&iothread->thread); + qemu_cond_destroy(&iothread->init_done_cond); + qemu_mutex_destroy(&iothread->init_done_lock); aio_context_unref(iothread->ctx); } @@ -64,11 +76,20 @@ static void iothread_complete(UserCreatable *obj, Error **errp) iothread->stopping = false; iothread->ctx = aio_context_new(); + qemu_mutex_init(&iothread->init_done_lock); + qemu_cond_init(&iothread->init_done_cond); + /* This assumes we are called from a thread with useful CPU affinity for us * to inherit. */ qemu_thread_create(&iothread->thread, iothread_run, iothread, QEMU_THREAD_JOINABLE); + + /* Wait for initialization to complete */ + qemu_mutex_lock(&iothread->init_done_lock); + qemu_cond_wait(&iothread->init_done_cond, + &iothread->init_done_lock); + qemu_mutex_unlock(&iothread->init_done_lock); } static void iothread_class_init(ObjectClass *klass, void *class_data)
Keep the thread ID around so we can report it via QMP. There's only one problem: qemu_get_thread_id() (gettid() wrapper on Linux) must be called from the thread itself. There is no way to get the thread ID outside the thread. This patch uses a condvar to wait for iothread_run() to populate the thread_id inside the thread. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- iothread.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)