Patchwork [V2,for-1.6,1/5] block: Repair the throttling code.

login
register
mail settings
Submitter Benoît Canet
Date July 23, 2013, 12:21 p.m.
Message ID <1374582067-9063-2-git-send-email-benoit@irqsave.net>
Download mbox | patch
Permalink /patch/261057/
State New
Headers show

Comments

Benoît Canet - July 23, 2013, 12:21 p.m.
The throttling code was segfaulting since commit
02ffb504485f0920cfc75a0982a602f824a9a4f4 because some qemu_co_queue_next caller
does not run in a coroutine.
qemu_co_queue_do_restart assume that the caller is a coroutinne.
As sugested by Stefan fix this by entering the coroutine directly.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block.c                   |    8 ++++----
 include/block/coroutine.h |    5 +++++
 qemu-coroutine-lock.c     |   14 ++++++++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)
Stefan Hajnoczi - July 23, 2013, 2:31 p.m.
On Tue, Jul 23, 2013 at 02:21:03PM +0200, Benoît Canet wrote:
> The throttling code was segfaulting since commit
> 02ffb504485f0920cfc75a0982a602f824a9a4f4 because some qemu_co_queue_next caller
> does not run in a coroutine.
> qemu_co_queue_do_restart assume that the caller is a coroutinne.
> As sugested by Stefan fix this by entering the coroutine directly.

Please mark qemu_co_queue_next() and qemu_co_queue_restart_all()
coroutine_fn.  They may only be called from coroutine context.

Also please add assert(qemu_in_coroutine()) to these functions so an
assertion fires when they are used incorrectly.

Patch

diff --git a/block.c b/block.c
index b560241..dc72643 100644
--- a/block.c
+++ b/block.c
@@ -127,7 +127,8 @@  void bdrv_io_limits_disable(BlockDriverState *bs)
 {
     bs->io_limits_enabled = false;
 
-    while (qemu_co_queue_next(&bs->throttled_reqs));
+    while (qemu_co_enter_next(&bs->throttled_reqs)) {
+    }
 
     if (bs->block_timer) {
         qemu_del_timer(bs->block_timer);
@@ -143,7 +144,7 @@  static void bdrv_block_timer(void *opaque)
 {
     BlockDriverState *bs = opaque;
 
-    qemu_co_queue_next(&bs->throttled_reqs);
+    qemu_co_enter_next(&bs->throttled_reqs);
 }
 
 void bdrv_io_limits_enable(BlockDriverState *bs)
@@ -1445,8 +1446,7 @@  void bdrv_drain_all(void)
          * a busy wait.
          */
         QTAILQ_FOREACH(bs, &bdrv_states, list) {
-            if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
-                qemu_co_queue_restart_all(&bs->throttled_reqs);
+            while (qemu_co_enter_next(&bs->throttled_reqs)) {
                 busy = true;
             }
         }
diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index 377805a..66e331c 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -138,6 +138,11 @@  bool qemu_co_queue_next(CoQueue *queue);
 void qemu_co_queue_restart_all(CoQueue *queue);
 
 /**
+ * Enter the next coroutine in the queue
+ */
+bool qemu_co_enter_next(CoQueue *queue);
+
+/**
  * Checks if the CoQueue is empty.
  */
 bool qemu_co_queue_empty(CoQueue *queue);
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index d9fea49..c61d300 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -98,6 +98,20 @@  void qemu_co_queue_restart_all(CoQueue *queue)
     qemu_co_queue_do_restart(queue, false);
 }
 
+bool qemu_co_enter_next(CoQueue *queue)
+{
+    Coroutine *next;
+
+    next = QTAILQ_FIRST(&queue->entries);
+    if (!next) {
+        return false;
+    }
+
+    QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
+    qemu_coroutine_enter(next, NULL);
+    return true;
+}
+
 bool qemu_co_queue_empty(CoQueue *queue)
 {
     return (QTAILQ_FIRST(&queue->entries) == NULL);