* [Qemu-devel] [PATCH V4 0/6] coroutine: mmap stack memory and stack size
@ 2016-07-11 9:07 Peter Lieven
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free Peter Lieven
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Peter Lieven @ 2016-07-11 9:07 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, eblake,
rth, armbru, Peter Lieven
I decided to split this from the rest of the Qemu RSS usage series as
it contains the more or less non contentious patches.
I omitted the MAP_GROWSDOWN flag in mmap as we are not 100% sure which
side effects it has.
I kept the guard page which is now nicely makes the stacks visible in
smaps. The old version of the relevent patch lacked the MAP_FIXED flag
in the second call to mmap.
The last patch which reduces the stack size of coroutines to 64kB
may be omitted if its found to risky.
v3->v4:
- Patch 1: add a static function to adjust the stack size [Richard]
- Patch 1: round up the stack size to multiple of the pagesize.
v2->v3:
- Patch 1,6: adjusted commit message to mention the guard page [Markus]
v1->v2:
- Patch 1: added an architecture dependend guard page [Richard]
- Patch 1: avoid stacks smaller than _SC_THREAD_STACK_MIN [Richard]
- Patch 1: use mmap+mprotect instead of mmap+mmap [Richard]
- Patch 5: u_int32_t -> uint32_t [Richard]
- Patch 5: only available if stack grows down
Peter Lieven (6):
oslib-posix: add helpers for stack alloc and free
coroutine: add a macro for the coroutine stack size
coroutine-ucontext: use helper for allocating stack memory
coroutine-sigaltstack: use helper for allocating stack memory
oslib-posix: add a configure switch to debug stack usage
coroutine: reduce stack size to 64kB
configure | 19 +++++++++++
include/qemu/coroutine_int.h | 2 ++
include/sysemu/os-posix.h | 23 +++++++++++++
util/coroutine-sigaltstack.c | 7 ++--
util/coroutine-ucontext.c | 9 +++--
util/coroutine-win32.c | 2 +-
util/oslib-posix.c | 81 ++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 133 insertions(+), 10 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free
2016-07-11 9:07 [Qemu-devel] [PATCH V4 0/6] coroutine: mmap stack memory and stack size Peter Lieven
@ 2016-07-11 9:07 ` Peter Lieven
2016-07-11 16:28 ` Richard Henderson
2016-07-11 16:39 ` Eric Blake
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 2/6] coroutine: add a macro for the coroutine stack size Peter Lieven
` (4 subsequent siblings)
5 siblings, 2 replies; 12+ messages in thread
From: Peter Lieven @ 2016-07-11 9:07 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, eblake,
rth, armbru, Peter Lieven
the allocated stack will be adjusted to the minimum supported stack size
by the OS and rounded up to be a multiple of the system pagesize.
Additionally an architecture dependent guard page is added to the stack
to catch stack overflows.
Signed-off-by: Peter Lieven <pl@kamp.de>
---
include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
util/oslib-posix.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 67 insertions(+)
diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 9c7dfdf..7630665 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -60,4 +60,27 @@ int qemu_utimens(const char *path, const qemu_timespec *times);
bool is_daemonized(void);
+/**
+ * qemu_alloc_stack:
+ * @sz: size of required stack in bytes
+ *
+ * Allocate memory that can be used as a stack, for instance for
+ * coroutines. If the memory cannot be allocated, this function
+ * will abort (like g_malloc()).
+ *
+ * The allocated stack must be freed with qemu_free_stack().
+ *
+ * Returns: pointer to (the lowest address of) the stack memory.
+ */
+void *qemu_alloc_stack(size_t sz);
+
+/**
+ * qemu_free_stack:
+ * @stack: stack to free
+ * @sz: size of stack in bytes
+ *
+ * Free a stack allocated via qemu_alloc_stack().
+ */
+void qemu_free_stack(void *stack, size_t sz);
+
#endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e2e1d4d..9e7bc65 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -497,3 +497,47 @@ pid_t qemu_fork(Error **errp)
}
return pid;
}
+
+static size_t adjust_stack_size(size_t sz)
+{
+ /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
+ sz = MAX(sz, sysconf(_SC_THREAD_STACK_MIN));
+ /* adjust stack size to a multiple of the page size */
+ sz = ROUND_UP(sz, getpagesize());
+ return sz;
+}
+
+void *qemu_alloc_stack(size_t sz)
+{
+ void *ptr, *guardpage;
+ size_t pagesz = getpagesize();
+ sz = adjust_stack_size(sz);
+
+ ptr = mmap(NULL, sz, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (ptr == MAP_FAILED) {
+ abort();
+ }
+
+#if defined(HOST_IA64)
+ /* separate register stack */
+ guardpage = ptr + (((sz - pagesz) / 2) & ~pagesz);
+#elif defined(HOST_HPPA)
+ /* stack grows up */
+ guardpage = ptr + sz - pagesz;
+#else
+ /* stack grows down */
+ guardpage = ptr;
+#endif
+ if (mprotect(guardpage, pagesz, PROT_NONE) != 0) {
+ abort();
+ }
+
+ return ptr;
+}
+
+void qemu_free_stack(void *stack, size_t sz)
+{
+ sz = adjust_stack_size(sz);
+ munmap(stack, sz);
+}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V4 2/6] coroutine: add a macro for the coroutine stack size
2016-07-11 9:07 [Qemu-devel] [PATCH V4 0/6] coroutine: mmap stack memory and stack size Peter Lieven
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free Peter Lieven
@ 2016-07-11 9:07 ` Peter Lieven
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 3/6] coroutine-ucontext: use helper for allocating stack memory Peter Lieven
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Peter Lieven @ 2016-07-11 9:07 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, eblake,
rth, armbru, Peter Lieven
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
include/qemu/coroutine_int.h | 2 ++
util/coroutine-sigaltstack.c | 2 +-
util/coroutine-ucontext.c | 2 +-
util/coroutine-win32.c | 2 +-
4 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index 42d6838..eac323a 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -28,6 +28,8 @@
#include "qemu/queue.h"
#include "qemu/coroutine.h"
+#define COROUTINE_STACK_SIZE (1 << 20)
+
typedef enum {
COROUTINE_YIELD = 1,
COROUTINE_TERMINATE = 2,
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index a7c3366..9c2854c 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -143,7 +143,7 @@ static void coroutine_trampoline(int signal)
Coroutine *qemu_coroutine_new(void)
{
- const size_t stack_size = 1 << 20;
+ const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineUContext *co;
CoroutineThreadState *coTS;
struct sigaction sa;
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 2bb7e10..31254ab 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -82,7 +82,7 @@ static void coroutine_trampoline(int i0, int i1)
Coroutine *qemu_coroutine_new(void)
{
- const size_t stack_size = 1 << 20;
+ const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineUContext *co;
ucontext_t old_uc, uc;
sigjmp_buf old_env;
diff --git a/util/coroutine-win32.c b/util/coroutine-win32.c
index 02e28e8..de6bd4f 100644
--- a/util/coroutine-win32.c
+++ b/util/coroutine-win32.c
@@ -71,7 +71,7 @@ static void CALLBACK coroutine_trampoline(void *co_)
Coroutine *qemu_coroutine_new(void)
{
- const size_t stack_size = 1 << 20;
+ const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineWin32 *co;
co = g_malloc0(sizeof(*co));
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V4 3/6] coroutine-ucontext: use helper for allocating stack memory
2016-07-11 9:07 [Qemu-devel] [PATCH V4 0/6] coroutine: mmap stack memory and stack size Peter Lieven
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free Peter Lieven
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 2/6] coroutine: add a macro for the coroutine stack size Peter Lieven
@ 2016-07-11 9:07 ` Peter Lieven
2016-07-11 9:08 ` [Qemu-devel] [PATCH V4 4/6] coroutine-sigaltstack: " Peter Lieven
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Peter Lieven @ 2016-07-11 9:07 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, eblake,
rth, armbru, Peter Lieven
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
util/coroutine-ucontext.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 31254ab..b7dea8c 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -82,7 +82,6 @@ static void coroutine_trampoline(int i0, int i1)
Coroutine *qemu_coroutine_new(void)
{
- const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineUContext *co;
ucontext_t old_uc, uc;
sigjmp_buf old_env;
@@ -101,17 +100,17 @@ Coroutine *qemu_coroutine_new(void)
}
co = g_malloc0(sizeof(*co));
- co->stack = g_malloc(stack_size);
+ co->stack = qemu_alloc_stack(COROUTINE_STACK_SIZE);
co->base.entry_arg = &old_env; /* stash away our jmp_buf */
uc.uc_link = &old_uc;
uc.uc_stack.ss_sp = co->stack;
- uc.uc_stack.ss_size = stack_size;
+ uc.uc_stack.ss_size = COROUTINE_STACK_SIZE;
uc.uc_stack.ss_flags = 0;
#ifdef CONFIG_VALGRIND_H
co->valgrind_stack_id =
- VALGRIND_STACK_REGISTER(co->stack, co->stack + stack_size);
+ VALGRIND_STACK_REGISTER(co->stack, co->stack + COROUTINE_STACK_SIZE);
#endif
arg.p = co;
@@ -149,7 +148,7 @@ void qemu_coroutine_delete(Coroutine *co_)
valgrind_stack_deregister(co);
#endif
- g_free(co->stack);
+ qemu_free_stack(co->stack, COROUTINE_STACK_SIZE);
g_free(co);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V4 4/6] coroutine-sigaltstack: use helper for allocating stack memory
2016-07-11 9:07 [Qemu-devel] [PATCH V4 0/6] coroutine: mmap stack memory and stack size Peter Lieven
` (2 preceding siblings ...)
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 3/6] coroutine-ucontext: use helper for allocating stack memory Peter Lieven
@ 2016-07-11 9:08 ` Peter Lieven
2016-07-11 9:08 ` [Qemu-devel] [PATCH V4 5/6] oslib-posix: add a configure switch to debug stack usage Peter Lieven
2016-07-11 9:08 ` [Qemu-devel] [PATCH V4 6/6] coroutine: reduce stack size to 64kB Peter Lieven
5 siblings, 0 replies; 12+ messages in thread
From: Peter Lieven @ 2016-07-11 9:08 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, eblake,
rth, armbru, Peter Lieven
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
util/coroutine-sigaltstack.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index 9c2854c..ccf4861 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -143,7 +143,6 @@ static void coroutine_trampoline(int signal)
Coroutine *qemu_coroutine_new(void)
{
- const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineUContext *co;
CoroutineThreadState *coTS;
struct sigaction sa;
@@ -164,7 +163,7 @@ Coroutine *qemu_coroutine_new(void)
*/
co = g_malloc0(sizeof(*co));
- co->stack = g_malloc(stack_size);
+ co->stack = qemu_alloc_stack(COROUTINE_STACK_SIZE);
co->base.entry_arg = &old_env; /* stash away our jmp_buf */
coTS = coroutine_get_thread_state();
@@ -189,7 +188,7 @@ Coroutine *qemu_coroutine_new(void)
* Set the new stack.
*/
ss.ss_sp = co->stack;
- ss.ss_size = stack_size;
+ ss.ss_size = COROUTINE_STACK_SIZE;
ss.ss_flags = 0;
if (sigaltstack(&ss, &oss) < 0) {
abort();
@@ -253,7 +252,7 @@ void qemu_coroutine_delete(Coroutine *co_)
{
CoroutineUContext *co = DO_UPCAST(CoroutineUContext, base, co_);
- g_free(co->stack);
+ qemu_free_stack(co->stack, COROUTINE_STACK_SIZE);
g_free(co);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V4 5/6] oslib-posix: add a configure switch to debug stack usage
2016-07-11 9:07 [Qemu-devel] [PATCH V4 0/6] coroutine: mmap stack memory and stack size Peter Lieven
` (3 preceding siblings ...)
2016-07-11 9:08 ` [Qemu-devel] [PATCH V4 4/6] coroutine-sigaltstack: " Peter Lieven
@ 2016-07-11 9:08 ` Peter Lieven
2016-07-11 9:08 ` [Qemu-devel] [PATCH V4 6/6] coroutine: reduce stack size to 64kB Peter Lieven
5 siblings, 0 replies; 12+ messages in thread
From: Peter Lieven @ 2016-07-11 9:08 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, eblake,
rth, armbru, Peter Lieven
this adds a knob to track the maximum stack usage of stacks
created by qemu_alloc_stack.
Signed-off-by: Peter Lieven <pl@kamp.de>
---
configure | 19 +++++++++++++++++++
util/oslib-posix.c | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/configure b/configure
index e9090a0..72685ea 100755
--- a/configure
+++ b/configure
@@ -296,6 +296,7 @@ libiscsi=""
libnfs=""
coroutine=""
coroutine_pool=""
+debug_stack_usage="no"
seccomp=""
glusterfs=""
glusterfs_xlator_opt="no"
@@ -1004,6 +1005,8 @@ for opt do
;;
--enable-coroutine-pool) coroutine_pool="yes"
;;
+ --enable-debug-stack-usage) debug_stack_usage="yes"
+ ;;
--disable-docs) docs="no"
;;
--enable-docs) docs="yes"
@@ -4301,6 +4304,17 @@ if test "$coroutine" = "gthread" -a "$coroutine_pool" = "yes"; then
error_exit "'gthread' coroutine backend does not support pool (use --disable-coroutine-pool)"
fi
+if test "$debug_stack_usage" = "yes"; then
+ if test "$cpu" = "ia64" -o "$cpu" = "hppa"; then
+ error_exit "stack usage debugging is not supported for $cpu"
+ fi
+ if test "$coroutine_pool" = "yes"; then
+ echo "WARN: disabling coroutine pool for stack usage debugging"
+ coroutine_pool=no
+ fi
+fi
+
+
##########################################
# check if we have open_by_handle_at
@@ -4885,6 +4899,7 @@ echo "QGA MSI support $guest_agent_msi"
echo "seccomp support $seccomp"
echo "coroutine backend $coroutine"
echo "coroutine pool $coroutine_pool"
+echo "debug stack usage $debug_stack_usage"
echo "GlusterFS support $glusterfs"
echo "Archipelago support $archipelago"
echo "gcov $gcov_tool"
@@ -5353,6 +5368,10 @@ else
echo "CONFIG_COROUTINE_POOL=0" >> $config_host_mak
fi
+if test "$debug_stack_usage" = "yes" ; then
+ echo "CONFIG_DEBUG_STACK_USAGE=y" >> $config_host_mak
+fi
+
if test "$open_by_handle_at" = "yes" ; then
echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak
fi
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 9e7bc65..860effb 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -50,6 +50,10 @@
#include <qemu/mmap-alloc.h>
+#ifdef CONFIG_DEBUG_STACK_USAGE
+#include "qemu/error-report.h"
+#endif
+
int qemu_get_thread_id(void)
{
#if defined(__linux__)
@@ -510,6 +514,9 @@ static size_t adjust_stack_size(size_t sz)
void *qemu_alloc_stack(size_t sz)
{
void *ptr, *guardpage;
+#ifdef CONFIG_DEBUG_STACK_USAGE
+ void *ptr2;
+#endif
size_t pagesz = getpagesize();
sz = adjust_stack_size(sz);
@@ -533,11 +540,41 @@ void *qemu_alloc_stack(size_t sz)
abort();
}
+#ifdef CONFIG_DEBUG_STACK_USAGE
+ for (ptr2 = ptr + pagesz; ptr2 < ptr + sz; ptr2 += sizeof(uint32_t)) {
+ *(uint32_t *)ptr2 = 0xdeadbeaf;
+ }
+#endif
+
return ptr;
}
+#ifdef CONFIG_DEBUG_STACK_USAGE
+static __thread unsigned int max_stack_usage;
+#endif
+
void qemu_free_stack(void *stack, size_t sz)
{
+#ifdef CONFIG_DEBUG_STACK_USAGE
+ unsigned int usage;
+ void *ptr;
+#endif
sz = adjust_stack_size(sz);
+
+#ifdef CONFIG_DEBUG_STACK_USAGE
+ for (ptr = stack + getpagesize(); ptr < stack + sz;
+ ptr += sizeof(uint32_t)) {
+ if (*(uint32_t *)ptr != 0xdeadbeaf) {
+ break;
+ }
+ }
+ usage = sz - (uintptr_t) (ptr - stack);
+ if (usage > max_stack_usage) {
+ error_report("thread %d max stack usage increased from %u to %u",
+ qemu_get_thread_id(), max_stack_usage, usage);
+ max_stack_usage = usage;
+ }
+#endif
+
munmap(stack, sz);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V4 6/6] coroutine: reduce stack size to 64kB
2016-07-11 9:07 [Qemu-devel] [PATCH V4 0/6] coroutine: mmap stack memory and stack size Peter Lieven
` (4 preceding siblings ...)
2016-07-11 9:08 ` [Qemu-devel] [PATCH V4 5/6] oslib-posix: add a configure switch to debug stack usage Peter Lieven
@ 2016-07-11 9:08 ` Peter Lieven
5 siblings, 0 replies; 12+ messages in thread
From: Peter Lieven @ 2016-07-11 9:08 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, eblake,
rth, armbru, Peter Lieven
evaluation with the recently introduced maximum stack usage monitoring revealed
that the actual used stack size was never above 4kB so allocating 1MB stack
for each coroutine is a lot of wasted memory. So reduce the stack size to
64kB which should still give enough head room. The guard page added
in qemu_alloc_stack will catch a potential stack overflow introduced
by this commit.
Signed-off-by: Peter Lieven <pl@kamp.de>
---
include/qemu/coroutine_int.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index eac323a..f84d777 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -28,7 +28,7 @@
#include "qemu/queue.h"
#include "qemu/coroutine.h"
-#define COROUTINE_STACK_SIZE (1 << 20)
+#define COROUTINE_STACK_SIZE (1 << 16)
typedef enum {
COROUTINE_YIELD = 1,
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free Peter Lieven
@ 2016-07-11 16:28 ` Richard Henderson
2016-07-11 16:39 ` Eric Blake
1 sibling, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2016-07-11 16:28 UTC (permalink / raw)
To: Peter Lieven, qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, eblake,
armbru
On 07/11/2016 02:07 AM, Peter Lieven wrote:
> the allocated stack will be adjusted to the minimum supported stack size
> by the OS and rounded up to be a multiple of the system pagesize.
> Additionally an architecture dependent guard page is added to the stack
> to catch stack overflows.
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
> include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
> util/oslib-posix.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 67 insertions(+)
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free Peter Lieven
2016-07-11 16:28 ` Richard Henderson
@ 2016-07-11 16:39 ` Eric Blake
2016-07-12 14:36 ` Peter Lieven
1 sibling, 1 reply; 12+ messages in thread
From: Eric Blake @ 2016-07-11 16:39 UTC (permalink / raw)
To: Peter Lieven, qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, rth,
armbru
[-- Attachment #1: Type: text/plain, Size: 1343 bytes --]
On 07/11/2016 03:07 AM, Peter Lieven wrote:
> the allocated stack will be adjusted to the minimum supported stack size
> by the OS and rounded up to be a multiple of the system pagesize.
> Additionally an architecture dependent guard page is added to the stack
> to catch stack overflows.
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
> include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
> util/oslib-posix.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 67 insertions(+)
>
> +
> +static size_t adjust_stack_size(size_t sz)
> +{
> + /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
> + sz = MAX(sz, sysconf(_SC_THREAD_STACK_MIN));
sz is unsigned, but sysconf() is signed. Furthermore, sysconf() is
permitted to return -1 if there is no such minimum. MAX() would then
operate on the common integral promotion between the two arguments,
which may treat (unsigned)(-1) as the larger of the two values, and give
you the wrong results.
I think it is theoretical (all platforms that we compile on have a
working sysconf(_SC_THREAD_STACK_MIN), right?), but still may be worth
being sure that sysconf() returned a positive value before computing MAX().
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free
2016-07-11 16:39 ` Eric Blake
@ 2016-07-12 14:36 ` Peter Lieven
2016-07-12 14:41 ` Peter Lieven
0 siblings, 1 reply; 12+ messages in thread
From: Peter Lieven @ 2016-07-12 14:36 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, rth,
armbru
Am 11.07.2016 um 18:39 schrieb Eric Blake:
> On 07/11/2016 03:07 AM, Peter Lieven wrote:
>> the allocated stack will be adjusted to the minimum supported stack size
>> by the OS and rounded up to be a multiple of the system pagesize.
>> Additionally an architecture dependent guard page is added to the stack
>> to catch stack overflows.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>> include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
>> util/oslib-posix.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 67 insertions(+)
>>
>> +
>> +static size_t adjust_stack_size(size_t sz)
>> +{
>> + /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
>> + sz = MAX(sz, sysconf(_SC_THREAD_STACK_MIN));
> sz is unsigned, but sysconf() is signed. Furthermore, sysconf() is
> permitted to return -1 if there is no such minimum. MAX() would then
> operate on the common integral promotion between the two arguments,
> which may treat (unsigned)(-1) as the larger of the two values, and give
> you the wrong results.
>
> I think it is theoretical (all platforms that we compile on have a
> working sysconf(_SC_THREAD_STACK_MIN), right?), but still may be worth
> being sure that sysconf() returned a positive value before computing MAX().
>
If you feel more comfortable I can surround it by a
if (sysconf(_SC_THREAD_STACK_MIN) > 0) { }
I wonder if the _SC_THREAD_STACK_MIN constant exists if there is no minimum?
Peter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free
2016-07-12 14:36 ` Peter Lieven
@ 2016-07-12 14:41 ` Peter Lieven
2016-07-12 15:32 ` Eric Blake
0 siblings, 1 reply; 12+ messages in thread
From: Peter Lieven @ 2016-07-12 14:41 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, rth,
armbru
Am 12.07.2016 um 16:36 schrieb Peter Lieven:
> Am 11.07.2016 um 18:39 schrieb Eric Blake:
>> On 07/11/2016 03:07 AM, Peter Lieven wrote:
>>> the allocated stack will be adjusted to the minimum supported stack size
>>> by the OS and rounded up to be a multiple of the system pagesize.
>>> Additionally an architecture dependent guard page is added to the stack
>>> to catch stack overflows.
>>>
>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>> ---
>>> include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
>>> util/oslib-posix.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 67 insertions(+)
>>>
>>> +
>>> +static size_t adjust_stack_size(size_t sz)
>>> +{
>>> + /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
>>> + sz = MAX(sz, sysconf(_SC_THREAD_STACK_MIN));
>> sz is unsigned, but sysconf() is signed. Furthermore, sysconf() is
>> permitted to return -1 if there is no such minimum. MAX() would then
>> operate on the common integral promotion between the two arguments,
>> which may treat (unsigned)(-1) as the larger of the two values, and give
>> you the wrong results.
>>
>> I think it is theoretical (all platforms that we compile on have a
>> working sysconf(_SC_THREAD_STACK_MIN), right?), but still may be worth
>> being sure that sysconf() returned a positive value before computing MAX().
>>
> If you feel more comfortable I can surround it by a
>
> if (sysconf(_SC_THREAD_STACK_MIN) > 0) { }
>
> I wonder if the _SC_THREAD_STACK_MIN constant exists if there is no minimum?
Update:
glibc basically does the following:
static gulong g_thread_min_stack_size = 0;
#ifdef _SC_THREAD_STACK_MIN
g_thread_min_stack_size = MAX (sysconf (_SC_THREAD_STACK_MIN), 0);
#endif /* _SC_THREAD_STACK_MIN */
stack_size = MAX (g_thread_min_stack_size, stack_size);
So we should do sth similar, I think?!
Peter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free
2016-07-12 14:41 ` Peter Lieven
@ 2016-07-12 15:32 ` Eric Blake
0 siblings, 0 replies; 12+ messages in thread
From: Eric Blake @ 2016-07-12 15:32 UTC (permalink / raw)
To: Peter Lieven, qemu-devel
Cc: kwolf, mreitz, pbonzini, mst, dgilbert, peter.maydell, rth,
armbru
[-- Attachment #1: Type: text/plain, Size: 646 bytes --]
On 07/12/2016 08:41 AM, Peter Lieven wrote:
>> I wonder if the _SC_THREAD_STACK_MIN constant exists if there is no minimum?
>
> Update:
>
> glibc basically does the following:
>
> static gulong g_thread_min_stack_size = 0;
>
> #ifdef _SC_THREAD_STACK_MIN
> g_thread_min_stack_size = MAX (sysconf (_SC_THREAD_STACK_MIN), 0);
> #endif /* _SC_THREAD_STACK_MIN */
>
> stack_size = MAX (g_thread_min_stack_size, stack_size);
>
>
> So we should do sth similar, I think?!
Yes, that would be a good pattern to copy.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-07-12 15:33 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-11 9:07 [Qemu-devel] [PATCH V4 0/6] coroutine: mmap stack memory and stack size Peter Lieven
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 1/6] oslib-posix: add helpers for stack alloc and free Peter Lieven
2016-07-11 16:28 ` Richard Henderson
2016-07-11 16:39 ` Eric Blake
2016-07-12 14:36 ` Peter Lieven
2016-07-12 14:41 ` Peter Lieven
2016-07-12 15:32 ` Eric Blake
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 2/6] coroutine: add a macro for the coroutine stack size Peter Lieven
2016-07-11 9:07 ` [Qemu-devel] [PATCH V4 3/6] coroutine-ucontext: use helper for allocating stack memory Peter Lieven
2016-07-11 9:08 ` [Qemu-devel] [PATCH V4 4/6] coroutine-sigaltstack: " Peter Lieven
2016-07-11 9:08 ` [Qemu-devel] [PATCH V4 5/6] oslib-posix: add a configure switch to debug stack usage Peter Lieven
2016-07-11 9:08 ` [Qemu-devel] [PATCH V4 6/6] coroutine: reduce stack size to 64kB Peter Lieven
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.