Re: [Qemu-devel] [PATCH v3 1/4] coroutine: introduce coroutines

[Paolo Bonzini <pbonzini@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

On 05/13/2011 11:26 AM, Stefan Hajnoczi wrote:
> This coroutines implementation is based on the gtk-vnc implementation
> written by Anthony Liguori<anthony@codemonkey.ws>  but it has been
> significantly rewritten by Kevin Wolf<kwolf@redhat.com>  to use
> setjmp()/longjmp() instead of the more expensive swapcontext().

Since in the future we'll have at least three implementations of 
coroutines (ucontext, win32 fibers, threads) can you squash in something 
like the attached to allow "subclassing" of coroutines using DO_UPCAST)? 
  Resolving conflicts with 4/4 should be trivial

(Sorry for not doing my homework completely; I started describing in 
English what I had in mind, but it was too much of a mouthful).

Paolo

[-- Attachment #2: 0001-allow-subclassing-of-coroutines.patch --]
[-- Type: text/x-patch, Size: 6062 bytes --]

>From d6798e8d30a5de9381a30638223e8283f0817660 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Fri, 13 May 2011 12:20:48 +0200
Subject: [PATCH] allow subclassing of coroutines

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 coroutine-ucontext.c |   45 +++++++++++++++++++++++++++++++++++++--------
 coroutine-win32.c    |   38 +++++++++++++++++++++++++++++++++-----
 qemu-coroutine-int.h |    4 ++--
 qemu-coroutine.c     |   23 ++++++-----------------
 4 files changed, 78 insertions(+), 32 deletions(-)

diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index 3a8973e..14a0e81 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -27,11 +27,33 @@
 #include <ucontext.h>
 #include "qemu-coroutine-int.h"
 
-static Coroutine *new_coroutine;
+struct CoroutineUContext { 
+    Coroutine base;
+
+    jmp_buf env;
+};
+
+typedef struct CoroutineUContext CoroutineUContext;
+
+static CoroutineUContext *new_coroutine;
+
+int qemu_coroutine_switch(Coroutine *from_, Coroutine *to_, int action)
+{
+    CoroutineUContext *from = DO_UPCAST(CoroutineUContext, base, from_);
+    CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, to_);
+    int ret;
+
+    ret = setjmp(from->env);
+    if (ret == 0) {
+        longjmp(to->env, action);
+    } else {
+        return ret;
+    }
+}
 
 static void continuation_trampoline(void)
 {
-    Coroutine *co = new_coroutine;
+    CoroutineUContext *co = new_coroutine;
 
     /* Initialize longjmp environment and switch back to
      * qemu_coroutine_init_env() in the old ucontext. */
@@ -40,16 +62,23 @@ static void continuation_trampoline(void)
     }
 
     while (true) {
-        co->entry(co->data);
-        if (!setjmp(co->env)) {
-            longjmp(co->caller->env, COROUTINE_TERMINATE);
-        }
+        co->base.entry(co->base.data);
+        qemu_coroutine_switch(&co->base, co->base.caller, COROUTINE_TERMINATE);
     }
 }
 
-int qemu_coroutine_init_env(Coroutine *co, size_t stack_size)
+Coroutine *qemu_coroutine_new(size_t stack_size)
+{
+    CoroutineUContext *co;
+    co = qemu_mallocz(sizeof(*co));
+    co->base.stack = qemu_malloc(stack_size);
+    return &co->base;
+}
+
+int qemu_coroutine_init_env(Coroutine *co_, size_t stack_size)
 {
     ucontext_t old_uc, uc;
+    CoroutineUContext *co = DO_UPCAST(CoroutineUContext, base, co_);
 
     /* Create a new ucontext for switching to the coroutine stack and setting
      * up a longjmp environment. */
@@ -58,7 +87,7 @@ int qemu_coroutine_init_env(Coroutine *co, size_t stack_size)
     }
 
     uc.uc_link = &old_uc;
-    uc.uc_stack.ss_sp = co->stack;
+    uc.uc_stack.ss_sp = co->base.stack;
     uc.uc_stack.ss_size = stack_size;
     uc.uc_stack.ss_flags = 0;
 
diff --git a/coroutine-win32.c b/coroutine-win32.c
index 99141dd..5301975 100644
--- a/coroutine-win32.c
+++ b/coroutine-win32.c
@@ -24,20 +24,48 @@
 
 #include "qemu-coroutine-int.h"
 
-static void __attribute__((used)) trampoline(Coroutine *co)
+struct CoroutineWin32 { 
+    Coroutine base;
+
+    jmp_buf env;
+};
+
+typedef struct CoroutineWin32 CoroutineWin32;
+
+int qemu_coroutine_switch(Coroutine *from_, Coroutine *to_, int action)
+{
+    CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, from_);
+    CoroutineWin32 *to = DO_UPCAST(CoroutineWin32, base, to_);
+    int ret;
+
+    ret = setjmp(from->env);
+    if (ret == 0) {
+        longjmp(to->env, action);
+    } else {
+        return ret;
+    }
+}
+
+static void __attribute__((used)) trampoline(CoroutineWin32 *co)
 {
     if (!setjmp(co->env)) {
         return;
     }
 
     while (true) {
-        co->entry(co->data);
-        if (!setjmp(co->env)) {
-            longjmp(co->caller->env, COROUTINE_TERMINATE);
-        }
+        co->base.entry(co->base.data);
+        qemu_coroutine_switch(&co->base, co->base.caller, COROUTINE_TERMINATE);
     }
 }
 
+Coroutine *qemu_coroutine_new(size_t stack_size)
+{
+    CoroutineWin32 *co;
+    co = qemu_mallocz(sizeof(*co));
+    co->base.stack = qemu_malloc(stack_size);
+    return &co->base;
+}
+
 int qemu_coroutine_init_env(Coroutine *co, size_t stack_size)
 {
 #ifdef __i386__
diff --git a/qemu-coroutine-int.h b/qemu-coroutine-int.h
index 71c6ee9..04eb41a 100644
--- a/qemu-coroutine-int.h
+++ b/qemu-coroutine-int.h
@@ -44,10 +44,10 @@ struct Coroutine {
     /* Used to pass arguments/return values for coroutines */
     void *data;
     CoroutineEntry *entry;
-
-    jmp_buf env;
 };
 
 int qemu_coroutine_init_env(Coroutine *co, size_t stack_size);
+int qemu_coroutine_switch(Coroutine *from_, Coroutine *to_, int action);
+Coroutine *qemu_coroutine_new(size_t stack_size);
 
 #endif
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index a80468b..02c345d 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -34,24 +34,15 @@ static void coroutine_terminate(Coroutine *co)
     qemu_free(co);
 }
 
-static Coroutine *coroutine_new(void)
-{
-    const size_t stack_size = 4 << 20;
-    Coroutine *co;
-
-    co = qemu_mallocz(sizeof(*co));
-    co->stack = qemu_malloc(stack_size);
-    qemu_coroutine_init_env(co, stack_size);
-    return co;
-}
-
 Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
 {
+    const size_t stack_size = 4 << 20;
     Coroutine *co;
 
-    co = coroutine_new();
+    co = qemu_coroutine_new(stack_size);
     co->entry = entry;
 
+    qemu_coroutine_init_env(co, stack_size);
     return co;
 }
 
@@ -76,7 +67,8 @@ static void *coroutine_swap(Coroutine *from, Coroutine *to, void *opaque)
 
     to->data = opaque;
 
-    ret = setjmp(from->env);
+    current = to;
+    ret = qemu_coroutine_switch(from, to, COROUTINE_YIELD);
     switch (ret) {
     case COROUTINE_YIELD:
         return from->data;
@@ -86,10 +78,7 @@ static void *coroutine_swap(Coroutine *from, Coroutine *to, void *opaque)
         coroutine_terminate(to);
         return to_data;
     default:
-        /* Switch to called coroutine */
-        current = to;
-        longjmp(to->env, COROUTINE_YIELD);
-        return NULL;
+        abort();
     }
 }
 
-- 
1.7.4.4