From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LuohI-0002Vt-4f for qemu-devel@nongnu.org; Fri, 17 Apr 2009 10:05:40 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LuohD-0002UN-8E for qemu-devel@nongnu.org; Fri, 17 Apr 2009 10:05:39 -0400 Received: from [199.232.76.173] (port=55467 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LuohD-0002UG-1D for qemu-devel@nongnu.org; Fri, 17 Apr 2009 10:05:35 -0400 Received: from e31.co.us.ibm.com ([32.97.110.149]:41386) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LuohC-0006U8-HE for qemu-devel@nongnu.org; Fri, 17 Apr 2009 10:05:34 -0400 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by e31.co.us.ibm.com (8.13.1/8.13.1) with ESMTP id n3HE2Er4023516 for ; Fri, 17 Apr 2009 08:02:14 -0600 Received: from d03av01.boulder.ibm.com (d03av01.boulder.ibm.com [9.17.195.167]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n3HE5Kjj064682 for ; Fri, 17 Apr 2009 08:05:28 -0600 Received: from d03av01.boulder.ibm.com (loopback [127.0.0.1]) by d03av01.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n3HE5HRj019518 for ; Fri, 17 Apr 2009 08:05:17 -0600 Message-ID: <49E88C9B.2050205@us.ibm.com> Date: Fri, 17 Apr 2009 09:05:15 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <20090407195126.467365249@localhost.localdomain> <20090407195443.121795529@localhost.localdomain> In-Reply-To: <20090407195443.121795529@localhost.localdomain> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [patch 05/11] 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, "qemu-devel@nongnu.org" mtosatti@redhat.com wrote: > Introduce a thread to handle host IO events. > > Signed-off-by: Marcelo Tosatti > > Index: trunk/qemu-common.h > =================================================================== > --- trunk.orig/qemu-common.h > +++ trunk/qemu-common.h > @@ -191,6 +191,10 @@ void main_loop_break(void); > /* Force QEMU to process pending events */ > void qemu_notify_event(void); > > +/* Unblock cpu */ > +void qemu_cpu_kick(void *env); > +int qemu_cpu_self(void *env); > + > typedef struct QEMUIOVector { > struct iovec *iov; > int niov; > Index: trunk/vl.c > =================================================================== > --- trunk.orig/vl.c > +++ trunk/vl.c > @@ -146,6 +146,7 @@ int main(int argc, char **argv) > #include "gdbstub.h" > #include "qemu-timer.h" > #include "qemu-char.h" > +#include "qemu-thread.h" > #include "cache-utils.h" > #include "block.h" > #include "dma.h" > @@ -278,6 +279,13 @@ uint8_t qemu_uuid[16]; > > static int io_thread_fd = -1; > > +QemuMutex qemu_global_mutex; > +QemuMutex qemu_fair_mutex; > + > +QemuThread io_thread; > +QemuThread cpus_thread; > +QemuCond halt_cond; > + > /***********************************************************/ > /* x86 ISA bus support */ > > @@ -1347,8 +1355,6 @@ static void host_alarm_handler(int host_ > write(alarm_timer_wfd, &byte, sizeof(byte)); > #endif > alarm_timer->flags |= ALARM_FLAG_EXPIRED; > - > - qemu_notify_event(); > } > } > Isn't this unsafe in TCG? If you have chained TBs in a tight loop, removing qemu_notify_event() eliminates the cpu_interrupt call which then means that you'll never break out of that tight TB loop. > @@ -2957,6 +2963,7 @@ int qemu_set_fd_handler2(int fd, > ioh->opaque = opaque; > ioh->deleted = 0; > } > + main_loop_break(); > return 0; > } > > @@ -3324,7 +3331,6 @@ static int ram_load(QEMUFile *f, void *o > > void qemu_service_io(void) > { > - qemu_notify_event(); > } > The same comment from above is applicable here. qemu_service_io() is called from a signal handler. We should s/qemu_service_io/qemu_notify_event/. > void main_loop_break(void) > @@ -3733,6 +3731,105 @@ static void host_main_loop_wait(int *tim > } > #endif > #ifndef CONFIG_IO_THREAD, doesn't main_loop_break need to do cpu_interrupt? > +static void qemu_wait_io_event(CPUState *env, int timeout) > +{ > + if (timeout) > + while (!tcg_has_work(env)) > + qemu_cond_timedwait(&halt_cond, &qemu_global_mutex, timeout); > What's the point of having a timeout? > + > +static void qemu_signal_lock(unsigned int msecs) > +{ > + qemu_mutex_lock(&qemu_fair_mutex); > + > + while (qemu_mutex_trylock(&qemu_global_mutex)) { > + qemu_thread_signal(&cpus_thread, SIGUSR1); > + if (!qemu_mutex_timedlock(&qemu_global_mutex, msecs)) > + break; > + } > + qemu_mutex_unlock(&qemu_fair_mutex); > +} > + > Now's a good time to attempt to move a lot of the main loop stuff into a separate file. We can do it after the IO thread series or you can make it part of the IO thread series. -- Regards, Anthony Liguori