From patchwork Mon Aug 8 11:37:38 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Avi Kivity X-Patchwork-Id: 108893 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 45997B6F57 for ; Mon, 8 Aug 2011 21:37:54 +1000 (EST) Received: from localhost ([::1]:38242 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QqO9W-0001YF-2E for incoming@patchwork.ozlabs.org; Mon, 08 Aug 2011 07:37:50 -0400 Received: from eggs.gnu.org ([140.186.70.92]:41183) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QqO9Q-0001Xy-6x for qemu-devel@nongnu.org; Mon, 08 Aug 2011 07:37:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QqO9P-0008IA-64 for qemu-devel@nongnu.org; Mon, 08 Aug 2011 07:37:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5366) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QqO9O-0008Hr-VC for qemu-devel@nongnu.org; Mon, 08 Aug 2011 07:37:43 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p78BbfsA009284 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 8 Aug 2011 07:37:41 -0400 Received: from cleopatra.tlv.redhat.com (cleopatra.tlv.redhat.com [10.35.255.11]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p78Bbe4L026622; Mon, 8 Aug 2011 07:37:41 -0400 Received: from s01.tlv.redhat.com (s01.tlv.redhat.com [10.35.255.8]) by cleopatra.tlv.redhat.com (Postfix) with ESMTP id 58389250B3E; Mon, 8 Aug 2011 14:37:40 +0300 (IDT) From: Avi Kivity To: qemu-devel@nongnu.org Date: Mon, 8 Aug 2011 14:37:38 +0300 Message-Id: <1312803458-2272-1-git-send-email-avi@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-MIME-Autoconverted: from 8bit to quoted-printable by mx1.redhat.com id p78BbfsA009284 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: kvm@vger.kernel.org Subject: [Qemu-devel] [PATCH] posix-aio-compat: fix latency issues 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 In certain circumstances, posix-aio-compat can incur a lot of latency: - threads are created by vcpu threads, so if vcpu affinity is set, aio threads inherit vcpu affinity. This can cause many aio threads to compete for one cpu. - we can create up to max_threads (64) aio threads in one go; since a pthread_create can take around 30μs, we have up to 2ms of cpu time under a global lock. Fix by: - moving thread creation to the main thread, so we inherit the main thread's affinity instead of the vcpu thread's affinity. - if a thread is currently being created, and we need to create yet another thread, let thread being born create the new thread, reducing the amount of time we spend under the main thread. - drop the local lock while creating a thread (we may still hold the global mutex, though) Note this doesn't eliminate latency completely; scheduler artifacts or lack of host cpu resources can still cause it. We may want pre-allocated threads when this cannot be tolerated. Thanks to Uli Obergfell of Red Hat for his excellent analysis and suggestions. Signed-off-by: Avi Kivity --- posix-aio-compat.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 46 insertions(+), 2 deletions(-) diff --git a/posix-aio-compat.c b/posix-aio-compat.c index 8dc00cb..aa30673 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -30,6 +30,7 @@ #include "block/raw-posix-aio.h" +static void do_spawn_thread(void); struct qemu_paiocb { BlockDriverAIOCB common; @@ -64,6 +65,9 @@ static pthread_attr_t attr; static int max_threads = 64; static int cur_threads = 0; static int idle_threads = 0; +static int new_threads = 0; /* backlog of threads we need to create */ +static int pending_threads = 0; /* threads created but not running yet */ +static QEMUBH *new_thread_bh; static QTAILQ_HEAD(, qemu_paiocb) request_list; #ifdef CONFIG_PREADV @@ -311,6 +315,13 @@ static void *aio_thread(void *unused) pid = getpid(); + mutex_lock(&lock); + if (new_threads) { + do_spawn_thread(); + } + pending_threads--; + mutex_unlock(&lock); + while (1) { struct qemu_paiocb *aiocb; ssize_t ret = 0; @@ -381,11 +392,18 @@ static void *aio_thread(void *unused) return NULL; } -static void spawn_thread(void) +static void do_spawn_thread(void) { sigset_t set, oldset; - cur_threads++; + if (!new_threads) { + return; + } + + new_threads--; + pending_threads++; + + mutex_unlock(&lock); /* block all signals */ if (sigfillset(&set)) die("sigfillset"); @@ -394,6 +412,31 @@ static void spawn_thread(void) thread_create(&thread_id, &attr, aio_thread, NULL); if (sigprocmask(SIG_SETMASK, &oldset, NULL)) die("sigprocmask restore"); + + mutex_lock(&lock); +} + +static void spawn_thread_bh_fn(void *opaque) +{ + mutex_lock(&lock); + do_spawn_thread(); + mutex_unlock(&lock); +} + +static void spawn_thread(void) +{ + cur_threads++; + new_threads++; + /* If there are threads being created, they will spawn new workers, so + * we don't spend time creating many threads in a loop holding a mutex or + * starving the current vcpu. + * + * If there are no idle threads, ask the main thread to create one, so we + * inherit the correct affinity instead of the vcpu affinity. + */ + if (!pending_threads) { + qemu_bh_schedule(new_thread_bh); + } } static void qemu_paio_submit(struct qemu_paiocb *aiocb) @@ -665,6 +708,7 @@ int paio_init(void) die2(ret, "pthread_attr_setdetachstate"); QTAILQ_INIT(&request_list); + new_thread_bh = qemu_bh_new(spawn_thread_bh_fn, NULL); posix_aio_state = s; return 0;