Re: [Qemu-devel] [PATCH V8 1/6] oslib-posix: add helpers for stack alloc and free

[Kevin Wolf <kwolf@redhat.com>]

Am 26.09.2016 um 16:43 hat Peter Lieven geschrieben:
> Am 26.09.2016 um 15:44 schrieb Kevin Wolf:
> >Am 26.09.2016 um 13:44 hat Peter Lieven geschrieben:
> >>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 | 27 +++++++++++++++++++++++++++
> >>  util/oslib-posix.c        | 43 +++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 70 insertions(+)
> >>
> >>diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
> >>index 9c7dfdf..4a0f493 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: pointer to a size_t holding the requested stack size
> >>+ *
> >>+ * 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 an
> >>+ * additional guard page to catch a potential stack overflow.
> >>+ * Note that the useable stack memory can be greater than the
> >>+ * requested stack size due to alignment and minimal stack size
> >>+ * restrictions. In this case the value of sz is adjusted.
> >>+ *
> >>+ * The allocated stack must be freed with qemu_free_stack().
> >>+ *
> >>+ * Returns: pointer to (the lowest address of) the stack memory.
> >Not quite. It's the pointer to the lowest address of the guard page,
> >while the returned stack size doesn't include the guard page. This is an
> >awkward interface, and consequently patch 3 fails to use it correctly.
> >
> >So you end up with something like:
> >
> >     |GGGG|....|....|....|
> >      **** **** ****
> >
> >     G = guard page
> >     . = allocated stack page
> >     * = stack as used for makecontext()
> >
> >That is, the guard page is included in the stack used to create the
> >coroutine context, and the last page stays unused. On systems where we
> >only allocate a single page for the stack, this obviously means that the
> >tests still fail.
> 
> you are right. so I should adjust the size to allocsz instead?

That's probably the easiest fix.

Kevin

> the other option would be to keep version 7 of this series and
> adjust the COROUTINE_SIZE to MAX(2*pagesize(), 1 << 16) to
> avoid the problem?
> 
> Peter