From patchwork Wed Sep 11 14:42:35 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Hajnoczi X-Patchwork-Id: 274299 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 55D092C0202 for ; Thu, 12 Sep 2013 00:43:41 +1000 (EST) Received: from localhost ([::1]:36318 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJlds-0003ol-1c for incoming@patchwork.ozlabs.org; Wed, 11 Sep 2013 10:43:40 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35929) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJldA-0003cL-HP for qemu-devel@nongnu.org; Wed, 11 Sep 2013 10:43:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VJld5-0007H4-L6 for qemu-devel@nongnu.org; Wed, 11 Sep 2013 10:42:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:25399) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJld5-0007Gr-CU for qemu-devel@nongnu.org; Wed, 11 Sep 2013 10:42:51 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r8BEgliI015742 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 11 Sep 2013 10:42:47 -0400 Received: from localhost (dhcp-64-106.muc.redhat.com [10.32.64.106]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r8BEgkQa012303; Wed, 11 Sep 2013 10:42:46 -0400 From: Stefan Hajnoczi To: Date: Wed, 11 Sep 2013 16:42:35 +0200 Message-Id: <1378910555-24753-1-git-send-email-stefanha@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Kevin Wolf , gabriel@kerneis.info, Stefan Hajnoczi Subject: [Qemu-devel] [PATCH] coroutine: add ./configure --disable-coroutine-pool X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 Signed-off-by: Stefan Hajnoczi Reviewed-by: Gabriel Kerneis --- 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 if test "$open_by_handle_at" = "yes" ; then echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak diff --git a/qemu-coroutine.c b/qemu-coroutine.c index 423430d..4708521 100644 --- a/qemu-coroutine.c +++ b/qemu-coroutine.c @@ -30,15 +30,17 @@ static unsigned int pool_size; Coroutine *qemu_coroutine_create(CoroutineEntry *entry) { - Coroutine *co; - - qemu_mutex_lock(&pool_lock); - co = QSLIST_FIRST(&pool); - if (co) { - QSLIST_REMOVE_HEAD(&pool, pool_next); - pool_size--; + Coroutine *co = NULL; + + if (CONFIG_COROUTINE_POOL) { + qemu_mutex_lock(&pool_lock); + co = QSLIST_FIRST(&pool); + if (co) { + QSLIST_REMOVE_HEAD(&pool, pool_next); + pool_size--; + } + qemu_mutex_unlock(&pool_lock); } - qemu_mutex_unlock(&pool_lock); if (!co) { co = qemu_coroutine_new(); @@ -51,15 +53,17 @@ Coroutine *qemu_coroutine_create(CoroutineEntry *entry) static void coroutine_delete(Coroutine *co) { - qemu_mutex_lock(&pool_lock); - if (pool_size < POOL_MAX_SIZE) { - QSLIST_INSERT_HEAD(&pool, co, pool_next); - co->caller = NULL; - pool_size++; + if (CONFIG_COROUTINE_POOL) { + qemu_mutex_lock(&pool_lock); + if (pool_size < POOL_MAX_SIZE) { + QSLIST_INSERT_HEAD(&pool, co, pool_next); + co->caller = NULL; + pool_size++; + qemu_mutex_unlock(&pool_lock); + return; + } qemu_mutex_unlock(&pool_lock); - return; } - qemu_mutex_unlock(&pool_lock); qemu_coroutine_delete(co); }