[Spice-commits] 3 commits - meson.build meson_options.txt src/continuation.c src/continuation.h src/coroutine_gthread.c src/coroutine.h src/coroutine_ucontext.c src/coroutine_winfibers.c
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Sat Aug 22 08:25:50 UTC 2020
meson.build | 9 ++++++
meson_options.txt | 5 +++
src/continuation.c | 66 ++++++++++++++++++++++++++--------------------
src/continuation.h | 4 --
src/coroutine.h | 7 +++-
src/coroutine_gthread.c | 4 +-
src/coroutine_ucontext.c | 9 ++++++
src/coroutine_winfibers.c | 2 -
8 files changed, 70 insertions(+), 36 deletions(-)
New commits:
commit 5989350429a79787f02e9c869c7769989781d320
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Fri Aug 14 05:26:35 2020 +0100
continuation: Simplify implementation
Instead of using ucontext and setjmp together just use setjmp and
limit ucontext usage to initialise the initial jmp_buf context.
This simplifies the code and reduce number of context operations done.
In particular using _setjmp/_longjmp moving from a context to
another does not require any system call.
On x64 the continuation structure is reduced from 2176 to 248 bytes.
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
Acked-by: Marc-André Lureau <marcandre.lureau at redhat.com>
diff --git a/src/continuation.c b/src/continuation.c
index d0b7745..65527ac 100644
--- a/src/continuation.c
+++ b/src/continuation.c
@@ -25,6 +25,7 @@
#endif
#include <errno.h>
+#include <ucontext.h>
#include <glib.h>
#include "continuation.h"
@@ -35,39 +36,51 @@
* union is a quick hack to let us do that
*/
union cc_arg {
- void *p;
int i[2];
+ void *p;
};
static void continuation_trampoline(int i0, int i1)
{
- union cc_arg arg;
- struct continuation *cc;
- arg.i[0] = i0;
- arg.i[1] = i1;
- cc = arg.p;
+ const union cc_arg arg = {{ i0, i1 }};
+ struct continuation *const cc = arg.p;
- if (_setjmp(cc->jmp) == 0) {
- ucontext_t tmp;
- swapcontext(&tmp, &cc->last);
+ if (_setjmp(cc->jmp) != 0) {
+ cc->entry(cc);
+ cc->exited = 1;
+ _longjmp(*((jmp_buf *) cc->last), 1);
}
- cc->entry(cc);
+ /* Here you would be tempted to use use uc_link and avoid the usage of
+ * setcontext; don't do it, it would potentially corruct stack returning.
+ * The return would "release" part of the stack which could be
+ * overridden for instance by a signal handler.
+ * This could corrupt some variables allocated on the stack.
+ * Although this function has very few variables which potentially
+ * will be allocated on registers the union and the call to
+ * _setjmp could reduce optimizations causing variables to be
+ * allocated on the stack.
+ */
+ setcontext((ucontext_t *) cc->last);
+ g_error("setcontext() failed: %s", g_strerror(errno));
}
void cc_init(struct continuation *cc)
{
volatile union cc_arg arg;
+ ucontext_t uc, uc_ret;
arg.p = cc;
- if (getcontext(&cc->uc) == -1)
+ if (getcontext(&uc) == -1)
g_error("getcontext() failed: %s", g_strerror(errno));
- cc->uc.uc_link = &cc->last;
- cc->uc.uc_stack.ss_sp = cc->stack;
- cc->uc.uc_stack.ss_size = cc->stack_size;
- cc->uc.uc_stack.ss_flags = 0;
+ cc->exited = 0;
+ uc.uc_link = NULL;
+ uc.uc_stack.ss_sp = cc->stack;
+ uc.uc_stack.ss_size = cc->stack_size;
+ uc.uc_stack.ss_flags = 0;
+ cc->last = &uc_ret;
- makecontext(&cc->uc, (void *)continuation_trampoline, 2, arg.i[0], arg.i[1]);
- swapcontext(&cc->last, &cc->uc);
+ makecontext(&uc, (void *)continuation_trampoline, 2, arg.i[0], arg.i[1]);
+ swapcontext(&uc_ret, &uc);
}
int cc_release(struct continuation *cc)
@@ -80,18 +93,15 @@ int cc_release(struct continuation *cc)
int cc_swap(struct continuation *from, struct continuation *to)
{
- to->exited = 0;
- if (getcontext(&to->last) == -1)
- return -1;
- else if (to->exited == 0)
- to->exited = 1; // so when coroutine finishes
- else if (to->exited == 1)
- return 1; // it ends up here
-
- if (_setjmp(from->jmp) == 0)
- _longjmp(to->jmp, 1);
+ if (!to->exited) {
+ to->last = &from->jmp;
+ if (_setjmp(from->jmp) == 0) {
+ _longjmp(to->jmp, 1);
+ }
- return 0;
+ return to->exited;
+ }
+ g_error("continuation routine already exited");
}
/*
* Local variables:
diff --git a/src/continuation.h b/src/continuation.h
index 79b68c4..d637fae 100644
--- a/src/continuation.h
+++ b/src/continuation.h
@@ -21,7 +21,6 @@
#pragma once
#include <stddef.h>
-#include <ucontext.h>
#include <setjmp.h>
struct continuation
@@ -32,9 +31,8 @@ struct continuation
int (*release)(struct continuation *cc);
/* private */
- ucontext_t uc;
- ucontext_t last;
int exited;
+ void *last;
jmp_buf jmp;
};
commit 942bd130ddd3c331045c1d3e4f580909901d285a
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Thu Aug 13 14:31:52 2020 +0100
Allows to use Valgrind with ucontext coroutines
Changing stack is not usually instrumentation friendly.
Allows to enable the usage of Valgrind memcheck calling some
valgrind macros.
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
Acked-by: Marc-André Lureau <marcandre.lureau at redhat.com>
diff --git a/meson.build b/meson.build
index 405c589..6bbb4a8 100644
--- a/meson.build
+++ b/meson.build
@@ -61,6 +61,7 @@ headers = [
'sys/types.h',
'netinet/in.h',
'arpa/inet.h',
+ 'valgrind/valgrind.h'
]
foreach header : headers
@@ -346,6 +347,14 @@ if d.found()
spice_gtk_config_data.set('USE_SMARTCARD', '1')
endif
+# valgrind
+if get_option('valgrind')
+ if spice_gtk_config_data.get('HAVE_VALGRIND_VALGRIND_H', '0') != '1'
+ error('Valgrind requested but headers not found')
+ endif
+ spice_gtk_config_data.set('HAVE_VALGRIND', '1')
+endif
+
#
# global C defines
#
diff --git a/meson_options.txt b/meson_options.txt
index 4bffbfa..3cbc7c6 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -85,3 +85,8 @@ option('recorder',
type : 'boolean',
value : false,
description: 'Enable recorder instrumentation')
+
+option('valgrind',
+ type : 'boolean',
+ value : false,
+ description : 'Enable Valgrind support')
diff --git a/src/coroutine.h b/src/coroutine.h
index 09e68bb..676fa97 100644
--- a/src/coroutine.h
+++ b/src/coroutine.h
@@ -44,6 +44,9 @@ struct coroutine
#if WITH_UCONTEXT
struct continuation cc;
+#ifdef HAVE_VALGRIND
+ unsigned int vg_stack;
+#endif
#elif WITH_WINFIBER
LPVOID fiber;
int ret;
diff --git a/src/coroutine_ucontext.c b/src/coroutine_ucontext.c
index 9d16ada..1802319 100644
--- a/src/coroutine_ucontext.c
+++ b/src/coroutine_ucontext.c
@@ -30,6 +30,9 @@
#include <stdlib.h>
#include <string.h>
#include <spice/macros.h>
+#ifdef HAVE_VALGRIND
+#include <valgrind/valgrind.h>
+#endif
#include "coroutine.h"
#ifndef MAP_ANONYMOUS
@@ -45,6 +48,9 @@ static int _coroutine_release(struct continuation *cc)
{
struct coroutine *co = SPICE_CONTAINEROF(cc, struct coroutine, cc);
+#ifdef HAVE_VALGRIND
+ VALGRIND_STACK_DEREGISTER(co->vg_stack);
+#endif
munmap(co->cc.stack, co->cc.stack_size);
co->caller = NULL;
@@ -71,6 +77,9 @@ void coroutine_init(struct coroutine *co)
if (co->cc.stack == MAP_FAILED)
g_error("mmap(%" G_GSIZE_FORMAT ") failed: %s",
co->stack_size, g_strerror(errno));
+#ifdef HAVE_VALGRIND
+ co->vg_stack = VALGRIND_STACK_REGISTER(co->cc.stack, co->cc.stack + co->stack_size);
+#endif
co->cc.entry = coroutine_trampoline;
co->cc.release = _coroutine_release;
commit f5121d14a76cada0ab1270a0e9f429a5347c87b8
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Thu Aug 13 11:20:16 2020 +0100
coroutine: Fix indentation
These files use tabs instead of spaces, fix indentation.
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
Acked-by: Marc-André Lureau <marcandre.lureau at redhat.com>
diff --git a/src/coroutine.h b/src/coroutine.h
index 96d8593..09e68bb 100644
--- a/src/coroutine.h
+++ b/src/coroutine.h
@@ -45,8 +45,8 @@ struct coroutine
#if WITH_UCONTEXT
struct continuation cc;
#elif WITH_WINFIBER
- LPVOID fiber;
- int ret;
+ LPVOID fiber;
+ int ret;
#else
GThread *thread;
gboolean runnable;
diff --git a/src/coroutine_gthread.c b/src/coroutine_gthread.c
index d5c11e6..3be8334 100644
--- a/src/coroutine_gthread.c
+++ b/src/coroutine_gthread.c
@@ -112,7 +112,7 @@ static void *coroutine_swap(struct coroutine *from, struct coroutine *to, void *
CO_DEBUG("LOCK");
g_mutex_lock(&run_lock);
while (!from->runnable) {
- CO_DEBUG("WAIT");
+ CO_DEBUG("WAIT");
g_cond_wait(&run_cond, &run_lock);
}
current = from;
@@ -152,5 +152,5 @@ void *coroutine_yield(void *arg)
gboolean coroutine_is_main(struct coroutine *co)
{
- return (co == &leader);
+ return (co == &leader);
}
diff --git a/src/coroutine_winfibers.c b/src/coroutine_winfibers.c
index 00fa578..0b899d7 100644
--- a/src/coroutine_winfibers.c
+++ b/src/coroutine_winfibers.c
@@ -107,7 +107,7 @@ void *coroutine_yield(void *arg)
gboolean coroutine_is_main(struct coroutine *co)
{
- return (co == &leader);
+ return (co == &leader);
}
/*
* Local variables:
More information about the Spice-commits
mailing list