From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60724 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PhkuI-0006jX-3s for qemu-devel@nongnu.org; Tue, 25 Jan 2011 10:34:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PhkuE-0007V4-KI for qemu-devel@nongnu.org; Tue, 25 Jan 2011 10:34:09 -0500 Received: from e38.co.us.ibm.com ([32.97.110.159]:52173) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PhkuE-0007Ul-6N for qemu-devel@nongnu.org; Tue, 25 Jan 2011 10:34:06 -0500 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by e38.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id p0PFJsf0005042 for ; Tue, 25 Jan 2011 08:19:54 -0700 Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p0PFY4TT092802 for ; Tue, 25 Jan 2011 08:34:04 -0700 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p0PFY4R2015289 for ; Tue, 25 Jan 2011 08:34:04 -0700 Message-ID: <4D3EED69.3030407@linux.vnet.ibm.com> Date: Tue, 25 Jan 2011 09:34:01 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 3/7] Add support for glib based threading and convert qemu thread to use it References: <1295902845-29807-1-git-send-email-aliguori@us.ibm.com> <1295902845-29807-4-git-send-email-aliguori@us.ibm.com> <20110125142448.GI23331@hall.aurel32.net> In-Reply-To: <20110125142448.GI23331@hall.aurel32.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: Stefan Hajnoczi , Marcelo Tosatti , qemu-devel@nongnu.org, Paul Brook , Arun Bharadwaj , Paulo Bonzini On 01/25/2011 08:24 AM, Aurelien Jarno wrote: > On Mon, Jan 24, 2011 at 03:00:41PM -0600, Anthony Liguori wrote: > =20 >> GLib is an extremely common library that has a portable thread impleme= ntation >> along with tons of other goodies. >> >> GLib and GObject have a fantastic amount of infrastructure we can leve= rage in >> QEMU including an object oriented programming infrastructure. >> >> Short term, it has a very nice thread pool implementation that we coul= d leverage >> in something like virtio-9p. >> >> Signed-off-by: Anthony Liguori >> >> diff --git a/Makefile b/Makefile >> index 6d601ee..bf24d1b 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -104,6 +104,8 @@ audio/audio.o audio/fmodaudio.o: QEMU_CFLAGS +=3D = $(FMOD_CFLAGS) >> >> QEMU_CFLAGS+=3D$(CURL_CFLAGS) >> >> +QEMU_CFLAGS+=3D$(GLIB_CFLAGS) >> + >> ui/cocoa.o: ui/cocoa.m >> >> ui/sdl.o audio/sdlaudio.o ui/sdl_zoom.o baum.o: QEMU_CFLAGS +=3D $(S= DL_CFLAGS) >> diff --git a/Makefile.objs b/Makefile.objs >> index c3e52c5..283e62a 100644 >> --- a/Makefile.objs >> +++ b/Makefile.objs >> @@ -316,3 +316,5 @@ vl.o: QEMU_CFLAGS+=3D$(GPROF_CFLAGS) >> >> vl.o: QEMU_CFLAGS+=3D$(SDL_CFLAGS) >> >> +vl.o: QEMU_CFLAGS+=3D$(GLIB_CFLAGS) >> + >> diff --git a/Makefile.target b/Makefile.target >> index e15b1c4..2c1c90e 100644 >> --- a/Makefile.target >> +++ b/Makefile.target >> @@ -204,6 +204,7 @@ QEMU_CFLAGS +=3D $(VNC_TLS_CFLAGS) >> QEMU_CFLAGS +=3D $(VNC_SASL_CFLAGS) >> QEMU_CFLAGS +=3D $(VNC_JPEG_CFLAGS) >> QEMU_CFLAGS +=3D $(VNC_PNG_CFLAGS) >> +QEMU_CFLAGS +=3D $(GLIB_CFLAGS) >> >> # xen backend driver support >> obj-$(CONFIG_XEN) +=3D xen_machine_pv.o xen_domainbuild.o >> diff --git a/configure b/configure >> index d5ac074..820fde9 100755 >> --- a/configure >> +++ b/configure >> @@ -1658,6 +1658,17 @@ EOF >> fi >> >> ########################################## >> +# glib support probe >> +if $pkg_config --modversion gthread-2.0> /dev/null 2>&1 ; then >> + glib_cflags=3D`$pkg_config --cflags gthread-2.0 2>/dev/null` >> + glib_libs=3D`$pkg_config --libs gthread-2.0 2>/dev/null` >> + libs_softmmu=3D"$glib_libs $libs_softmmu" >> +else >> + echo "glib-2.0 required to compile QEMU" >> + exit 1 >> +fi >> + >> +########################################## >> # kvm probe >> if test "$kvm" !=3D "no" ; then >> cat> $TMPC<> @@ -2677,6 +2688,7 @@ if test "$bluez" =3D "yes" ; then >> echo "CONFIG_BLUEZ=3Dy">> $config_host_mak >> echo "BLUEZ_CFLAGS=3D$bluez_cflags">> $config_host_mak >> fi >> +echo "GLIB_CFLAGS=3D$glib_cflags">> $config_host_mak >> if test "$xen" =3D "yes" ; then >> echo "CONFIG_XEN=3Dy">> $config_host_mak >> fi >> diff --git a/qemu-thread.c b/qemu-thread.c >> index fbc78fe..2c521ab 100644 >> --- a/qemu-thread.c >> +++ b/qemu-thread.c >> @@ -10,183 +10,142 @@ >> * See the COPYING file in the top-level directory. >> * >> */ >> -#include >> -#include >> -#include >> -#include >> -#include >> -#include >> -#include >> =20 > Why removing? > =20 It looked like unnecessary includes crept in so I remove them all. It=20 still built for me but apparently not for you. I'll update accordingly. Maybe we should put signal.h in qemu-common.h? Regards, Anthony Liguori >> -#include "qemu-thread.h" >> >> -static void error_exit(int err, const char *msg) >> -{ >> - fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err)); >> - exit(1); >> -} >> +#include "qemu-common.h" >> +#include "qemu-thread.h" >> >> void qemu_mutex_init(QemuMutex *mutex) >> { >> - int err; >> - >> - err =3D pthread_mutex_init(&mutex->lock, NULL); >> - if (err) >> - error_exit(err, __func__); >> + g_static_mutex_init(&mutex->lock); >> } >> >> void qemu_mutex_destroy(QemuMutex *mutex) >> { >> - int err; >> - >> - err =3D pthread_mutex_destroy(&mutex->lock); >> - if (err) >> - error_exit(err, __func__); >> + g_static_mutex_free(&mutex->lock); >> } >> >> void qemu_mutex_lock(QemuMutex *mutex) >> { >> - int err; >> - >> - err =3D pthread_mutex_lock(&mutex->lock); >> - if (err) >> - error_exit(err, __func__); >> + g_static_mutex_lock(&mutex->lock); >> } >> >> int qemu_mutex_trylock(QemuMutex *mutex) >> { >> - return pthread_mutex_trylock(&mutex->lock); >> -} >> - >> -static void timespec_add_ms(struct timespec *ts, uint64_t msecs) >> -{ >> - ts->tv_sec =3D ts->tv_sec + (long)(msecs / 1000); >> - ts->tv_nsec =3D (ts->tv_nsec + ((long)msecs % 1000) * 1000000); >> - if (ts->tv_nsec>=3D 1000000000) { >> - ts->tv_nsec -=3D 1000000000; >> - ts->tv_sec++; >> - } >> -} >> - >> -int qemu_mutex_timedlock(QemuMutex *mutex, uint64_t msecs) >> -{ >> - int err; >> - struct timespec ts; >> - >> - clock_gettime(CLOCK_REALTIME,&ts); >> - timespec_add_ms(&ts, msecs); >> - >> - err =3D pthread_mutex_timedlock(&mutex->lock,&ts); >> - if (err&& err !=3D ETIMEDOUT) >> - error_exit(err, __func__); >> - return err; >> + return g_static_mutex_trylock(&mutex->lock); >> } >> >> void qemu_mutex_unlock(QemuMutex *mutex) >> { >> - int err; >> - >> - err =3D pthread_mutex_unlock(&mutex->lock); >> - if (err) >> - error_exit(err, __func__); >> + g_static_mutex_unlock(&mutex->lock); >> } >> >> void qemu_cond_init(QemuCond *cond) >> { >> - int err; >> - >> - err =3D pthread_cond_init(&cond->cond, NULL); >> - if (err) >> - error_exit(err, __func__); >> + cond->cond =3D g_cond_new(); >> } >> >> void qemu_cond_destroy(QemuCond *cond) >> { >> - int err; >> - >> - err =3D pthread_cond_destroy(&cond->cond); >> - if (err) >> - error_exit(err, __func__); >> + g_cond_free(cond->cond); >> } >> >> void qemu_cond_signal(QemuCond *cond) >> { >> - int err; >> - >> - err =3D pthread_cond_signal(&cond->cond); >> - if (err) >> - error_exit(err, __func__); >> + g_cond_signal(cond->cond); >> } >> >> void qemu_cond_broadcast(QemuCond *cond) >> { >> - int err; >> - >> - err =3D pthread_cond_broadcast(&cond->cond); >> - if (err) >> - error_exit(err, __func__); >> + g_cond_broadcast(cond->cond); >> } >> >> void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) >> { >> - int err; >> - >> - err =3D pthread_cond_wait(&cond->cond,&mutex->lock); >> - if (err) >> - error_exit(err, __func__); >> + g_cond_wait(cond->cond, g_static_mutex_get_mutex(&mutex->lock)); >> } >> >> int qemu_cond_timedwait(QemuCond *cond, QemuMutex *mutex, uint64_t m= secs) >> { >> - struct timespec ts; >> - int err; >> + GTimeVal abs_time; >> + >> + assert(cond->cond !=3D NULL); >> >> - clock_gettime(CLOCK_REALTIME,&ts); >> - timespec_add_ms(&ts, msecs); >> + g_get_current_time(&abs_time); >> + g_time_val_add(&abs_time, msecs * 1000); /* MSEC to USEC */ >> >> - err =3D pthread_cond_timedwait(&cond->cond,&mutex->lock,&ts); >> - if (err&& err !=3D ETIMEDOUT) >> - error_exit(err, __func__); >> - return err; >> + return g_cond_timed_wait(cond->cond, >> + g_static_mutex_get_mutex(&mutex->lock),&= abs_time); >> +} >> + >> +struct trampoline_data >> +{ >> + QemuThread *thread; >> + void *(*startfn)(void *); >> + void *opaque; >> + QemuMutex lock; >> +}; >> + >> +static gpointer thread_trampoline(gpointer data) >> +{ >> + struct trampoline_data *td =3D data; >> + gpointer retval; >> + >> + td->thread->tid =3D pthread_self(); >> + qemu_mutex_unlock(&td->lock); >> + >> + retval =3D td->startfn(td->opaque); >> + qemu_free(td); >> + >> + return retval; >> } >> >> void qemu_thread_create(QemuThread *thread, >> - void *(*start_routine)(void*), >> - void *arg) >> + void *(*start_routine)(void*), >> + void *arg) >> { >> - int err; >> + struct trampoline_data *td =3D qemu_malloc(sizeof(*td)); >> + sigset_t set, old; >> >> - /* Leave signal handling to the iothread. */ >> - sigset_t set, oldset; >> + td->startfn =3D start_routine; >> + td->opaque =3D arg; >> + td->thread =3D thread; >> + qemu_mutex_init(&td->lock); >> + >> + /* on behalf of the new thread */ >> + qemu_mutex_lock(&td->lock); >> >> sigfillset(&set); >> =20 > If it is still in use here? This simply doesn't build: > > qemu-thread.c: In function =E2=80=98q_thread_create_nosignal=E2=80=99: > qemu-thread.c:34: error: implicit declaration of function =E2=80=98sigf= illset=E2=80=99 > qemu-thread.c:34: error: nested extern declaration of =E2=80=98sigfills= et=E2=80=99 > qemu-thread.c:35: error: implicit declaration of function =E2=80=98pthr= ead_sigmask=E2=80=99 > qemu-thread.c:35: error: nested extern declaration of =E2=80=98pthread_= sigmask=E2=80=99 > qemu-thread.c:35: error: =E2=80=98SIG_SETMASK=E2=80=99 undeclared (firs= t use in this function) > qemu-thread.c:35: error: (Each undeclared identifier is reported only o= nce > qemu-thread.c:35: error: for each function it appears in.) > qemu-thread.c: In function =E2=80=98qemu_sthread_create=E2=80=99: > qemu-thread.c:80: error: =E2=80=98SIG_SETMASK=E2=80=99 undeclared (firs= t use in this function) > qemu-thread.c: In function =E2=80=98qemu_sthread_signal=E2=80=99: > qemu-thread.c:96: error: implicit declaration of function =E2=80=98pthr= ead_kill=E2=80=99 > qemu-thread.c:96: error: nested extern declaration of =E2=80=98pthread_= kill=E2=80=99 > > > =20 >> - pthread_sigmask(SIG_SETMASK,&set,&oldset); >> - err =3D pthread_create(&thread->thread, NULL, start_routine, arg)= ; >> - if (err) >> - error_exit(err, __func__); >> + pthread_sigmask(SIG_SETMASK,&set,&old); >> + thread->thread =3D g_thread_create(thread_trampoline, td, TRUE, N= ULL); >> + pthread_sigmask(SIG_SETMASK,&old, NULL); >> + >> + /* we're transfering ownership of this lock to the thread so we n= o >> + * longer hold it here */ >> >> - pthread_sigmask(SIG_SETMASK,&oldset, NULL); >> + qemu_mutex_lock(&td->lock); >> + /* validate tid */ >> + qemu_mutex_unlock(&td->lock); >> + >> + qemu_mutex_destroy(&td->lock); >> } >> >> void qemu_thread_signal(QemuThread *thread, int sig) >> { >> - int err; >> - >> - err =3D pthread_kill(thread->thread, sig); >> - if (err) >> - error_exit(err, __func__); >> + pthread_kill(thread->tid, sig); >> } >> >> void qemu_thread_self(QemuThread *thread) >> { >> - thread->thread =3D pthread_self(); >> + thread->thread =3D g_thread_self(); >> + thread->tid =3D pthread_self(); >> } >> >> int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2) >> { >> - return pthread_equal(thread1->thread, thread2->thread); >> + return (thread1->thread =3D=3D thread2->thread); >> } >> >> void qemu_thread_exit(void *retval) >> { >> - pthread_exit(retval); >> + g_thread_exit(retval); >> } >> diff --git a/qemu-thread.h b/qemu-thread.h >> index 19bb30c..dc22a60 100644 >> --- a/qemu-thread.h >> +++ b/qemu-thread.h >> @@ -1,18 +1,19 @@ >> #ifndef __QEMU_THREAD_H >> #define __QEMU_THREAD_H 1 >> -#include "semaphore.h" >> -#include "pthread.h" >> +#include >> +#include >> >> struct QemuMutex { >> - pthread_mutex_t lock; >> + GStaticMutex lock; >> }; >> >> struct QemuCond { >> - pthread_cond_t cond; >> + GCond *cond; >> }; >> >> struct QemuThread { >> - pthread_t thread; >> + GThread *thread; >> + pthread_t tid; >> }; >> >> typedef struct QemuMutex QemuMutex; >> diff --git a/vl.c b/vl.c >> index 0292184..bbe0931 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -165,6 +165,8 @@ int main(int argc, char **argv) >> >> #include "ui/qemu-spice.h" >> >> +#include >> + >> //#define DEBUG_NET >> //#define DEBUG_SLIRP >> >> @@ -1918,6 +1920,8 @@ int main(int argc, char **argv, char **envp) >> atexit(qemu_run_exit_notifiers); >> error_set_progname(argv[0]); >> >> + g_thread_init(NULL); >> + >> init_clocks(); >> >> qemu_cache_utils_init(envp); >> --=20 >> 1.7.0.4 >> >> >> >> =20 > =20