diff mbox

[V6,1/6] oslib-posix: add helpers for stack alloc and free

Message ID 1471871071-17819-2-git-send-email-pl@kamp.de
State New
Headers show

Commit Message

Peter Lieven Aug. 22, 2016, 1:04 p.m. UTC 
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. The memory for the guard page is deductated from
stack memory so that the usable stack size is effectively reduced by the size
of one page. This is equivalent to how the glibc stack allocation routines
behave.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 include/sysemu/os-posix.h | 27 +++++++++++++++++++++++++++
 util/oslib-posix.c        | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

Comments

Richard Henderson Aug. 22, 2016, 3:19 p.m. UTC | #1 
On 08/22/2016 06:04 AM, Peter Lieven wrote:
> +static size_t adjust_stack_size(size_t sz)
> +{
> +#ifdef _SC_THREAD_STACK_MIN
> +    /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
> +    sz = MAX(MAX(sysconf(_SC_THREAD_STACK_MIN), 0), sz);
> +#endif

You need to place the sysconf result into a local variable.  What you have now 
expands to 4 invocations of the function.

You might also consider passing in the pagesize, since you've already called 
getpagesize in one of the two users of this function.


r~
Peter Lieven Aug. 23, 2016, 7:16 a.m. UTC | #2 
Am 22.08.2016 um 17:19 schrieb Richard Henderson:
> On 08/22/2016 06:04 AM, Peter Lieven wrote:
>> +static size_t adjust_stack_size(size_t sz)
>> +{
>> +#ifdef _SC_THREAD_STACK_MIN
>> +    /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
>> +    sz = MAX(MAX(sysconf(_SC_THREAD_STACK_MIN), 0), sz);
>> +#endif
>
> You need to place the sysconf result into a local variable.  What you have now expands to 4 invocations of the function.

okay, I thought it would only be 2 calls and its not critical. but I can change that.

>
> You might also consider passing in the pagesize, since you've already called getpagesize in one of the two users of this function.

I can change this as well, but isn't getpagesize just a macro on most systems?
If the call of getpagesize is critical we should cache it somewhere as there are more critical sections that call it repeatly.

Peter
diff mbox

Patch

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 9c7dfdf..87e60fe 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -60,4 +60,31 @@  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()). This function also inserts a
+ * guard page to catch a potential stack overflow. The memory
+ * for the guard page is deductated from stack memory so that
+ * the usable stack size is effectively sz bytes minus the size
+ * of one page.
+ *
+ * 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 f2d4e9e..76b028e 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -499,3 +499,49 @@  pid_t qemu_fork(Error **errp)
     }
     return pid;
 }
+
+static size_t adjust_stack_size(size_t sz)
+{
+#ifdef _SC_THREAD_STACK_MIN
+    /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
+    sz = MAX(MAX(sysconf(_SC_THREAD_STACK_MIN), 0), sz);
+#endif
+    /* 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);
+}