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

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

Comments

Jan-Simon Möller - Oct. 25, 2009, 3:22 p.m.
Hi !

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>
Laurent Desnogues - Oct. 25, 2009, 3:54 p.m.
2009/10/25 Jan-Simon Möller <dl9pf@gmx.de>:
> Hi !
>
> 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>
>
>
> diff --git a/configure b/configure
> index 43d87c5..076f45a 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
> @@ -2576,3 +2580,10 @@ d=libuser
>  mkdir -p $d
>  rm -f $d/Makefile
>  ln -s $source_path/Makefile.user $d/Makefile
> +
> +# change default stacksize in usermode
> +#
> +if test "$user_mode_stacksize" != "" ; then
> + echo "user_mode_stack   $user_mode_stacksize"
> + echo "QEMU_CFLAGS+=-DUSER_MODE_STACKSIZE=$user_mode_stacksize" >> $config_host_mak
> +fi

Wouldn't it be better to set this earlier only if target_user_only
is set to yes and use cflags?

Something like this:

if test "$target_user_only" = "yes" -a "$user_mode_stacksize" != ""; then
  cflags="-DUSER_MODE_STACKSIZE $cflags"
fi

just after the test for pie (line 2478) for instance.

The informative echo should go with the others (line 1853).


> diff --git a/linux-user/main.c b/linux-user/main.c
> index 81a1ada..30c0a87 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -51,7 +51,11 @@ 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 */
> +#if defined(USER_MODE_STACKSIZE)
> +unsigned long x86_stack_size = USER_MODE_STACKSIZE;
> +#else
>  unsigned long x86_stack_size = 512 * 1024;
> +#endif

I'd prefer:

#ifndef USER_MODE_STACKSIZE
#define x86_stack_size (512 * 1024)
#endif
unsigned long x86_stack_size = USER_MODE_STACKSIZE;


Laurent

>  void gemu_log(const char *fmt, ...)
>  {
>
>
Jan-Simon Möller - Oct. 25, 2009, 5:41 p.m.
Am Sonntag 25 Oktober 2009 16:54:16 schrieb Laurent Desnogues:
> Wouldn't it be better to set this earlier only if target_user_only
> is set to yes and use cflags?
> 
> Something like this:
> 
> if test "$target_user_only" = "yes" -a "$user_mode_stacksize" != ""; then
>   cflags="-DUSER_MODE_STACKSIZE $cflags"
> fi
> 
> just after the test for pie (line 2478) for instance.
Done. 
> The informative echo should go with the others (line 1853).
Its not clear at this stage, if its "default" or the cmdline value, 
as $target_user_only isn't evaluated until line 2171 .

I'd leave therefore the echo in line 2491.
> 
>
> I'd prefer:
> 
> #ifndef USER_MODE_STACKSIZE
> #define x86_stack_size (512 * 1024)
> #endif
> unsigned long x86_stack_size = USER_MODE_STACKSIZE;
> 
You mean probably?
#ifndef USER_MODE_STACKSIZE
#define USER_MODE_STACKSIZE (512 * 1024)
#endif
unsigned long x86_stack_size = USER_MODE_STACKSIZE;

Done.

Patch will follow in next mail as reply.

Tnx and have phun !

Jan-Simon

Patch

diff --git a/configure b/configure
index 43d87c5..076f45a 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
@@ -2576,3 +2580,10 @@  d=libuser
 mkdir -p $d
 rm -f $d/Makefile
 ln -s $source_path/Makefile.user $d/Makefile
+
+# change default stacksize in usermode
+#
+if test "$user_mode_stacksize" != "" ; then
+ echo "user_mode_stack   $user_mode_stacksize"
+ echo "QEMU_CFLAGS+=-DUSER_MODE_STACKSIZE=$user_mode_stacksize" >> $config_host_mak
+fi
diff --git a/linux-user/main.c b/linux-user/main.c
index 81a1ada..30c0a87 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -51,7 +51,11 @@  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 */
+#if defined(USER_MODE_STACKSIZE)
+unsigned long x86_stack_size = USER_MODE_STACKSIZE;
+#else
 unsigned long x86_stack_size = 512 * 1024;
+#endif
 
 void gemu_log(const char *fmt, ...)
 {