From 5b3290d65d3e21aebed16fd352ea974fb6404e93 Mon Sep 17 00:00:00 2001
From: Avi Kivity <avi@redhat.com>
Date: Sun, 9 Oct 2011 15:54:10 +0200
Subject: [PATCH] posix-aio-compat: fix latency issues
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

RH-Author: Avi Kivity <avi@redhat.com>
Message-id: <1318175650-16683-1-git-send-email-avi@redhat.com>
Patchwork-id: 33946
O-Subject: [PATCH RHEL5] posix-aio-compat: fix latency issues
Bugzilla: 725629
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
RH-Acked-by: Markus Armbruster <armbru@redhat.com>

Bugzilla: 725629
Upstream: e4ea78ee76a

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 <avi@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu/posix-aio-compat.c |   47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 45 insertions(+), 2 deletions(-)

Signed-off-by: Michal Novotny <mignov@gmail.com>
---
 qemu/posix-aio-compat.c |   47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/qemu/posix-aio-compat.c b/qemu/posix-aio-compat.c
index d361c6a..aa3c71d 100644
--- a/qemu/posix-aio-compat.c
+++ b/qemu/posix-aio-compat.c
@@ -19,15 +19,21 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include "osdep.h"
+#include "qemu-common.h"
 
 #include "posix-aio-compat.h"
 
+static void do_spawn_thread(void);
+
 static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
 static pthread_t thread_id;
 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 TAILQ_HEAD(, qemu_paiocb) request_list;
 
 #define QEMU_AIO_READ         0x0001
@@ -86,6 +92,11 @@ static void *aio_thread(void *unused)
     if (sigfillset(&set)) die("sigfillset");
     if (sigprocmask(SIG_BLOCK, &set, NULL)) die("sigprocmask");
 
+    mutex_lock(&lock);
+    pending_threads--;
+    mutex_unlock(&lock);
+    do_spawn_thread();
+
     while (1) {
         struct qemu_paiocb *aiocb;
         size_t offset;
@@ -166,12 +177,22 @@ static void *aio_thread(void *unused)
     return NULL;
 }
 
-static void spawn_thread(void)
+static void do_spawn_thread(void)
 {
     int ret;
     pthread_attr_t attr;
 
-    cur_threads++;
+    mutex_lock(&lock);
+    if (!new_threads) {
+        mutex_unlock(&lock);
+        return;
+    }
+
+    new_threads--;
+    pending_threads++;
+
+    mutex_unlock(&lock);
+
     idle_threads++;
 
     ret = pthread_attr_init(&attr);
@@ -183,9 +204,31 @@ static void spawn_thread(void)
     if (ret) die2 (ret, "pthread_attr_destroy");
 }
 
+static void spawn_thread_bh_fn(void *opaque)
+{
+    do_spawn_thread();
+}
+
+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);
+    }
+}
+
 int qemu_paio_init(struct qemu_paioinit *aioinit)
 {
     TAILQ_INIT(&request_list);
+    new_thread_bh = qemu_bh_new(spawn_thread_bh_fn, NULL);
 
     return 0;
 }
-- 
1.7.4.4