* [Qemu-devel] [PATCH v2 1/3] coroutine: adding sigaltstack method (.c source)
2012-02-28 11:25 [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine Alex Barcelo
@ 2012-02-28 11:25 ` Alex Barcelo
2012-02-28 11:25 ` [Qemu-devel] [PATCH v2 2/3] coroutine: adding configure choose mechanism for coroutine backend Alex Barcelo
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Alex Barcelo @ 2012-02-28 11:25 UTC (permalink / raw)
To: qemu-devel, Kevin Wolf; +Cc: Alex Barcelo
This file is based in both coroutine-ucontext.c and
pth_mctx.c (from the GNU Portable Threads library).
The mechanism used to change stacks is the sigaltstack
function (variant 2 of the pth library).
v2: Some corrections. Moving global variables into
thread storage (CoroutineThreadState).
Signed-off-by: Alex Barcelo <abarcelo@ac.upc.edu>
---
coroutine-sigaltstack.c | 334 +++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 334 insertions(+), 0 deletions(-)
create mode 100644 coroutine-sigaltstack.c
diff --git a/coroutine-sigaltstack.c b/coroutine-sigaltstack.c
new file mode 100644
index 0000000..7ff2d33
--- /dev/null
+++ b/coroutine-sigaltstack.c
@@ -0,0 +1,334 @@
+/*
+ * sigaltstack coroutine initialization code
+ *
+ * Copyright (C) 2006 Anthony Liguori <anthony@codemonkey.ws>
+ * Copyright (C) 2011 Kevin Wolf <kwolf@redhat.com>
+ * Copyright (C) 2012 Alex Barcelo <abarcelo@ac.upc.edu>
+** This file is partly based on pth_mctx.c, from the GNU Portable Threads
+** Copyright (c) 1999-2006 Ralf S. Engelschall <rse@engelschall.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
+#ifdef _FORTIFY_SOURCE
+#undef _FORTIFY_SOURCE
+#endif
+#include <stdlib.h>
+#include <setjmp.h>
+#include <stdint.h>
+#include <pthread.h>
+#include <signal.h>
+#include "qemu-common.h"
+#include "qemu-coroutine-int.h"
+
+enum {
+ /* Maximum free pool size prevents holding too many freed coroutines */
+ POOL_MAX_SIZE = 64,
+};
+
+/** Free list to speed up creation */
+static QSLIST_HEAD(, Coroutine) pool = QSLIST_HEAD_INITIALIZER(pool);
+static unsigned int pool_size;
+
+typedef struct {
+ Coroutine base;
+ void *stack;
+ jmp_buf env;
+} CoroutineUContext;
+
+/**
+ * Per-thread coroutine bookkeeping
+ */
+typedef struct {
+ /** Currently executing coroutine */
+ Coroutine *current;
+
+ /** The default coroutine */
+ CoroutineUContext leader;
+
+ /** Information for the signal handler (trampoline) */
+ jmp_buf tr_reenter;
+ volatile sig_atomic_t tr_called;
+ void *tr_handler;
+} CoroutineThreadState;
+
+static pthread_key_t thread_state_key;
+
+static CoroutineThreadState *coroutine_get_thread_state(void)
+{
+ CoroutineThreadState *s = pthread_getspecific(thread_state_key);
+
+ if (!s) {
+ s = g_malloc0(sizeof(*s));
+ s->current = &s->leader.base;
+ pthread_setspecific(thread_state_key, s);
+ }
+ return s;
+}
+
+static void qemu_coroutine_thread_cleanup(void *opaque)
+{
+ CoroutineThreadState *s = opaque;
+
+ g_free(s);
+}
+
+static void __attribute__((destructor)) coroutine_cleanup(void)
+{
+ Coroutine *co;
+ Coroutine *tmp;
+
+ QSLIST_FOREACH_SAFE(co, &pool, pool_next, tmp) {
+ g_free(DO_UPCAST(CoroutineUContext, base, co)->stack);
+ g_free(co);
+ }
+}
+
+static void __attribute__((constructor)) coroutine_init(void)
+{
+ int ret;
+
+ ret = pthread_key_create(&thread_state_key, qemu_coroutine_thread_cleanup);
+ if (ret != 0) {
+ fprintf(stderr, "unable to create leader key: %s\n", strerror(errno));
+ abort();
+ }
+}
+
+/* "boot" function
+ * This is what starts the coroutine, is called from the trampoline
+ * (from the signal handler when it is not signal handling, read ahead
+ * for more information).
+ */
+static void coroutine_bootstrap(CoroutineUContext *self, Coroutine *co)
+{
+ /* Initialize longjmp environment and switch back the caller */
+ if (!setjmp(self->env)) {
+ longjmp(*(jmp_buf *)co->entry_arg, 1);
+ }
+
+ while (true) {
+ co->entry(co->entry_arg);
+ qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
+ }
+}
+
+/*
+ * This is used as the signal handler. This is called with the brand new stack
+ * (thanks to sigaltstack). We have to return, given that this is a signal
+ * handler and the sigmask and some other things are changed.
+ */
+static void coroutine_trampoline(int signal)
+{
+ CoroutineUContext *self;
+ Coroutine *co;
+ CoroutineThreadState *coTS;
+
+ /* Get the thread specific information */
+ coTS = coroutine_get_thread_state();
+ self = coTS->tr_handler;
+ coTS->tr_called = 1;
+ co = &self->base;
+
+ /*
+ * Here we have to do a bit of a ping pong between the caller, given that
+ * this is a signal handler and we have to do a return "soon". Then the
+ * caller can reestablish everything and do a longjmp here again.
+ */
+ if (!setjmp(coTS->tr_reenter)) {
+ return;
+ }
+
+ /*
+ * Ok, the caller has longjmp'ed back to us, so now prepare
+ * us for the real machine state switching. We have to jump
+ * into another function here to get a new stack context for
+ * the auto variables (which have to be auto-variables
+ * because the start of the thread happens later). Else with
+ * PIC (i.e. Position Independent Code which is used when PTH
+ * is built as a shared library) most platforms would
+ * horrible core dump as experience showed.
+ */
+ coroutine_bootstrap(self, co);
+}
+
+static Coroutine *coroutine_new(void)
+{
+ const size_t stack_size = 1 << 20;
+ CoroutineUContext *co;
+ CoroutineThreadState *coTS;
+ struct sigaction sa;
+ struct sigaction osa;
+ struct sigaltstack ss;
+ struct sigaltstack oss;
+ sigset_t sigs;
+ sigset_t osigs;
+ jmp_buf old_env;
+
+ /* The way to manipulate stack is with the sigaltstack function. We
+ * prepare a stack, with it delivering a signal to ourselves and then
+ * put setjmp/longjmp where needed.
+ * This has been done keeping coroutine-ucontext as a model and with the
+ * pth ideas (GNU Portable Threads). See coroutine-ucontext for the basics
+ * of the coroutines and see pth_mctx.c (from the pth project) for the
+ * sigaltstack way of manipulating stacks.
+ */
+
+ co = g_malloc0(sizeof(*co));
+ co->stack = g_malloc(stack_size);
+ co->base.entry_arg = &old_env; /* stash away our jmp_buf */
+
+ coTS = coroutine_get_thread_state();
+ coTS->tr_handler = co;
+
+ /*
+ * Preserve the SIGUSR2 signal state, block SIGUSR2,
+ * and establish our signal handler. The signal will
+ * later transfer control onto the signal stack.
+ */
+ sigemptyset(&sigs);
+ sigaddset(&sigs, SIGUSR2);
+ pthread_sigmask(SIG_BLOCK, &sigs, &osigs);
+ sa.sa_handler = coroutine_trampoline;
+ sigfillset(&sa.sa_mask);
+ sa.sa_flags = SA_ONSTACK;
+ if (sigaction(SIGUSR2, &sa, &osa) != 0) {
+ abort();
+ }
+
+ /*
+ * Set the new stack.
+ */
+ ss.ss_sp = co->stack;
+ ss.ss_size = stack_size;
+ ss.ss_flags = 0;
+ if (sigaltstack(&ss, &oss) < 0) {
+ abort();
+ }
+
+ /*
+ * Now transfer control onto the signal stack and set it up.
+ * It will return immediately via "return" after the setjmp()
+ * was performed. Be careful here with race conditions. The
+ * signal can be delivered the first time sigsuspend() is
+ * called.
+ */
+ coTS->tr_called = 0;
+ kill(getpid(), SIGUSR2);
+ sigfillset(&sigs);
+ sigdelset(&sigs, SIGUSR2);
+ while (!coTS->tr_called) {
+ sigsuspend(&sigs);
+ }
+
+ /*
+ * Inform the system that we are back off the signal stack by
+ * removing the alternative signal stack. Be careful here: It
+ * first has to be disabled, before it can be removed.
+ */
+ sigaltstack(NULL, &ss);
+ ss.ss_flags = SS_DISABLE;
+ if (sigaltstack(&ss, NULL) < 0) {
+ abort();
+ }
+ sigaltstack(NULL, &ss);
+ if (!(oss.ss_flags & SS_DISABLE)) {
+ sigaltstack(&oss, NULL);
+ }
+
+ /*
+ * Restore the old SIGUSR2 signal handler and mask
+ */
+ sigaction(SIGUSR2, &osa, NULL);
+ pthread_sigmask(SIG_SETMASK, &osigs, NULL);
+
+ /*
+ * Now enter the trampoline again, but this time not as a signal
+ * handler. Instead we jump into it directly. The functionally
+ * redundant ping-pong pointer arithmentic is neccessary to avoid
+ * type-conversion warnings related to the `volatile' qualifier and
+ * the fact that `jmp_buf' usually is an array type.
+ */
+ if (!setjmp(old_env)) {
+ longjmp(coTS->tr_reenter, 1);
+ }
+
+ /*
+ * Ok, we returned again, so now we're finished
+ */
+
+ return &co->base;
+}
+
+Coroutine *qemu_coroutine_new(void)
+{
+ Coroutine *co;
+
+ co = QSLIST_FIRST(&pool);
+ if (co) {
+ QSLIST_REMOVE_HEAD(&pool, pool_next);
+ pool_size--;
+ } else {
+ co = coroutine_new();
+ }
+ return co;
+}
+
+void qemu_coroutine_delete(Coroutine *co_)
+{
+ CoroutineUContext *co = DO_UPCAST(CoroutineUContext, base, co_);
+
+ if (pool_size < POOL_MAX_SIZE) {
+ QSLIST_INSERT_HEAD(&pool, &co->base, pool_next);
+ co->base.caller = NULL;
+ pool_size++;
+ return;
+ }
+
+ g_free(co->stack);
+ g_free(co);
+}
+
+CoroutineAction qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
+ CoroutineAction action)
+{
+ CoroutineUContext *from = DO_UPCAST(CoroutineUContext, base, from_);
+ CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, to_);
+ CoroutineThreadState *s = coroutine_get_thread_state();
+ int ret;
+
+ s->current = to_;
+
+ ret = setjmp(from->env);
+ if (ret == 0) {
+ longjmp(to->env, action);
+ }
+ return ret;
+}
+
+Coroutine *qemu_coroutine_self(void)
+{
+ CoroutineThreadState *s = coroutine_get_thread_state();
+
+ return s->current;
+}
+
+bool qemu_in_coroutine(void)
+{
+ CoroutineThreadState *s = pthread_getspecific(thread_state_key);
+
+ return s && s->current->caller;
+}
+
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] coroutine: adding configure choose mechanism for coroutine backend
2012-02-28 11:25 [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine Alex Barcelo
2012-02-28 11:25 ` [Qemu-devel] [PATCH v2 1/3] coroutine: adding sigaltstack method (.c source) Alex Barcelo
@ 2012-02-28 11:25 ` Alex Barcelo
2012-02-28 11:25 ` [Qemu-devel] [PATCH v2 3/3] coroutine: adding configure option for sigaltstack " Alex Barcelo
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Alex Barcelo @ 2012-02-28 11:25 UTC (permalink / raw)
To: qemu-devel, Kevin Wolf; +Cc: Alex Barcelo
Configure tries, as a default, ucontext functions for the
coroutines. But now the user can force another backend by
--with-coroutine=BACKEND option
v2: Using --with-coroutine=BACKEND instead of enable
disable individual configure options
Signed-off-by: Alex Barcelo <abarcelo@ac.upc.edu>
---
configure | 37 +++++++++++++++++++++++++++++--------
1 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/configure b/configure
index f9d5330..5a12c56 100755
--- a/configure
+++ b/configure
@@ -191,6 +191,7 @@ opengl=""
zlib="yes"
guest_agent="yes"
libiscsi=""
+coroutine=""
# parse CC options first
for opt do
@@ -771,6 +772,8 @@ for opt do
;;
--with-pkgversion=*) pkgversion=" ($optarg)"
;;
+ --with-coroutine=*) coroutine="$optarg"
+ ;;
--disable-docs) docs="no"
;;
--enable-docs) docs="yes"
@@ -1095,6 +1098,8 @@ echo " --disable-usb-redir disable usb network redirection support"
echo " --enable-usb-redir enable usb network redirection support"
echo " --disable-guest-agent disable building of the QEMU Guest Agent"
echo " --enable-guest-agent enable building of the QEMU Guest Agent"
+echo " --with-coroutine=BACKEND coroutine backend. Supported options:"
+echo " gthread, ucontext, windows"
echo ""
echo "NOTE: The object files are built at the place where configure is launched"
exit 1
@@ -2722,21 +2727,36 @@ EOF
fi
##########################################
-# check if we have makecontext
-# (and that it's not a glibc stub which always returns -1)
+# check and set a backend for coroutine
-ucontext_coroutine=no
-if test "$darwin" != "yes"; then
- cat > $TMPC << EOF
+# default is ucontext, but always fallback to gthread
+# windows autodetected by make
+if test "$coroutine" = "" -o "$coroutine" = "ucontext"; then
+ if test "$darwin" != "yes"; then
+ cat > $TMPC << EOF
#include <ucontext.h>
#ifdef __stub_makecontext
#error Ignoring glibc stub makecontext which will always fail
#endif
int main(void) { makecontext(0, 0, 0); return 0; }
EOF
- if compile_prog "" "" ; then
- ucontext_coroutine=yes
+ if compile_prog "" "" ; then
+ coroutine_backend=ucontext
+ else
+ coroutine_backend=gthread
+ fi
+ else
+ echo "Silently falling back into gthread backend under darwin"
fi
+elif test "$coroutine" = "gthread" ; then
+ coroutine_backend=gthread
+elif test "$coroutine" = "windows" ; then
+ coroutine_backend=windows
+else
+ echo
+ echo "Error: unknown coroutine backend $coroutine"
+ echo
+ exit 1
fi
##########################################
@@ -2930,6 +2950,7 @@ echo "usb net redir $usb_redir"
echo "OpenGL support $opengl"
echo "libiscsi support $libiscsi"
echo "build guest agent $guest_agent"
+echo "coroutine backend $coroutine_backend"
if test "$sdl_too_old" = "yes"; then
echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3251,7 +3272,7 @@ if test "$rbd" = "yes" ; then
echo "CONFIG_RBD=y" >> $config_host_mak
fi
-if test "$ucontext_coroutine" = "yes" ; then
+if test "$coroutine_backend" = "ucontext" ; then
echo "CONFIG_UCONTEXT_COROUTINE=y" >> $config_host_mak
fi
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] coroutine: adding configure option for sigaltstack coroutine backend
2012-02-28 11:25 [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine Alex Barcelo
2012-02-28 11:25 ` [Qemu-devel] [PATCH v2 1/3] coroutine: adding sigaltstack method (.c source) Alex Barcelo
2012-02-28 11:25 ` [Qemu-devel] [PATCH v2 2/3] coroutine: adding configure choose mechanism for coroutine backend Alex Barcelo
@ 2012-02-28 11:25 ` Alex Barcelo
2012-03-07 22:01 ` [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine Alex Barcelo
2012-03-09 17:22 ` Kevin Wolf
4 siblings, 0 replies; 10+ messages in thread
From: Alex Barcelo @ 2012-02-28 11:25 UTC (permalink / raw)
To: qemu-devel, Kevin Wolf; +Cc: Alex Barcelo
It's possible to use sigaltstack backend with --with-coroutine=sigaltstack
v2: changed from enable/disable configure flags
Signed-off-by: Alex Barcelo <abarcelo@ac.upc.edu>
---
Makefile.objs | 4 ++++
configure | 6 +++++-
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/Makefile.objs b/Makefile.objs
index 808de6a..9dcd226 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -17,8 +17,12 @@ coroutine-obj-y += qemu-coroutine-sleep.o
ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
coroutine-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
else
+ifeq ($(CONFIG_SIGALTSTACK_COROUTINE),y)
+coroutine-obj-$(CONFIG_POSIX) += coroutine-sigaltstack.o
+else
coroutine-obj-$(CONFIG_POSIX) += coroutine-gthread.o
endif
+endif
coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
#######################################################################
diff --git a/configure b/configure
index 5a12c56..b473936 100755
--- a/configure
+++ b/configure
@@ -1099,7 +1099,7 @@ echo " --enable-usb-redir enable usb network redirection support"
echo " --disable-guest-agent disable building of the QEMU Guest Agent"
echo " --enable-guest-agent enable building of the QEMU Guest Agent"
echo " --with-coroutine=BACKEND coroutine backend. Supported options:"
-echo " gthread, ucontext, windows"
+echo " gthread, ucontext, sigaltstack, windows"
echo ""
echo "NOTE: The object files are built at the place where configure is launched"
exit 1
@@ -2752,6 +2752,8 @@ elif test "$coroutine" = "gthread" ; then
coroutine_backend=gthread
elif test "$coroutine" = "windows" ; then
coroutine_backend=windows
+elif test "$coroutine" = "sigaltstack" ; then
+ coroutine_backend=sigaltstack
else
echo
echo "Error: unknown coroutine backend $coroutine"
@@ -3274,6 +3276,8 @@ fi
if test "$coroutine_backend" = "ucontext" ; then
echo "CONFIG_UCONTEXT_COROUTINE=y" >> $config_host_mak
+elif test "$coroutine_backend" = "sigaltstack" ; then
+ echo "CONFIG_SIGALTSTACK_COROUTINE=y" >> $config_host_mak
fi
if test "$open_by_handle_at" = "yes" ; then
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine
2012-02-28 11:25 [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine Alex Barcelo
` (2 preceding siblings ...)
2012-02-28 11:25 ` [Qemu-devel] [PATCH v2 3/3] coroutine: adding configure option for sigaltstack " Alex Barcelo
@ 2012-03-07 22:01 ` Alex Barcelo
2012-03-07 22:17 ` Peter Maydell
2012-03-09 17:22 ` Kevin Wolf
4 siblings, 1 reply; 10+ messages in thread
From: Alex Barcelo @ 2012-03-07 22:01 UTC (permalink / raw)
To: qemu-devel, Kevin Wolf
Is this patch okay? The first version had some comments, and now the
v2 has been a bit too silent, not sure if that's a good sign or a bad
sign.
Thanks & sorry!
On Tue, Feb 28, 2012 at 12:25, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
> This series of patches implements coroutines method with
> sigaltstack.
>
> The flow of creation and management of the coroutines is
> quite similar to the coroutine-ucontext.c. The way to use
> sigaltstack to achieve the needed stack manipulation is
> done in a way quite similar to the GNU Portable Threads
> (file pth_mctx.c, variant 2).
>
> This v2 has some corrections and improved patches, but it's
> essentially the same. At the moment, the default backend is
> ucontext (the former default method for coroutines).
>
> Alex Barcelo (3):
> coroutine: adding sigaltstack method (.c source)
> coroutine: adding configure choose mechanism for coroutine backend
> coroutine: adding configure option for sigaltstack coroutine backend
>
> Makefile.objs | 4 +
> configure | 41 +++++-
> coroutine-sigaltstack.c | 334 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 371 insertions(+), 8 deletions(-)
> create mode 100644 coroutine-sigaltstack.c
>
> --
> 1.7.5.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine
2012-03-07 22:01 ` [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine Alex Barcelo
@ 2012-03-07 22:17 ` Peter Maydell
2012-03-07 22:44 ` Alex Barcelo
0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2012-03-07 22:17 UTC (permalink / raw)
To: Alex Barcelo; +Cc: Kevin Wolf, qemu-devel
On 7 March 2012 22:01, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
> Is this patch okay? The first version had some comments, and now the
> v2 has been a bit too silent, not sure if that's a good sign or a bad
> sign.
Did you see the comment I added to an earlier thread regarding
FORTIFY_SOURCE?
I think in general my opinion is swinging round to:
* coroutines are a portability nightmare
* so we should either (a) ideally avoid them altogether
or (b) defer to GNU Pth to do the hard work
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine
2012-03-07 22:17 ` Peter Maydell
@ 2012-03-07 22:44 ` Alex Barcelo
2012-03-07 23:03 ` Peter Maydell
2012-03-08 10:15 ` Stefan Hajnoczi
0 siblings, 2 replies; 10+ messages in thread
From: Alex Barcelo @ 2012-03-07 22:44 UTC (permalink / raw)
To: Peter Maydell; +Cc: Kevin Wolf, qemu-devel
On Wed, Mar 7, 2012 at 23:17, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 March 2012 22:01, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
>> Is this patch okay? The first version had some comments, and now the
>> v2 has been a bit too silent, not sure if that's a good sign or a bad
>> sign.
>
> Did you see the comment I added to an earlier thread regarding
> FORTIFY_SOURCE?
Sorry about that! Yes, I got the comment mixed between some other
threads, and now I was checking and didn't remember it.
About the FORTIFY_SOURCE... I don't know very well what does that mean
and what it does, I kept it from the ucontext code without thinking a
lot (irresponsibly? yes, maybe a bit).
> I think in general my opinion is swinging round to:
> * coroutines are a portability nightmare
agreed ;)
> * so we should either (a) ideally avoid them altogether
seems better the coroutines than state machines on every I/O process
> or (b) defer to GNU Pth to do the hard work
My first impression was that Portable Threads is not the same as
coroutines (as the actual qemu uses coroutines). But then, thinking a
bit more about them... maybe GNU Pth can be used as a backend in some
simple way. Well, I suppose I can check them a bit deeper :)
But, moving into libpth means a to be (absolutely?) dependent on a new
library (although it's a very portable one, more portable than the
actual code). Not sure what I would do (if it was my choice) with the
glib implementation... Is there some roadmap involving coroutines and
sync/async I/O? Is this library a problem?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine
2012-03-07 22:44 ` Alex Barcelo
@ 2012-03-07 23:03 ` Peter Maydell
2012-03-08 10:15 ` Stefan Hajnoczi
1 sibling, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2012-03-07 23:03 UTC (permalink / raw)
To: Alex Barcelo; +Cc: Kevin Wolf, qemu-devel
On 7 March 2012 22:44, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
> On Wed, Mar 7, 2012 at 23:17, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Did you see the comment I added to an earlier thread regarding
>> FORTIFY_SOURCE?
>
> Sorry about that! Yes, I got the comment mixed between some other
> threads, and now I was checking and didn't remember it.
>
> About the FORTIFY_SOURCE... I don't know very well what does that mean
> and what it does, I kept it from the ucontext code without thinking a
> lot (irresponsibly? yes, maybe a bit).
Ha. I should have read your actual patch, sorry :-)
That bit of code does correctly disable a glibc sanity check which
would otherwise cause this code to fail, so your patch is OK on this
point.
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine
2012-03-07 22:44 ` Alex Barcelo
2012-03-07 23:03 ` Peter Maydell
@ 2012-03-08 10:15 ` Stefan Hajnoczi
1 sibling, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2012-03-08 10:15 UTC (permalink / raw)
To: Alex Barcelo; +Cc: Kevin Wolf, Peter Maydell, qemu-devel
On Wed, Mar 7, 2012 at 10:44 PM, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
> On Wed, Mar 7, 2012 at 23:17, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 7 March 2012 22:01, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
>>> Is this patch okay? The first version had some comments, and now the
>>> v2 has been a bit too silent, not sure if that's a good sign or a bad
>>> sign.
>>
>> Did you see the comment I added to an earlier thread regarding
>> FORTIFY_SOURCE?
>
> Sorry about that! Yes, I got the comment mixed between some other
> threads, and now I was checking and didn't remember it.
>
> About the FORTIFY_SOURCE... I don't know very well what does that mean
> and what it does, I kept it from the ucontext code without thinking a
> lot (irresponsibly? yes, maybe a bit).
>
>> I think in general my opinion is swinging round to:
>> * coroutines are a portability nightmare
> agreed ;)
>> * so we should either (a) ideally avoid them altogether
> seems better the coroutines than state machines on every I/O process
Going back to callbacks is moving in the wrong direction. Instead we
could take a step forward and come up with a proper threading model
for QEMU and convert coroutine code to execute in worker threads.
In fact, Paolo has already posted some ideas about a threaded block
layer and a prototype git branch.
I think this is the right approach to follow.
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine
2012-02-28 11:25 [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine Alex Barcelo
` (3 preceding siblings ...)
2012-03-07 22:01 ` [Qemu-devel] [PATCH v2 0/3] New sigaltstack backend for coroutine Alex Barcelo
@ 2012-03-09 17:22 ` Kevin Wolf
4 siblings, 0 replies; 10+ messages in thread
From: Kevin Wolf @ 2012-03-09 17:22 UTC (permalink / raw)
To: Alex Barcelo; +Cc: qemu-devel
Am 28.02.2012 12:25, schrieb Alex Barcelo:
> This series of patches implements coroutines method with
> sigaltstack.
>
> The flow of creation and management of the coroutines is
> quite similar to the coroutine-ucontext.c. The way to use
> sigaltstack to achieve the needed stack manipulation is
> done in a way quite similar to the GNU Portable Threads
> (file pth_mctx.c, variant 2).
>
> This v2 has some corrections and improved patches, but it's
> essentially the same. At the moment, the default backend is
> ucontext (the former default method for coroutines).
>
> Alex Barcelo (3):
> coroutine: adding sigaltstack method (.c source)
> coroutine: adding configure choose mechanism for coroutine backend
> coroutine: adding configure option for sigaltstack coroutine backend
>
> Makefile.objs | 4 +
> configure | 41 +++++-
> coroutine-sigaltstack.c | 334 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 371 insertions(+), 8 deletions(-)
> create mode 100644 coroutine-sigaltstack.c
Thanks, applied all to the block branch.
Kevin
^ permalink raw reply [flat|nested] 10+ messages in thread