9ae3a8
From 0bf037188149824a8ae161ad9dea7d650b6f7346 Mon Sep 17 00:00:00 2001
9ae3a8
From: Gerd Hoffmann <kraxel@redhat.com>
9ae3a8
Date: Tue, 21 Jan 2014 11:25:16 -0500
9ae3a8
Subject: [PATCH 5/6] QEMUBH: make AioContext's bh re-entrant
9ae3a8
9ae3a8
Message-id: <1390303517-20167-2-git-send-email-kraxel@redhat.com>
9ae3a8
Patchwork-id: 56866
9ae3a8
O-Subject: [RHEL-7 qemu-kvm PATCH 1/2] QEMUBH: make AioContext's bh re-entrant
9ae3a8
Bugzilla: 1009297
9ae3a8
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
9ae3a8
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
9ae3a8
9ae3a8
From: Liu Ping Fan <qemulist@gmail.com>
9ae3a8
9ae3a8
BH will be used outside big lock, so introduce lock to protect
9ae3a8
between the writers, ie, bh's adders and deleter. The lock only
9ae3a8
affects the writers and bh's callback does not take this extra lock.
9ae3a8
Note that for the same AioContext, aio_bh_poll() can not run in
9ae3a8
parallel yet.
9ae3a8
9ae3a8
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
9ae3a8
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
(cherry picked from commit dcc772e2f2b7c2a68644133fea2b874f6751a57b)
9ae3a8
---
9ae3a8
 async.c             | 33 +++++++++++++++++++++++++++++++--
9ae3a8
 include/block/aio.h |  7 +++++++
9ae3a8
 2 files changed, 38 insertions(+), 2 deletions(-)
9ae3a8
9ae3a8
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
9ae3a8
---
9ae3a8
 async.c             | 33 +++++++++++++++++++++++++++++++--
9ae3a8
 include/block/aio.h |  7 +++++++
9ae3a8
 2 files changed, 38 insertions(+), 2 deletions(-)
9ae3a8
9ae3a8
diff --git a/async.c b/async.c
9ae3a8
index 90fe906..5ce3633 100644
9ae3a8
--- a/async.c
9ae3a8
+++ b/async.c
9ae3a8
@@ -47,11 +47,16 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
9ae3a8
     bh->ctx = ctx;
9ae3a8
     bh->cb = cb;
9ae3a8
     bh->opaque = opaque;
9ae3a8
+    qemu_mutex_lock(&ctx->bh_lock);
9ae3a8
     bh->next = ctx->first_bh;
9ae3a8
+    /* Make sure that the members are ready before putting bh into list */
9ae3a8
+    smp_wmb();
9ae3a8
     ctx->first_bh = bh;
9ae3a8
+    qemu_mutex_unlock(&ctx->bh_lock);
9ae3a8
     return bh;
9ae3a8
 }
9ae3a8
 
9ae3a8
+/* Multiple occurrences of aio_bh_poll cannot be called concurrently */
9ae3a8
 int aio_bh_poll(AioContext *ctx)
9ae3a8
 {
9ae3a8
     QEMUBH *bh, **bhp, *next;
9ae3a8
@@ -61,9 +66,15 @@ int aio_bh_poll(AioContext *ctx)
9ae3a8
 
9ae3a8
     ret = 0;
9ae3a8
     for (bh = ctx->first_bh; bh; bh = next) {
9ae3a8
+        /* Make sure that fetching bh happens before accessing its members */
9ae3a8
+        smp_read_barrier_depends();
9ae3a8
         next = bh->next;
9ae3a8
         if (!bh->deleted && bh->scheduled) {
9ae3a8
             bh->scheduled = 0;
9ae3a8
+            /* Paired with write barrier in bh schedule to ensure reading for
9ae3a8
+             * idle & callbacks coming after bh's scheduling.
9ae3a8
+             */
9ae3a8
+            smp_rmb();
9ae3a8
             if (!bh->idle)
9ae3a8
                 ret = 1;
9ae3a8
             bh->idle = 0;
9ae3a8
@@ -75,6 +86,7 @@ int aio_bh_poll(AioContext *ctx)
9ae3a8
 
9ae3a8
     /* remove deleted bhs */
9ae3a8
     if (!ctx->walking_bh) {
9ae3a8
+        qemu_mutex_lock(&ctx->bh_lock);
9ae3a8
         bhp = &ctx->first_bh;
9ae3a8
         while (*bhp) {
9ae3a8
             bh = *bhp;
9ae3a8
@@ -85,6 +97,7 @@ int aio_bh_poll(AioContext *ctx)
9ae3a8
                 bhp = &bh->next;
9ae3a8
             }
9ae3a8
         }
9ae3a8
+        qemu_mutex_unlock(&ctx->bh_lock);
9ae3a8
     }
9ae3a8
 
9ae3a8
     return ret;
9ae3a8
@@ -94,24 +107,38 @@ void qemu_bh_schedule_idle(QEMUBH *bh)
9ae3a8
 {
9ae3a8
     if (bh->scheduled)
9ae3a8
         return;
9ae3a8
-    bh->scheduled = 1;
9ae3a8
     bh->idle = 1;
9ae3a8
+    /* Make sure that idle & any writes needed by the callback are done
9ae3a8
+     * before the locations are read in the aio_bh_poll.
9ae3a8
+     */
9ae3a8
+    smp_wmb();
9ae3a8
+    bh->scheduled = 1;
9ae3a8
 }
9ae3a8
 
9ae3a8
 void qemu_bh_schedule(QEMUBH *bh)
9ae3a8
 {
9ae3a8
     if (bh->scheduled)
9ae3a8
         return;
9ae3a8
-    bh->scheduled = 1;
9ae3a8
     bh->idle = 0;
9ae3a8
+    /* Make sure that idle & any writes needed by the callback are done
9ae3a8
+     * before the locations are read in the aio_bh_poll.
9ae3a8
+     */
9ae3a8
+    smp_wmb();
9ae3a8
+    bh->scheduled = 1;
9ae3a8
     aio_notify(bh->ctx);
9ae3a8
 }
9ae3a8
 
9ae3a8
+
9ae3a8
+/* This func is async.
9ae3a8
+ */
9ae3a8
 void qemu_bh_cancel(QEMUBH *bh)
9ae3a8
 {
9ae3a8
     bh->scheduled = 0;
9ae3a8
 }
9ae3a8
 
9ae3a8
+/* This func is async.The bottom half will do the delete action at the finial
9ae3a8
+ * end.
9ae3a8
+ */
9ae3a8
 void qemu_bh_delete(QEMUBH *bh)
9ae3a8
 {
9ae3a8
     bh->scheduled = 0;
9ae3a8
@@ -176,6 +203,7 @@ aio_ctx_finalize(GSource     *source)
9ae3a8
     thread_pool_free(ctx->thread_pool);
9ae3a8
     aio_set_event_notifier(ctx, &ctx->notifier, NULL, NULL);
9ae3a8
     event_notifier_cleanup(&ctx->notifier);
9ae3a8
+    qemu_mutex_destroy(&ctx->bh_lock);
9ae3a8
     g_array_free(ctx->pollfds, TRUE);
9ae3a8
 }
9ae3a8
 
9ae3a8
@@ -211,6 +239,7 @@ AioContext *aio_context_new(void)
9ae3a8
     ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
9ae3a8
     ctx->pollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
9ae3a8
     ctx->thread_pool = NULL;
9ae3a8
+    qemu_mutex_init(&ctx->bh_lock);
9ae3a8
     event_notifier_init(&ctx->notifier, false);
9ae3a8
     aio_set_event_notifier(ctx, &ctx->notifier, 
9ae3a8
                            (EventNotifierHandler *)
9ae3a8
diff --git a/include/block/aio.h b/include/block/aio.h
9ae3a8
index 1836793..cc77771 100644
9ae3a8
--- a/include/block/aio.h
9ae3a8
+++ b/include/block/aio.h
9ae3a8
@@ -17,6 +17,7 @@
9ae3a8
 #include "qemu-common.h"
9ae3a8
 #include "qemu/queue.h"
9ae3a8
 #include "qemu/event_notifier.h"
9ae3a8
+#include "qemu/thread.h"
9ae3a8
 
9ae3a8
 typedef struct BlockDriverAIOCB BlockDriverAIOCB;
9ae3a8
 typedef void BlockDriverCompletionFunc(void *opaque, int ret);
9ae3a8
@@ -53,6 +54,8 @@ typedef struct AioContext {
9ae3a8
      */
9ae3a8
     int walking_handlers;
9ae3a8
 
9ae3a8
+    /* lock to protect between bh's adders and deleter */
9ae3a8
+    QemuMutex bh_lock;
9ae3a8
     /* Anchor of the list of Bottom Halves belonging to the context */
9ae3a8
     struct QEMUBH *first_bh;
9ae3a8
 
9ae3a8
@@ -127,6 +130,8 @@ void aio_notify(AioContext *ctx);
9ae3a8
  * aio_bh_poll: Poll bottom halves for an AioContext.
9ae3a8
  *
9ae3a8
  * These are internal functions used by the QEMU main loop.
9ae3a8
+ * And notice that multiple occurrences of aio_bh_poll cannot
9ae3a8
+ * be called concurrently
9ae3a8
  */
9ae3a8
 int aio_bh_poll(AioContext *ctx);
9ae3a8
 
9ae3a8
@@ -163,6 +168,8 @@ void qemu_bh_cancel(QEMUBH *bh);
9ae3a8
  * Deleting a bottom half frees the memory that was allocated for it by
9ae3a8
  * qemu_bh_new.  It also implies canceling the bottom half if it was
9ae3a8
  * scheduled.
9ae3a8
+ * This func is async. The bottom half will do the delete action at the finial
9ae3a8
+ * end.
9ae3a8
  *
9ae3a8
  * @bh: The bottom half to be deleted.
9ae3a8
  */
9ae3a8
-- 
9ae3a8
1.8.3.1
9ae3a8