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

[Stefan Hajnoczi]

On Mon, Aug 08, 2016 at 08:29:58PM +0200, Peter Lieven wrote:
> Am 08.08.2016 um 12:37 schrieb Stefan Hajnoczi:
> > On Tue, Jul 12, 2016 at 06:23:01PM +0200, 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 <address@hidden>
> >> ---
> >>  include/sysemu/os-posix.h | 23 +++++++++++++++++++++++
> >>  util/oslib-posix.c        | 46 
> >> ++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 69 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..2303ca6 100644
> >> --- a/util/oslib-posix.c
> >> +++ b/util/oslib-posix.c
> >> @@ -497,3 +497,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,
> > It's cleaner to count for the guard page separately and give the caller
> > the sz bytes they expected:
> >
> >   sz + pagesz
> 
> I don't mind to change that, but the glibc stack allocation routine
> does it exactly this way. (see glibc/nptl/allocatestack.c).
> As we already used other parts (minimal stack size, positioning
> of the guard page), we mimicked this as well.

Okay but please include a comment explaining the rationale.

Stefan

signature.asc
Description: PGP signature