From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:40096) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QKEqC-0004mD-U3 for qemu-devel@nongnu.org; Wed, 11 May 2011 15:13:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QKEqB-00004v-KX for qemu-devel@nongnu.org; Wed, 11 May 2011 15:13:00 -0400 Received: from moutng.kundenserver.de ([212.227.17.9]:64273) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QKEqB-0008WV-8A for qemu-devel@nongnu.org; Wed, 11 May 2011 15:12:59 -0400 Message-ID: <4DCADFB5.8030206@mail.berlios.de> Date: Wed, 11 May 2011 21:12:53 +0200 From: Stefan Weil MIME-Version: 1.0 References: <1305108925-26048-1-git-send-email-stefanha@linux.vnet.ibm.com> <1305108925-26048-2-git-send-email-stefanha@linux.vnet.ibm.com> In-Reply-To: <1305108925-26048-2-git-send-email-stefanha@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] coroutine: introduce coroutines List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Anthony Liguori , Venkateswararao Jujjuri , qemu-devel@nongnu.org Am 11.05.2011 12:15, schrieb Stefan Hajnoczi: > From: Kevin Wolf > > Asynchronous code is becoming very complex. At the same time > synchronous code is growing because it is convenient to write. > Sometimes duplicate code paths are even added, one synchronous and the > other asynchronous. This patch introduces coroutines which allow code > that looks synchronous but is asynchronous under the covers. > > A coroutine has its own stack and is therefore able to preserve state > across blocking operations, which traditionally require callback > functions and manual marshalling of parameters. > > Creating and starting a coroutine is easy: > > coroutine = qemu_coroutine_create(my_coroutine); > qemu_coroutine_enter(coroutine, my_data); > > The coroutine then executes until it returns or yields: > > void coroutine_fn my_coroutine(void *opaque) { > MyData *my_data = opaque; > > /* do some work */ > > qemu_coroutine_yield(); > > /* do some more work */ > } > > Yielding switches control back to the caller of qemu_coroutine_enter(). > This is typically used to switch back to the main thread's event loop > after issuing an asynchronous I/O request. The request callback will > then invoke qemu_coroutine_enter() once more to switch back to the > coroutine. > > Note that coroutines never execute concurrently and should only be used > from threads which hold the global mutex. This restriction makes > programming with coroutines easier than with threads. Race conditions > cannot occur since only one coroutine may be active at any time. Other > coroutines can only run across yield. > > This coroutines implementation is based on the gtk-vnc implementation > written by Anthony Liguori but it has been > significantly rewritten by Kevin Wolf to use > setjmp()/longjmp() instead of the more expensive swapcontext(). > > Signed-off-by: Kevin Wolf > Signed-off-by: Stefan Hajnoczi > --- Hi Stefan, you might want to add the following or a similar patch: diff --git a/qemu-coroutine.c b/qemu-coroutine.c index 0927f58..9cd0dd7 100644 --- a/qemu-coroutine.c +++ b/qemu-coroutine.c @@ -91,7 +91,10 @@ static void *coroutine_swap(Coroutine *from, Coroutine *to, void *opaque) case COROUTINE_TERMINATE: current = to->caller; qemu_coroutine_terminate(to); - return to->data; + opaque = to->data; + qemu_free(to->stack); + qemu_free(to); + return opaque; default: /* Switch to called coroutine */ current = to; I tested your test code with Valgrind. Beside of the memory leaks which are fixed with the small modification shown above, Valgrind has a lot of complains. Maybe you can try it yourself, otherwise please wait until I have finished analyzing the Valgrind results. At a first glance, I'm afraid that debugging with gdb or Valgrind might become more difficult when coroutines are used. This is different with threads: they are fully supported by gdb. The w32 build needs additional libraries (ws2_32, maybe more), then check-coroutine works. Cheers, Stefan W.