Re: [Qemu-devel] [PATCH] coroutine: add ./configure --disable-coroutine-pool

[Kevin Wolf <kwolf@redhat.com>]

Am 11.09.2013 um 16:42 hat Stefan Hajnoczi geschrieben:
> The 'gthread' coroutine backend was written before the freelist (aka
> pool) existed in qemu-coroutine.c.
> 
> This means that every thread is expected to exit when its coroutine
> terminates.  It is not possible to reuse threads from a pool.
> 
> This patch automatically disables the pool when 'gthread' is used.  This
> allows the 'gthread' backend to work again (for example,
> tests/test-coroutine completes successfully instead of hanging).
> 
> I considered implementing thread reuse but I don't want quirks like CPU
> affinity differences due to coroutine threads being recycled.  The
> 'gthread' backend is a reference backend and it's therefore okay to skip
> the pool optimization.
> 
> Note this patch also makes it easy to toggle the pool for benchmarking
> purposes:
> 
>   ./configure --with-coroutine-backend=ucontext \
>               --disable-coroutine-pool
> 
> Reported-by: Gabriel Kerneis <gabriel@kerneis.info>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  configure        | 24 ++++++++++++++++++++++++
>  qemu-coroutine.c | 34 +++++++++++++++++++---------------
>  2 files changed, 43 insertions(+), 15 deletions(-)
> 
> diff --git a/configure b/configure
> index e989609..5cf6341 100755
> --- a/configure
> +++ b/configure
> @@ -235,6 +235,7 @@ guest_agent=""
>  want_tools="yes"
>  libiscsi=""
>  coroutine=""
> +coroutine_pool=""
>  seccomp=""
>  glusterfs=""
>  glusterfs_discard="no"
> @@ -875,6 +876,10 @@ for opt do
>    ;;
>    --with-coroutine=*) coroutine="$optarg"
>    ;;
> +  --disable-coroutine-pool) coroutine_pool="no"
> +  ;;
> +  --enable-coroutine-pool) coroutine_pool="yes"
> +  ;;
>    --disable-docs) docs="no"
>    ;;
>    --enable-docs) docs="yes"
> @@ -1161,6 +1166,8 @@ echo "  --disable-seccomp        disable seccomp support"
>  echo "  --enable-seccomp         enables seccomp support"
>  echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
>  echo "                           gthread, ucontext, sigaltstack, windows"
> +echo "  --disable-coroutine-pool disable coroutine freelist (worse performance)"
> +echo "  --enable-coroutine-pool  enable coroutine freelist (better performance)"
>  echo "  --enable-glusterfs       enable GlusterFS backend"
>  echo "  --disable-glusterfs      disable GlusterFS backend"
>  echo "  --enable-gcov            enable test coverage analysis with gcov"
> @@ -3279,6 +3286,17 @@ else
>    esac
>  fi
>  
> +if test "$coroutine_pool" = ""; then
> +  if test "$coroutine" = "gthread"; then
> +    coroutine_pool=no
> +  else
> +    coroutine_pool=yes
> +  fi
> +fi
> +if test "$coroutine" = "gthread" -a "$coroutine_pool" = "yes"; then
> +  error_exit "'gthread' coroutine backend does not support pool (use --disable-coroutine-pool)"
> +fi
> +
>  ##########################################
>  # check if we have open_by_handle_at
>  
> @@ -3644,6 +3662,7 @@ echo "libiscsi support  $libiscsi"
>  echo "build guest agent $guest_agent"
>  echo "seccomp support   $seccomp"
>  echo "coroutine backend $coroutine"
> +echo "coroutine pool    $coroutine_pool"
>  echo "GlusterFS support $glusterfs"
>  echo "virtio-blk-data-plane $virtio_blk_data_plane"
>  echo "gcov              $gcov_tool"
> @@ -3999,6 +4018,11 @@ if test "$rbd" = "yes" ; then
>  fi
>  
>  echo "CONFIG_COROUTINE_BACKEND=$coroutine" >> $config_host_mak
> +if test "$coroutine_pool" = "yes" ; then
> +  echo "CONFIG_COROUTINE_POOL=1" >> $config_host_mak
> +else
> +  echo "CONFIG_COROUTINE_POOL=0" >> $config_host_mak
> +fi

Doesn't work.

In config-host.make we do get:

    CONFIG_COROUTINE_POOL=0

But when config-host.h is generated from it, I assume it's only checked
if the variable is defined, so we end up with:

    #define CONFIG_COROUTINE_POOL 1

That is, you never really disable the pool.

Kevin