Patchwork Make usermode stacksize (-s) configurable at compile-time

login
register
mail settings
Submitter Jan-Simon Möller
Date Oct. 25, 2009, 5:49 p.m.
Message ID <200910251849.44301.dl9pf@gmx.de>
Download mbox | patch
Permalink /patch/36864/
State New
Headers show

Comments

Jan-Simon Möller - Oct. 25, 2009, 5:49 p.m.
We encontered problems with the low default value for the stacksize in usermode (ok, whats "low"). 
For environments like scratchbox, its hard to use "-s" as qemu is called by binfmt mechanism.
The attached patch makes this configurable at compile-time.

Signed-off-by: Jan-Simon Möller <dl9pf@gmx.de>
Jamie Lokier - Oct. 26, 2009, 12:53 a.m.
Jan-Simon Möller wrote:
> We encontered problems with the low default value for the stacksize in usermode (ok, whats "low"). 
> For environments like scratchbox, its hard to use "-s" as qemu is called by binfmt mechanism.
> The attached patch makes this configurable at compile-time.

Note, this will only change the stack size for the main thread, at
least with LinuxThreads.  Other thread stacks are allocated by the
pthread (LinuxThreads or NPTL), and their stack size is changed by
pthread_attr_setstacksize instead.

-- Jamie
Riku Voipio - Oct. 26, 2009, 12:25 p.m.
On Sun, Oct 25, 2009 at 06:49:44PM +0100, Jan-Simon Möller wrote:
> We encontered problems with the low default value for the stacksize in usermode (ok, whats "low"). 
> For environments like scratchbox, its hard to use "-s" as qemu is called by binfmt mechanism.
> The attached patch makes this configurable at compile-time.

fwiw in scratchbox we have a wrapper in between binfmt and qemu (called misc_runner) that sets any
necessary command line parameters for qemu. You might want to consider a similar approach so we
don't need to add a configure option (and thus hardcode the setting) to qemu for every command
line option that might need changing.

> Signed-off-by: Jan-Simon Möller <dl9pf@gmx.de>
> 
> diff --git a/configure b/configure                                     
> index 43d87c5..dad9175 100755                                          
> --- a/configure                                                        
> +++ b/configure                                                        
> @@ -220,6 +220,7 @@ uname_release=""                                   
>  io_thread="no"                                                        
>  mixemu="no"                                                           
>  kerneldir=""                                                          
> +user_mode_stacksize=""                                                
>  aix="no"                                                              
>  blobs="yes"                                                           
>  pkgversion=""                                                         
> @@ -557,6 +558,8 @@ for opt do                                         
>    ;;                                                                  
>    --kerneldir=*) kerneldir="$optarg"                                  
>    ;;                                                                  
> +  --user-mode-stacksize=*) user_mode_stacksize="$optarg"              
> +  ;;                                                                  
>    --with-pkgversion=*) pkgversion=" ($optarg)"                        
>    ;;                                                                  
>    --disable-docs) docs="no"                                           
> @@ -713,6 +716,7 @@ echo "  --enable-linux-aio       enable Linux AIO support"
>  echo "  --enable-io-thread       enable IO thread"
>  echo "  --disable-blobs          disable installing provided firmware blobs"
>  echo "  --kerneldir=PATH         look for kernel includes in PATH"
> +echo "  --user-mode-stacksize=   set default stack size in bytes (as -s, only in usermode)"
>  echo ""
>  echo "NOTE: The object files are built at the place where configure is launched"
>  exit 1
> @@ -2481,6 +2485,16 @@ if test "$target_user_only" = "yes" -a "$static" = "no" -a \
>    ldflags="-pie $ldflags"
>  fi
> 
> +# change default stacksize in usermode
> +#
> +if test "$target_user_only" = "yes" -a "$user_mode_stacksize" != "" ; then
> + cflags="-DUSER_MODE_STACKSIZE=$user_mode_stacksize $cflags"
> + echo "user_mode_stack   $user_mode_stacksize"
> +else
> + echo "user_mode_stack   default"
> +fi
> +
> +
>  if test "$target_softmmu" = "yes" -a \( \
>          "$TARGET_ARCH" = "microblaze" -o \
>          "$TARGET_ARCH" = "cris" \) ; then
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 81a1ada..9ac2421 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -51,7 +51,10 @@ const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
>  /* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
>     we allocate a bigger stack. Need a better solution, for example
>     by remapping the process stack directly at the right place */
> -unsigned long x86_stack_size = 512 * 1024;
> +#ifndef USER_MODE_STACKSIZE
> +#define USER_MODE_STACKSIZE (512 * 1024)
> +#endif
> +unsigned long x86_stack_size = USER_MODE_STACKSIZE;
> 
>  void gemu_log(const char *fmt, ...)
>  {
> 

> diff --git a/configure b/configure
> index 43d87c5..dad9175 100755
> --- a/configure
> +++ b/configure
> @@ -220,6 +220,7 @@ uname_release=""
>  io_thread="no"
>  mixemu="no"
>  kerneldir=""
> +user_mode_stacksize=""
>  aix="no"
>  blobs="yes"
>  pkgversion=""
> @@ -557,6 +558,8 @@ for opt do
>    ;;
>    --kerneldir=*) kerneldir="$optarg"
>    ;;
> +  --user-mode-stacksize=*) user_mode_stacksize="$optarg"
> +  ;;
>    --with-pkgversion=*) pkgversion=" ($optarg)"
>    ;;
>    --disable-docs) docs="no"
> @@ -713,6 +716,7 @@ echo "  --enable-linux-aio       enable Linux AIO support"
>  echo "  --enable-io-thread       enable IO thread"
>  echo "  --disable-blobs          disable installing provided firmware blobs"
>  echo "  --kerneldir=PATH         look for kernel includes in PATH"
> +echo "  --user-mode-stacksize=   set default stack size in bytes (as -s, only in usermode)"
>  echo ""
>  echo "NOTE: The object files are built at the place where configure is launched"
>  exit 1
> @@ -2481,6 +2485,16 @@ if test "$target_user_only" = "yes" -a "$static" = "no" -a \
>    ldflags="-pie $ldflags"
>  fi
>  
> +# change default stacksize in usermode
> +#
> +if test "$target_user_only" = "yes" -a "$user_mode_stacksize" != "" ; then
> + cflags="-DUSER_MODE_STACKSIZE=$user_mode_stacksize $cflags"
> + echo "user_mode_stack   $user_mode_stacksize"
> +else
> + echo "user_mode_stack   default"
> +fi
> +
> +
>  if test "$target_softmmu" = "yes" -a \( \
>          "$TARGET_ARCH" = "microblaze" -o \
>          "$TARGET_ARCH" = "cris" \) ; then
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 81a1ada..9ac2421 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -51,7 +51,10 @@ const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
>  /* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
>     we allocate a bigger stack. Need a better solution, for example
>     by remapping the process stack directly at the right place */
> -unsigned long x86_stack_size = 512 * 1024;
> +#ifndef USER_MODE_STACKSIZE
> +#define USER_MODE_STACKSIZE (512 * 1024)
> +#endif
> +unsigned long x86_stack_size = USER_MODE_STACKSIZE;
>  
>  void gemu_log(const char *fmt, ...)
>  {
Jan-Simon Möller - Oct. 27, 2009, 7:56 p.m.
> > The attached patch makes this configurable at compile-time.
> 
> fwiw in scratchbox we have a wrapper in between binfmt and qemu (called misc_runner) that sets any
> necessary command line parameters for qemu. You might want to consider a similar approach so we
> don't need to add a configure option (and thus hardcode the setting) to qemu for every command
> line option that might need changing.
I got your point. But OTOH I just make the default configurable.

IMHO this leads to:
* add an (optional) config-file for the usermode to overwrite defaults
 or
* extend and generalize the wrapper and use it in binfmt instead of qemu directly

Let me sleep over it a bit - for now: other opinions ?

Best,
Jan-Simon

Patch

diff --git a/configure b/configure
index 43d87c5..dad9175 100755
--- a/configure
+++ b/configure
@@ -220,6 +220,7 @@  uname_release=""
 io_thread="no"
 mixemu="no"
 kerneldir=""
+user_mode_stacksize=""
 aix="no"
 blobs="yes"
 pkgversion=""
@@ -557,6 +558,8 @@  for opt do
   ;;
   --kerneldir=*) kerneldir="$optarg"
   ;;
+  --user-mode-stacksize=*) user_mode_stacksize="$optarg"
+  ;;
   --with-pkgversion=*) pkgversion=" ($optarg)"
   ;;
   --disable-docs) docs="no"
@@ -713,6 +716,7 @@  echo "  --enable-linux-aio       enable Linux AIO support"
 echo "  --enable-io-thread       enable IO thread"
 echo "  --disable-blobs          disable installing provided firmware blobs"
 echo "  --kerneldir=PATH         look for kernel includes in PATH"
+echo "  --user-mode-stacksize=   set default stack size in bytes (as -s, only in usermode)"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
@@ -2481,6 +2485,16 @@  if test "$target_user_only" = "yes" -a "$static" = "no" -a \
   ldflags="-pie $ldflags"
 fi
 
+# change default stacksize in usermode
+#
+if test "$target_user_only" = "yes" -a "$user_mode_stacksize" != "" ; then
+ cflags="-DUSER_MODE_STACKSIZE=$user_mode_stacksize $cflags"
+ echo "user_mode_stack   $user_mode_stacksize"
+else
+ echo "user_mode_stack   default"
+fi
+
+
 if test "$target_softmmu" = "yes" -a \( \
         "$TARGET_ARCH" = "microblaze" -o \
         "$TARGET_ARCH" = "cris" \) ; then
diff --git a/linux-user/main.c b/linux-user/main.c
index 81a1ada..9ac2421 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -51,7 +51,10 @@  const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
 /* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
    we allocate a bigger stack. Need a better solution, for example
    by remapping the process stack directly at the right place */
-unsigned long x86_stack_size = 512 * 1024;
+#ifndef USER_MODE_STACKSIZE
+#define USER_MODE_STACKSIZE (512 * 1024)
+#endif
+unsigned long x86_stack_size = USER_MODE_STACKSIZE;
 
 void gemu_log(const char *fmt, ...)
 {