From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LkilG-0002e3-BI for qemu-devel@nongnu.org; Fri, 20 Mar 2009 13:44:02 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Lkil9-0002bo-9i for qemu-devel@nongnu.org; Fri, 20 Mar 2009 13:43:59 -0400 Received: from [199.232.76.173] (port=60238 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Lkil9-0002bj-1H for qemu-devel@nongnu.org; Fri, 20 Mar 2009 13:43:55 -0400 Received: from mail-qy0-f111.google.com ([209.85.221.111]:52263) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Lkil8-0006cW-Jy for qemu-devel@nongnu.org; Fri, 20 Mar 2009 13:43:54 -0400 Received: by qyk9 with SMTP id 9so1412127qyk.4 for ; Fri, 20 Mar 2009 10:43:53 -0700 (PDT) Message-ID: <49C3D5D5.4020006@codemonkey.ws> Date: Fri, 20 Mar 2009 12:43:49 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <20090319145705.988576615@localhost.localdomain> <20090319150537.910101569@localhost.localdomain> In-Reply-To: <20090319150537.910101569@localhost.localdomain> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [patch 2/7] qemu: separate thread for io Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mtosatti@redhat.com Cc: qemu-devel@nongnu.org mtosatti@redhat.com wrote: > Index: qemu/exec.c > =================================================================== > --- qemu.orig/exec.c > +++ qemu/exec.c > @@ -1513,6 +1513,20 @@ void cpu_interrupt(CPUState *env, int ma > /* FIXME: This is probably not threadsafe. A different thread could > be in the middle of a read-modify-write operation. */ > env->interrupt_request |= mask; > + > + switch(mask) { > + case CPU_INTERRUPT_HARD: > + case CPU_INTERRUPT_SMI: > + case CPU_INTERRUPT_NMI: > + case CPU_INTERRUPT_EXIT: > + /* > + * only unlink the TB's if we're called from cpu thread context, > + * otherwise signal cpu thread to do it. > + */ > + if (qemu_notify_event(env)) > + return; > + } > + > #if defined(USE_NPTL) > /* FIXME: TB unchaining isn't SMP safe. For now just ignore the > problem and hope the cpu will stop of its own accord. For userspace > Index: qemu/qemu-common.h > =================================================================== > --- qemu.orig/qemu-common.h > +++ qemu/qemu-common.h > @@ -190,6 +190,8 @@ int cpu_load(QEMUFile *f, void *opaque, > > /* Force QEMU to stop what it's doing and service IO */ > void qemu_service_io(void); > +void main_loop_break(void); > +int qemu_notify_event(void *env); > > typedef struct QEMUIOVector { > struct iovec *iov; > Index: qemu/vl.c > =================================================================== > --- qemu.orig/vl.c > +++ qemu/vl.c > @@ -155,6 +155,7 @@ > //#define DEBUG_NET > //#define DEBUG_SLIRP > > +#define SIG_IPI (SIGRTMIN+4) > > #ifdef DEBUG_IOPORT > # define LOG_IOPORT(...) qemu_log_mask(CPU_LOG_IOPORT, ## __VA_ARGS__) > @@ -265,6 +266,10 @@ static QEMUTimer *nographic_timer; > uint8_t qemu_uuid[16]; > > QemuMutex qemu_global_mutex; > +QemuMutex qemu_fair_mutex; > + > +QemuThread io_thread; > +QemuThread cpus_thread; > > /***********************************************************/ > /* x86 ISA bus support */ > @@ -1328,7 +1333,6 @@ static void host_alarm_handler(int host_ > qemu_get_clock(vm_clock))) || > qemu_timer_expired(active_timers[QEMU_TIMER_REALTIME], > qemu_get_clock(rt_clock))) { > - CPUState *env = next_cpu; > > #ifdef _WIN32 > struct qemu_alarm_win32 *data = ((struct qemu_alarm_timer*)dwUser)->priv; > @@ -1339,15 +1343,6 @@ static void host_alarm_handler(int host_ > #endif > alarm_timer->flags |= ALARM_FLAG_EXPIRED; > > - if (env) { > - /* stop the currently executing cpu because a timer occured */ > - cpu_interrupt(env, CPU_INTERRUPT_EXIT); > -#ifdef USE_KQEMU > - if (env->kqemu_enabled) { > - kqemu_cpu_interrupt(env); > - } > -#endif > - } > event_pending = 1; > } > } > @@ -2878,6 +2873,7 @@ int qemu_set_fd_handler2(int fd, > ioh->opaque = opaque; > ioh->deleted = 0; > } > + main_loop_break(); > return 0; > } > > @@ -3334,6 +3330,7 @@ void qemu_bh_schedule(QEMUBH *bh) > if (env) { > cpu_interrupt(env, CPU_INTERRUPT_EXIT); > } > + main_loop_break(); > } > > void qemu_bh_cancel(QEMUBH *bh) > @@ -3611,6 +3608,155 @@ static void host_main_loop_wait(int *tim > } > #endif > > +static int wait_signal(int timeout) > +{ > + struct timespec ts; > + sigset_t waitset; > + > + ts.tv_sec = timeout / 1000; > + ts.tv_nsec = (timeout % 1000) * 1000000; > + sigemptyset(&waitset); > + sigaddset(&waitset, SIGUSR1); > + > + return sigtimedwait(&waitset, NULL, &ts); > +} > + > +static int has_work(CPUState *env) > +{ > + int r = 0; > + if (!env->halted) > + r = 1; > + if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) > + r = 1; > + return r; > +} > + > +static void qemu_wait_io_event(CPUState *env, int timeout) > +{ > + qemu_mutex_unlock(&qemu_global_mutex); > + > + if (timeout && !has_work(env)) > + wait_signal(timeout); > + /* > + * Users of qemu_global_mutex can be starved, having no chance > + * to acquire it since this path will get to it first. > + * So use another lock to provide fairness. > + */ > + qemu_mutex_lock(&qemu_fair_mutex); > + qemu_mutex_unlock(&qemu_fair_mutex); > This is extremely subtle. I think it can be made a bit more explicit via something like: int generation = qemu_io_generation() + 1; qemu_mutex_unlock(&qemu_global_mutex); if (timeout && !has_work(env)) wait_signal(timeout); qemu_mutex_lock(&qemu_global_mutex) while (qemu_io_generation() < generation) qemu_cond_wait(&qemu_io_generation_cond, &qemu_global_mutex); Then, in main_loop_wait(): > void main_loop_wait(int timeout) > { > IOHandlerRecord *ioh; > @@ -3660,7 +3806,7 @@ void main_loop_wait(int timeout) > */ > qemu_mutex_unlock(&qemu_global_mutex); > ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv); > - qemu_mutex_lock(&qemu_global_mutex); > qemu_mutex_lock(&qemu_global_mutex); qemu_io_generation_add(); qemu_cond_signal(&qemu_io_generation_cond); This will ensure that the TCG loop backs off of qemu_global_mutex for at least one main_loop_wait() iteration. Regards, Anthony Liguori