[Qemu-devel] [PATCH v3 0/7] bdrv_flush_io_queue removal, shared LinuxAioState

[Qemu-devel] [PATCH v3 0/7] bdrv_flush_io_queue removal, shared LinuxAioState

[Paolo Bonzini]

Patch 1 comes from Kevin's series to do BlockBackend throttling.

Patches 2-5 are from my bdrv_drain patches.

Patches 6-7 are new but based on Ming Lei's old submission.
I'm including them here because they apply on top of patches 2-5.
It would be nice to have them too in 2.6.

Paolo

Kevin Wolf (1):
  block: Don't disable I/O throttling on sync requests

Paolo Bonzini (6):
  block: make bdrv_start_throttled_reqs return void
  block: move restarting of throttled reqs to block/throttle-groups.c
  block: introduce bdrv_no_throttling_begin/end
  block: plug whole tree at once, introduce bdrv_io_unplugged_begin/end
  linux-aio: make it more type safe
  linux-aio: share one LinuxAioState within an AioContext

 async.c                            |  23 +++++++
 block.c                            |   1 -
 block/block-backend.c              |   6 +-
 block/io.c                         | 128 +++++++++++++++++++----------------
 block/linux-aio.c                  |  60 +++++++----------
 block/raw-posix.c                  | 133 ++++---------------------------------
 block/raw-win32.c                  |   2 +-
 block/throttle-groups.c            |  20 ++++++
 include/block/aio.h                |  13 ++++
 include/block/block.h              |   3 +-
 include/block/block_int.h          |  14 ++--
 {block => include/block}/raw-aio.h |  15 +++--
 include/block/throttle-groups.h    |   1 +
 13 files changed, 189 insertions(+), 230 deletions(-)
 rename {block => include/block}/raw-aio.h (80%)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/7] block: Don't disable I/O throttling on sync requests

[Paolo Bonzini]

From: Kevin Wolf <kwolf@redhat.com>

We had to disable I/O throttling with synchronous requests because we
didn't use to run timers in nested event loops when the code was
introduced. This isn't true any more, and throttling works just fine
even when using the synchronous API.

The removed code is in fact dead code since commit a8823a3b ('block: Use
blk_co_pwritev() for blk_write()') because I/O throttling can only be
set on the top layer, but BlockBackend always uses the coroutine
interface now instead of using the sync API emulation in block.c.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <1458660792-3035-2-git-send-email-kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/io.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/block/io.c b/block/io.c
index 41d954ca..cbba269 100644
--- a/block/io.c
+++ b/block/io.c
@@ -561,17 +561,6 @@ static int bdrv_prwv_co(BlockDriverState *bs, int64_t offset,
         .flags = flags,
     };
 
-    /**
-     * In sync call context, when the vcpu is blocked, this throttling timer
-     * will not fire; so the I/O throttling function has to be disabled here
-     * if it has been enabled.
-     */
-    if (bs->io_limits_enabled) {
-        fprintf(stderr, "Disabling I/O throttling on '%s' due "
-                        "to synchronous I/O.\n", bdrv_get_device_name(bs));
-        bdrv_io_limits_disable(bs);
-    }
-
     if (qemu_in_coroutine()) {
         /* Fast-path if already in coroutine context */
         bdrv_rw_co_entry(&rwco);
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/7] block: make bdrv_start_throttled_reqs return void

[Paolo Bonzini]

The return value is unused and I am not sure why it would be useful.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/io.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/block/io.c b/block/io.c
index cbba269..3703426 100644
--- a/block/io.c
+++ b/block/io.c
@@ -69,10 +69,8 @@ void bdrv_set_io_limits(BlockDriverState *bs,
     }
 }
 
-/* this function drain all the throttled IOs */
-static bool bdrv_start_throttled_reqs(BlockDriverState *bs)
+static void bdrv_start_throttled_reqs(BlockDriverState *bs)
 {
-    bool drained = false;
     bool enabled = bs->io_limits_enabled;
     int i;
 
@@ -80,13 +78,11 @@ static bool bdrv_start_throttled_reqs(BlockDriverState *bs)
 
     for (i = 0; i < 2; i++) {
         while (qemu_co_enter_next(&bs->throttled_reqs[i])) {
-            drained = true;
+            ;
         }
     }
 
     bs->io_limits_enabled = enabled;
-
-    return drained;
 }
 
 void bdrv_io_limits_disable(BlockDriverState *bs)
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/7] block: move restarting of throttled reqs to block/throttle-groups.c

[Paolo Bonzini]

We want to remove throttled_reqs from block/io.c.  This is the easy
part---hide the handling of throttled_reqs during disable/enable of
throttling within throttle-groups.c.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        v1->v2: only restart first request after throttle_group_config
---
 block/io.c                      | 15 +--------------
 block/throttle-groups.c         | 16 ++++++++++++++++
 include/block/throttle-groups.h |  1 +
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/block/io.c b/block/io.c
index 3703426..93696df 100644
--- a/block/io.c
+++ b/block/io.c
@@ -60,28 +60,15 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
 void bdrv_set_io_limits(BlockDriverState *bs,
                         ThrottleConfig *cfg)
 {
-    int i;
-
     throttle_group_config(bs, cfg);
-
-    for (i = 0; i < 2; i++) {
-        qemu_co_enter_next(&bs->throttled_reqs[i]);
-    }
 }
 
 static void bdrv_start_throttled_reqs(BlockDriverState *bs)
 {
     bool enabled = bs->io_limits_enabled;
-    int i;
 
     bs->io_limits_enabled = false;
-
-    for (i = 0; i < 2; i++) {
-        while (qemu_co_enter_next(&bs->throttled_reqs[i])) {
-            ;
-        }
-    }
-
+    throttle_group_restart_bs(bs);
     bs->io_limits_enabled = enabled;
 }
 
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 4920e09..9f52d2b 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -313,6 +313,17 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
     qemu_mutex_unlock(&tg->lock);
 }
 
+void throttle_group_restart_bs(BlockDriverState *bs)
+{
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        while (qemu_co_enter_next(&bs->throttled_reqs[i])) {
+            ;
+        }
+    }
+}
+
 /* Update the throttle configuration for a particular group. Similar
  * to throttle_config(), but guarantees atomicity within the
  * throttling group.
@@ -335,6 +346,11 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
     }
     throttle_config(ts, tt, cfg);
     qemu_mutex_unlock(&tg->lock);
+
+    aio_context_acquire(bdrv_get_aio_context(bs));
+    qemu_co_enter_next(&bs->throttled_reqs[0]);
+    qemu_co_enter_next(&bs->throttled_reqs[1]);
+    aio_context_release(bdrv_get_aio_context(bs));
 }
 
 /* Get the throttle configuration from a particular group. Similar to
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index aba28f3..395f72d 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -38,6 +38,7 @@ void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg);
 
 void throttle_group_register_bs(BlockDriverState *bs, const char *groupname);
 void throttle_group_unregister_bs(BlockDriverState *bs);
+void throttle_group_restart_bs(BlockDriverState *bs);
 
 void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
                                                         unsigned int bytes,
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/7] block: introduce bdrv_no_throttling_begin/end

[Paolo Bonzini]

Extract the handling of throttling from bdrv_flush_io_queue.  These
new functions will soon become BdrvChildRole callbacks, as they can
be generalized to "beginning of drain" and "end of drain".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	v2->v3: rebase, drop recursion

 block.c                   |  1 -
 block/block-backend.c     |  6 ++----
 block/io.c                | 30 ++++++++++++++++++------------
 block/throttle-groups.c   |  4 ++++
 include/block/block_int.h |  9 ++++++---
 5 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/block.c b/block.c
index b4107fc..9cc0eb4 100644
--- a/block.c
+++ b/block.c
@@ -2300,7 +2300,6 @@ static void swap_feature_fields(BlockDriverState *bs_top,
 
     assert(!bs_new->throttle_state);
     if (bs_top->throttle_state) {
-        assert(bs_top->io_limits_enabled);
         bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top));
         bdrv_io_limits_disable(bs_top);
     }
diff --git a/block/block-backend.c b/block/block-backend.c
index dca21d1..51e9e49 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -800,7 +800,6 @@ int blk_read_unthrottled(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
                          int nb_sectors)
 {
     BlockDriverState *bs = blk_bs(blk);
-    bool enabled;
     int ret;
 
     ret = blk_check_request(blk, sector_num, nb_sectors);
@@ -808,10 +807,9 @@ int blk_read_unthrottled(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
         return ret;
     }
 
-    enabled = bs->io_limits_enabled;
-    bs->io_limits_enabled = false;
+    bdrv_no_throttling_begin(bs);
     ret = blk_read(blk, sector_num, buf, nb_sectors);
-    bs->io_limits_enabled = enabled;
+    bdrv_no_throttling_end(bs);
     return ret;
 }
 
diff --git a/block/io.c b/block/io.c
index 93696df..516cfb9 100644
--- a/block/io.c
+++ b/block/io.c
@@ -63,28 +63,31 @@ void bdrv_set_io_limits(BlockDriverState *bs,
     throttle_group_config(bs, cfg);
 }
 
-static void bdrv_start_throttled_reqs(BlockDriverState *bs)
+void bdrv_no_throttling_begin(BlockDriverState *bs)
 {
-    bool enabled = bs->io_limits_enabled;
+    if (bs->io_limits_disabled++ == 0) {
+        throttle_group_restart_bs(bs);
+    }
+}
 
-    bs->io_limits_enabled = false;
-    throttle_group_restart_bs(bs);
-    bs->io_limits_enabled = enabled;
+void bdrv_no_throttling_end(BlockDriverState *bs)
+{
+    --bs->io_limits_disabled;
 }
 
 void bdrv_io_limits_disable(BlockDriverState *bs)
 {
-    bs->io_limits_enabled = false;
-    bdrv_start_throttled_reqs(bs);
+    assert(bs->throttle_state);
+    bdrv_no_throttling_begin(bs);
     throttle_group_unregister_bs(bs);
+    bdrv_no_throttling_end(bs);
 }
 
 /* should be called before bdrv_set_io_limits if a limit is set */
 void bdrv_io_limits_enable(BlockDriverState *bs, const char *group)
 {
-    assert(!bs->io_limits_enabled);
+    assert(!bs->throttle_state);
     throttle_group_register_bs(bs, group);
-    bs->io_limits_enabled = true;
 }
 
 void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
@@ -249,6 +252,7 @@ void bdrv_drain(BlockDriverState *bs)
 {
     bool busy = true;
 
+    bdrv_no_throttling_begin(bs);
     bdrv_drain_recurse(bs);
     while (busy) {
         /* Keep iterating */
@@ -256,6 +260,7 @@ void bdrv_drain(BlockDriverState *bs)
          busy = bdrv_requests_pending(bs);
          busy |= aio_poll(bdrv_get_aio_context(bs), busy);
     }
+    bdrv_no_throttling_end(bs);
 }
 
 /*
@@ -278,6 +283,7 @@ void bdrv_drain_all(void)
         if (bs->job) {
             block_job_pause(bs->job);
         }
+        bdrv_no_throttling_begin(bs);
         bdrv_drain_recurse(bs);
         aio_context_release(aio_context);
 
@@ -319,6 +325,7 @@ void bdrv_drain_all(void)
         AioContext *aio_context = bdrv_get_aio_context(bs);
 
         aio_context_acquire(aio_context);
+        bdrv_no_throttling_end(bs);
         if (bs->job) {
             block_job_resume(bs->job);
         }
@@ -921,7 +928,7 @@ int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs,
     }
 
     /* throttling disk I/O */
-    if (bs->io_limits_enabled) {
+    if (bs->throttle_state) {
         throttle_group_co_io_limits_intercept(bs, bytes, false);
     }
 
@@ -1263,7 +1270,7 @@ int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs,
     }
 
     /* throttling disk I/O */
-    if (bs->io_limits_enabled) {
+    if (bs->throttle_state) {
         throttle_group_co_io_limits_intercept(bs, bytes, true);
     }
 
@@ -2698,7 +2705,6 @@ void bdrv_flush_io_queue(BlockDriverState *bs)
     } else if (bs->file) {
         bdrv_flush_io_queue(bs->file->bs);
     }
-    bdrv_start_throttled_reqs(bs);
 }
 
 void bdrv_drained_begin(BlockDriverState *bs)
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 9f52d2b..b348886 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -219,6 +219,10 @@ static bool throttle_group_schedule_timer(BlockDriverState *bs,
     ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
     bool must_wait;
 
+    if (bs->io_limits_disabled) {
+        return false;
+    }
+
     /* Check if any of the timers in this group is already armed */
     if (tg->any_timer_armed[is_write]) {
         return true;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ba6e9ac..cfcbd96 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -412,10 +412,10 @@ struct BlockDriverState {
 
     /* I/O throttling.
      * throttle_state tells us if this BDS has I/O limits configured.
-     * io_limits_enabled tells us if they are currently being
-     * enforced, but it can be temporarily set to false */
+     * io_limits_disabled tells us if they are currently being enforced */
     CoQueue      throttled_reqs[2];
-    bool         io_limits_enabled;
+    unsigned int io_limits_disabled;
+
     /* The following fields are protected by the ThrottleGroup lock.
      * See the ThrottleGroup documentation for details. */
     ThrottleState *throttle_state;
@@ -722,4 +722,7 @@ void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in);
 
 void blockdev_close_all_bdrv_states(void);
 
+void bdrv_no_throttling_begin(BlockDriverState *bs);
+void bdrv_no_throttling_end(BlockDriverState *bs);
+
 #endif /* BLOCK_INT_H */
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/7] block: plug whole tree at once, introduce bdrv_io_unplugged_begin/end

[Paolo Bonzini]

Extract the handling of io_plug "depth" from linux-aio.c and let the
main bdrv_drain loop do nothing but wait on I/O.  bdrv_flush_io_queue
disappears.

Like the two newly introduced functions, bdrv_io_plug and bdrv_io_unplug
now operate on all children.  The visit order is now symmetrical between
plug and unplug, making it possible for formats to implement plug/unplug.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/io.c                | 72 +++++++++++++++++++++++++++++++++++------------
 block/linux-aio.c         | 13 ++++-----
 block/raw-aio.h           |  2 +-
 block/raw-posix.c         | 16 +----------
 include/block/block.h     |  3 +-
 include/block/block_int.h |  5 +++-
 6 files changed, 67 insertions(+), 44 deletions(-)

diff --git a/block/io.c b/block/io.c
index 516cfb9..2569e54 100644
--- a/block/io.c
+++ b/block/io.c
@@ -253,13 +253,14 @@ void bdrv_drain(BlockDriverState *bs)
     bool busy = true;
 
     bdrv_no_throttling_begin(bs);
+    bdrv_io_unplugged_begin(bs);
     bdrv_drain_recurse(bs);
     while (busy) {
         /* Keep iterating */
-         bdrv_flush_io_queue(bs);
          busy = bdrv_requests_pending(bs);
          busy |= aio_poll(bdrv_get_aio_context(bs), busy);
     }
+    bdrv_io_unplugged_end(bs);
     bdrv_no_throttling_end(bs);
 }
 
@@ -284,6 +285,7 @@ void bdrv_drain_all(void)
             block_job_pause(bs->job);
         }
         bdrv_no_throttling_begin(bs);
+        bdrv_io_unplugged_begin(bs);
         bdrv_drain_recurse(bs);
         aio_context_release(aio_context);
 
@@ -308,7 +310,6 @@ void bdrv_drain_all(void)
             aio_context_acquire(aio_context);
             while ((bs = bdrv_next(bs))) {
                 if (aio_context == bdrv_get_aio_context(bs)) {
-                    bdrv_flush_io_queue(bs);
                     if (bdrv_requests_pending(bs)) {
                         busy = true;
                         aio_poll(aio_context, busy);
@@ -325,6 +326,7 @@ void bdrv_drain_all(void)
         AioContext *aio_context = bdrv_get_aio_context(bs);
 
         aio_context_acquire(aio_context);
+        bdrv_io_unplugged_end(bs);
         bdrv_no_throttling_end(bs);
         if (bs->job) {
             block_job_resume(bs->job);
@@ -2679,31 +2681,65 @@ void bdrv_add_before_write_notifier(BlockDriverState *bs,
 
 void bdrv_io_plug(BlockDriverState *bs)
 {
-    BlockDriver *drv = bs->drv;
-    if (drv && drv->bdrv_io_plug) {
-        drv->bdrv_io_plug(bs);
-    } else if (bs->file) {
-        bdrv_io_plug(bs->file->bs);
+    BdrvChild *child;
+
+    if (bs->io_plugged++ == 0 && bs->io_plug_disabled == 0) {
+        BlockDriver *drv = bs->drv;
+        if (drv && drv->bdrv_io_plug) {
+            drv->bdrv_io_plug(bs);
+        }
+    }
+
+    QLIST_FOREACH(child, &bs->children, next) {
+        bdrv_io_plug(child->bs);
     }
 }
 
 void bdrv_io_unplug(BlockDriverState *bs)
 {
-    BlockDriver *drv = bs->drv;
-    if (drv && drv->bdrv_io_unplug) {
-        drv->bdrv_io_unplug(bs);
-    } else if (bs->file) {
-        bdrv_io_unplug(bs->file->bs);
+    BdrvChild *child;
+
+    QLIST_FOREACH(child, &bs->children, next) {
+        bdrv_io_unplug(child->bs);
+    }
+
+    if (--bs->io_plugged == 0 && bs->io_plug_disabled == 0) {
+        BlockDriver *drv = bs->drv;
+        if (drv && drv->bdrv_io_unplug) {
+            drv->bdrv_io_unplug(bs);
+        }
     }
 }
 
-void bdrv_flush_io_queue(BlockDriverState *bs)
+void bdrv_io_unplugged_begin(BlockDriverState *bs)
 {
-    BlockDriver *drv = bs->drv;
-    if (drv && drv->bdrv_flush_io_queue) {
-        drv->bdrv_flush_io_queue(bs);
-    } else if (bs->file) {
-        bdrv_flush_io_queue(bs->file->bs);
+    BdrvChild *child;
+
+    if (bs->io_plug_disabled++ == 0 && bs->io_plugged > 0) {
+        BlockDriver *drv = bs->drv;
+        if (drv && drv->bdrv_io_unplug) {
+            drv->bdrv_io_unplug(bs);
+        }
+    }
+
+    QLIST_FOREACH(child, &bs->children, next) {
+        bdrv_io_unplugged_begin(child->bs);
+    }
+}
+
+void bdrv_io_unplugged_end(BlockDriverState *bs)
+{
+    BdrvChild *child;
+
+    QLIST_FOREACH(child, &bs->children, next) {
+        bdrv_io_unplugged_end(child->bs);
+    }
+
+    if (--bs->io_plug_disabled == 0 && bs->io_plugged > 0) {
+        BlockDriver *drv = bs->drv;
+        if (drv && drv->bdrv_io_plug) {
+            drv->bdrv_io_plug(bs);
+        }
     }
 }
 
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 805757e..102bf92 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -220,19 +220,16 @@ void laio_io_plug(BlockDriverState *bs, void *aio_ctx)
 {
     struct qemu_laio_state *s = aio_ctx;
 
-    s->io_q.plugged++;
+    assert(!s->io_q.plugged);
+    s->io_q.plugged = 1;
 }
 
-void laio_io_unplug(BlockDriverState *bs, void *aio_ctx, bool unplug)
+void laio_io_unplug(BlockDriverState *bs, void *aio_ctx)
 {
     struct qemu_laio_state *s = aio_ctx;
 
-    assert(s->io_q.plugged > 0 || !unplug);
-
-    if (unplug && --s->io_q.plugged > 0) {
-        return;
-    }
-
+    assert(s->io_q.plugged);
+    s->io_q.plugged = 0;
     if (!s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
         ioq_submit(s);
     }
diff --git a/block/raw-aio.h b/block/raw-aio.h
index 31d791f..7600060 100644
--- a/block/raw-aio.h
+++ b/block/raw-aio.h
@@ -41,7 +41,7 @@ BlockAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd,
 void laio_detach_aio_context(void *s, AioContext *old_context);
 void laio_attach_aio_context(void *s, AioContext *new_context);
 void laio_io_plug(BlockDriverState *bs, void *aio_ctx);
-void laio_io_unplug(BlockDriverState *bs, void *aio_ctx, bool unplug);
+void laio_io_unplug(BlockDriverState *bs, void *aio_ctx);
 #endif
 
 #ifdef _WIN32
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 8866121..7ecbee3 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1343,17 +1343,7 @@ static void raw_aio_unplug(BlockDriverState *bs)
 #ifdef CONFIG_LINUX_AIO
     BDRVRawState *s = bs->opaque;
     if (s->use_aio) {
-        laio_io_unplug(bs, s->aio_ctx, true);
-    }
-#endif
-}
-
-static void raw_aio_flush_io_queue(BlockDriverState *bs)
-{
-#ifdef CONFIG_LINUX_AIO
-    BDRVRawState *s = bs->opaque;
-    if (s->use_aio) {
-        laio_io_unplug(bs, s->aio_ctx, false);
+        laio_io_unplug(bs, s->aio_ctx);
     }
 #endif
 }
@@ -1947,7 +1937,6 @@ BlockDriver bdrv_file = {
     .bdrv_refresh_limits = raw_refresh_limits,
     .bdrv_io_plug = raw_aio_plug,
     .bdrv_io_unplug = raw_aio_unplug,
-    .bdrv_flush_io_queue = raw_aio_flush_io_queue,
 
     .bdrv_truncate = raw_truncate,
     .bdrv_getlength = raw_getlength,
@@ -2310,7 +2299,6 @@ static BlockDriver bdrv_host_device = {
     .bdrv_refresh_limits = raw_refresh_limits,
     .bdrv_io_plug = raw_aio_plug,
     .bdrv_io_unplug = raw_aio_unplug,
-    .bdrv_flush_io_queue = raw_aio_flush_io_queue,
 
     .bdrv_truncate      = raw_truncate,
     .bdrv_getlength	= raw_getlength,
@@ -2440,7 +2428,6 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_refresh_limits = raw_refresh_limits,
     .bdrv_io_plug = raw_aio_plug,
     .bdrv_io_unplug = raw_aio_unplug,
-    .bdrv_flush_io_queue = raw_aio_flush_io_queue,
 
     .bdrv_truncate      = raw_truncate,
     .bdrv_getlength      = raw_getlength,
@@ -2576,7 +2563,6 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_refresh_limits = raw_refresh_limits,
     .bdrv_io_plug = raw_aio_plug,
     .bdrv_io_unplug = raw_aio_unplug,
-    .bdrv_flush_io_queue = raw_aio_flush_io_queue,
 
     .bdrv_truncate      = raw_truncate,
     .bdrv_getlength      = raw_getlength,
diff --git a/include/block/block.h b/include/block/block.h
index 01349ef..ba2a18c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -520,7 +520,8 @@ int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
 
 void bdrv_io_plug(BlockDriverState *bs);
 void bdrv_io_unplug(BlockDriverState *bs);
-void bdrv_flush_io_queue(BlockDriverState *bs);
+void bdrv_io_unplugged_begin(BlockDriverState *bs);
+void bdrv_io_unplugged_end(BlockDriverState *bs);
 
 /**
  * bdrv_drained_begin:
diff --git a/include/block/block_int.h b/include/block/block_int.h
index cfcbd96..627556b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -282,7 +282,6 @@ struct BlockDriver {
     /* io queue for linux-aio */
     void (*bdrv_io_plug)(BlockDriverState *bs);
     void (*bdrv_io_unplug)(BlockDriverState *bs);
-    void (*bdrv_flush_io_queue)(BlockDriverState *bs);
 
     /**
      * Try to get @bs's logical and physical block size.
@@ -475,6 +474,10 @@ struct BlockDriverState {
     uint64_t write_threshold_offset;
     NotifierWithReturn write_threshold_notifier;
 
+    /* counters for nested bdrv_io_plug and bdrv_io_unplugged_begin */
+    unsigned io_plugged;
+    unsigned io_plug_disabled;
+
     int quiesce_counter;
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/7] linux-aio: make it more type safe

[Paolo Bonzini]

Replace void* with an opaque LinuxAioState type.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/linux-aio.c | 46 +++++++++++++++++-----------------------------
 block/raw-aio.h   | 15 ++++++++-------
 block/raw-posix.c |  4 ++--
 3 files changed, 27 insertions(+), 38 deletions(-)

diff --git a/block/linux-aio.c b/block/linux-aio.c
index 102bf92..90ec98e 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -30,7 +30,7 @@
 
 struct qemu_laiocb {
     BlockAIOCB common;
-    struct qemu_laio_state *ctx;
+    LinuxAioState *ctx;
     struct iocb iocb;
     ssize_t ret;
     size_t nbytes;
@@ -46,7 +46,7 @@ typedef struct {
     QSIMPLEQ_HEAD(, qemu_laiocb) pending;
 } LaioQueue;
 
-struct qemu_laio_state {
+struct LinuxAioState {
     io_context_t ctx;
     EventNotifier e;
 
@@ -60,7 +60,7 @@ struct qemu_laio_state {
     int event_max;
 };
 
-static void ioq_submit(struct qemu_laio_state *s);
+static void ioq_submit(LinuxAioState *s);
 
 static inline ssize_t io_event_ret(struct io_event *ev)
 {
@@ -70,8 +70,7 @@ static inline ssize_t io_event_ret(struct io_event *ev)
 /*
  * Completes an AIO request (calls the callback and frees the ACB).
  */
-static void qemu_laio_process_completion(struct qemu_laio_state *s,
-    struct qemu_laiocb *laiocb)
+static void qemu_laio_process_completion(struct qemu_laiocb *laiocb)
 {
     int ret;
 
@@ -99,7 +98,7 @@ static void qemu_laio_process_completion(struct qemu_laio_state *s,
  *
  * The function is somewhat tricky because it supports nested event loops, for
  * example when a request callback invokes aio_poll().  In order to do this,
- * the completion events array and index are kept in qemu_laio_state.  The BH
+ * the completion events array and index are kept in LinuxAioState.  The BH
  * reschedules itself as long as there are completions pending so it will
  * either be called again in a nested event loop or will be called after all
  * events have been completed.  When there are no events left to complete, the
@@ -107,7 +106,7 @@ static void qemu_laio_process_completion(struct qemu_laio_state *s,
  */
 static void qemu_laio_completion_bh(void *opaque)
 {
-    struct qemu_laio_state *s = opaque;
+    LinuxAioState *s = opaque;
 
     /* Fetch more completion events when empty */
     if (s->event_idx == s->event_max) {
@@ -136,7 +135,7 @@ static void qemu_laio_completion_bh(void *opaque)
         laiocb->ret = io_event_ret(&s->events[s->event_idx]);
         s->event_idx++;
 
-        qemu_laio_process_completion(s, laiocb);
+        qemu_laio_process_completion(laiocb);
     }
 
     if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
@@ -146,7 +145,7 @@ static void qemu_laio_completion_bh(void *opaque)
 
 static void qemu_laio_completion_cb(EventNotifier *e)
 {
-    struct qemu_laio_state *s = container_of(e, struct qemu_laio_state, e);
+    LinuxAioState *s = container_of(e, LinuxAioState, e);
 
     if (event_notifier_test_and_clear(&s->e)) {
         qemu_bh_schedule(s->completion_bh);
@@ -185,7 +184,7 @@ static void ioq_init(LaioQueue *io_q)
     io_q->blocked = false;
 }
 
-static void ioq_submit(struct qemu_laio_state *s)
+static void ioq_submit(LinuxAioState *s)
 {
     int ret, len;
     struct qemu_laiocb *aiocb;
@@ -216,18 +215,14 @@ static void ioq_submit(struct qemu_laio_state *s)
     s->io_q.blocked = (s->io_q.n > 0);
 }
 
-void laio_io_plug(BlockDriverState *bs, void *aio_ctx)
+void laio_io_plug(BlockDriverState *bs, LinuxAioState *s)
 {
-    struct qemu_laio_state *s = aio_ctx;
-
     assert(!s->io_q.plugged);
     s->io_q.plugged = 1;
 }
 
-void laio_io_unplug(BlockDriverState *bs, void *aio_ctx)
+void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s)
 {
-    struct qemu_laio_state *s = aio_ctx;
-
     assert(s->io_q.plugged);
     s->io_q.plugged = 0;
     if (!s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
@@ -235,11 +230,10 @@ void laio_io_unplug(BlockDriverState *bs, void *aio_ctx)
     }
 }
 
-BlockAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd,
+BlockAIOCB *laio_submit(BlockDriverState *bs, LinuxAioState *s, int fd,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockCompletionFunc *cb, void *opaque, int type)
 {
-    struct qemu_laio_state *s = aio_ctx;
     struct qemu_laiocb *laiocb;
     struct iocb *iocbs;
     off_t offset = sector_num * 512;
@@ -281,26 +275,22 @@ out_free_aiocb:
     return NULL;
 }
 
-void laio_detach_aio_context(void *s_, AioContext *old_context)
+void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context)
 {
-    struct qemu_laio_state *s = s_;
-
     aio_set_event_notifier(old_context, &s->e, false, NULL);
     qemu_bh_delete(s->completion_bh);
 }
 
-void laio_attach_aio_context(void *s_, AioContext *new_context)
+void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context)
 {
-    struct qemu_laio_state *s = s_;
-
     s->completion_bh = aio_bh_new(new_context, qemu_laio_completion_bh, s);
     aio_set_event_notifier(new_context, &s->e, false,
                            qemu_laio_completion_cb);
 }
 
-void *laio_init(void)
+LinuxAioState *laio_init(void)
 {
-    struct qemu_laio_state *s;
+    LinuxAioState *s;
 
     s = g_malloc0(sizeof(*s));
     if (event_notifier_init(&s->e, false) < 0) {
@@ -322,10 +312,8 @@ out_free_state:
     return NULL;
 }
 
-void laio_cleanup(void *s_)
+void laio_cleanup(LinuxAioState *s)
 {
-    struct qemu_laio_state *s = s_;
-
     event_notifier_cleanup(&s->e);
 
     if (io_destroy(s->ctx) != 0) {
diff --git a/block/raw-aio.h b/block/raw-aio.h
index 7600060..7427f76 100644
--- a/block/raw-aio.h
+++ b/block/raw-aio.h
@@ -33,15 +33,16 @@
 
 /* linux-aio.c - Linux native implementation */
 #ifdef CONFIG_LINUX_AIO
-void *laio_init(void);
-void laio_cleanup(void *s);
-BlockAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd,
+typedef struct LinuxAioState LinuxAioState;
+LinuxAioState *laio_init(void);
+void laio_cleanup(LinuxAioState *s);
+BlockAIOCB *laio_submit(BlockDriverState *bs, LinuxAioState *s, int fd,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockCompletionFunc *cb, void *opaque, int type);
-void laio_detach_aio_context(void *s, AioContext *old_context);
-void laio_attach_aio_context(void *s, AioContext *new_context);
-void laio_io_plug(BlockDriverState *bs, void *aio_ctx);
-void laio_io_unplug(BlockDriverState *bs, void *aio_ctx);
+void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context);
+void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context);
+void laio_io_plug(BlockDriverState *bs, LinuxAioState *s);
+void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s);
 #endif
 
 #ifdef _WIN32
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 7ecbee3..0f14d0f 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -137,7 +137,7 @@ typedef struct BDRVRawState {
 
 #ifdef CONFIG_LINUX_AIO
     int use_aio;
-    void *aio_ctx;
+    LinuxAioState *aio_ctx;
 #endif
 #ifdef CONFIG_XFS
     bool is_xfs:1;
@@ -396,7 +396,7 @@ static void raw_attach_aio_context(BlockDriverState *bs,
 }
 
 #ifdef CONFIG_LINUX_AIO
-static int raw_set_aio(void **aio_ctx, int *use_aio, int bdrv_flags)
+static int raw_set_aio(LinuxAioState **aio_ctx, int *use_aio, int bdrv_flags)
 {
     int ret = -1;
     assert(aio_ctx != NULL);
-- 
1.8.3.1

[Qemu-devel] [PATCH 7/7] linux-aio: share one LinuxAioState within an AioContext

[Paolo Bonzini]

This has better performance because it executes fewer system calls
and does not use a bottom half per disk.

Originally proposed by Ming Lei.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 async.c                            |  23 +++++++
 block/linux-aio.c                  |   3 +
 block/raw-posix.c                  | 119 +++++--------------------------------
 block/raw-win32.c                  |   2 +-
 include/block/aio.h                |  13 ++++
 {block => include/block}/raw-aio.h |   0
 6 files changed, 54 insertions(+), 106 deletions(-)
 rename {block => include/block}/raw-aio.h (100%)

diff --git a/async.c b/async.c
index d4dd2cc..dc7e50c 100644
--- a/async.c
+++ b/async.c
@@ -28,6 +28,7 @@
 #include "block/thread-pool.h"
 #include "qemu/main-loop.h"
 #include "qemu/atomic.h"
+#include "block/raw-aio.h"
 
 /***********************************************************/
 /* bottom halves (can be seen as timers which expire ASAP) */
@@ -241,6 +242,14 @@ aio_ctx_finalize(GSource     *source)
     qemu_bh_delete(ctx->notify_dummy_bh);
     thread_pool_free(ctx->thread_pool);
 
+#ifdef CONFIG_LINUX_AIO
+    if (ctx->linux_aio) {
+        laio_detach_aio_context(ctx->linux_aio, ctx);
+        laio_cleanup(ctx->linux_aio);
+        ctx->linux_aio = NULL;
+    }
+#endif
+
     qemu_mutex_lock(&ctx->bh_lock);
     while (ctx->first_bh) {
         QEMUBH *next = ctx->first_bh->next;
@@ -281,6 +290,17 @@ ThreadPool *aio_get_thread_pool(AioContext *ctx)
     return ctx->thread_pool;
 }
 
+#ifdef CONFIG_LINUX_AIO
+LinuxAioState *aio_get_linux_aio(AioContext *ctx)
+{
+    if (!ctx->linux_aio) {
+        ctx->linux_aio = laio_init();
+        laio_attach_aio_context(ctx->linux_aio, ctx);
+    }
+    return ctx->linux_aio;
+}
+#endif
+
 void aio_notify(AioContext *ctx)
 {
     /* Write e.g. bh->scheduled before reading ctx->notify_me.  Pairs
@@ -344,6 +364,9 @@ AioContext *aio_context_new(Error **errp)
                            false,
                            (EventNotifierHandler *)
                            event_notifier_dummy_cb);
+#ifdef CONFIG_LINUX_AIO
+    ctx->linux_aio = NULL;
+#endif
     ctx->thread_pool = NULL;
     qemu_mutex_init(&ctx->bh_lock);
     rfifolock_init(&ctx->lock, aio_rfifolock_cb, ctx);
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 90ec98e..b73ca14 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -47,6 +47,8 @@ typedef struct {
 } LaioQueue;
 
 struct LinuxAioState {
+    AioContext *aio_context;
+
     io_context_t ctx;
     EventNotifier e;
 
@@ -283,6 +285,7 @@ void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context)
 
 void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context)
 {
+    s->aio_context = new_context;
     s->completion_bh = aio_bh_new(new_context, qemu_laio_completion_bh, s);
     aio_set_event_notifier(new_context, &s->e, false,
                            qemu_laio_completion_cb);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 0f14d0f..e6cf233 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -31,7 +31,7 @@
 #include "trace.h"
 #include "block/thread-pool.h"
 #include "qemu/iov.h"
-#include "raw-aio.h"
+#include "block/raw-aio.h"
 #include "qapi/util.h"
 #include "qapi/qmp/qstring.h"
 
@@ -135,10 +135,6 @@ typedef struct BDRVRawState {
     int open_flags;
     size_t buf_align;
 
-#ifdef CONFIG_LINUX_AIO
-    int use_aio;
-    LinuxAioState *aio_ctx;
-#endif
 #ifdef CONFIG_XFS
     bool is_xfs:1;
 #endif
@@ -152,9 +148,6 @@ typedef struct BDRVRawState {
 typedef struct BDRVRawReopenState {
     int fd;
     int open_flags;
-#ifdef CONFIG_LINUX_AIO
-    int use_aio;
-#endif
 } BDRVRawReopenState;
 
 static int fd_open(BlockDriverState *bs);
@@ -372,58 +365,15 @@ static void raw_parse_flags(int bdrv_flags, int *open_flags)
     }
 }
 
-static void raw_detach_aio_context(BlockDriverState *bs)
-{
 #ifdef CONFIG_LINUX_AIO
-    BDRVRawState *s = bs->opaque;
-
-    if (s->use_aio) {
-        laio_detach_aio_context(s->aio_ctx, bdrv_get_aio_context(bs));
-    }
-#endif
-}
-
-static void raw_attach_aio_context(BlockDriverState *bs,
-                                   AioContext *new_context)
+static bool raw_use_aio(int bdrv_flags)
 {
-#ifdef CONFIG_LINUX_AIO
-    BDRVRawState *s = bs->opaque;
-
-    if (s->use_aio) {
-        laio_attach_aio_context(s->aio_ctx, new_context);
-    }
-#endif
-}
-
-#ifdef CONFIG_LINUX_AIO
-static int raw_set_aio(LinuxAioState **aio_ctx, int *use_aio, int bdrv_flags)
-{
-    int ret = -1;
-    assert(aio_ctx != NULL);
-    assert(use_aio != NULL);
     /*
      * Currently Linux do AIO only for files opened with O_DIRECT
      * specified so check NOCACHE flag too
      */
-    if ((bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
-                      (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) {
-
-        /* if non-NULL, laio_init() has already been run */
-        if (*aio_ctx == NULL) {
-            *aio_ctx = laio_init();
-            if (!*aio_ctx) {
-                goto error;
-            }
-        }
-        *use_aio = 1;
-    } else {
-        *use_aio = 0;
-    }
-
-    ret = 0;
-
-error:
-    return ret;
+    return (bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
+                         (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO);
 }
 #endif
 
@@ -492,13 +442,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     s->fd = fd;
 
 #ifdef CONFIG_LINUX_AIO
-    if (raw_set_aio(&s->aio_ctx, &s->use_aio, bdrv_flags)) {
-        qemu_close(fd);
-        ret = -errno;
-        error_setg_errno(errp, -ret, "Could not set AIO state");
-        goto fail;
-    }
-    if (!s->use_aio && (bdrv_flags & BDRV_O_NATIVE_AIO)) {
+    if (!raw_use_aio(bdrv_flags) && (bdrv_flags & BDRV_O_NATIVE_AIO)) {
         error_setg(errp, "aio=native was specified, but it requires "
                          "cache.direct=on, which was not specified.");
         ret = -EINVAL;
@@ -564,8 +508,6 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     }
 #endif
 
-    raw_attach_aio_context(bs, bdrv_get_aio_context(bs));
-
     ret = 0;
 fail:
     if (filename && (bdrv_flags & BDRV_O_TEMPORARY)) {
@@ -606,18 +548,6 @@ static int raw_reopen_prepare(BDRVReopenState *state,
     state->opaque = g_new0(BDRVRawReopenState, 1);
     raw_s = state->opaque;
 
-#ifdef CONFIG_LINUX_AIO
-    raw_s->use_aio = s->use_aio;
-
-    /* we can use s->aio_ctx instead of a copy, because the use_aio flag is
-     * valid in the 'false' condition even if aio_ctx is set, and raw_set_aio()
-     * won't override aio_ctx if aio_ctx is non-NULL */
-    if (raw_set_aio(&s->aio_ctx, &raw_s->use_aio, state->flags)) {
-        error_setg(errp, "Could not set AIO state");
-        return -1;
-    }
-#endif
-
     if (s->type == FTYPE_CD) {
         raw_s->open_flags |= O_NONBLOCK;
     }
@@ -700,9 +630,6 @@ static void raw_reopen_commit(BDRVReopenState *state)
 
     qemu_close(s->fd);
     s->fd = raw_s->fd;
-#ifdef CONFIG_LINUX_AIO
-    s->use_aio = raw_s->use_aio;
-#endif
 
     g_free(state->opaque);
     state->opaque = NULL;
@@ -1317,8 +1244,9 @@ static BlockAIOCB *raw_aio_submit(BlockDriverState *bs,
         if (!bdrv_qiov_is_aligned(bs, qiov)) {
             type |= QEMU_AIO_MISALIGNED;
 #ifdef CONFIG_LINUX_AIO
-        } else if (s->use_aio) {
-            return laio_submit(bs, s->aio_ctx, s->fd, sector_num, qiov,
+        } else if (bs->open_flags & BDRV_O_NATIVE_AIO) {
+            LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
+            return laio_submit(bs, aio, s->fd, sector_num, qiov,
                                nb_sectors, cb, opaque, type);
 #endif
         }
@@ -1331,9 +1259,9 @@ static BlockAIOCB *raw_aio_submit(BlockDriverState *bs,
 static void raw_aio_plug(BlockDriverState *bs)
 {
 #ifdef CONFIG_LINUX_AIO
-    BDRVRawState *s = bs->opaque;
-    if (s->use_aio) {
-        laio_io_plug(bs, s->aio_ctx);
+    if (bs->open_flags & BDRV_O_NATIVE_AIO) {
+        LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
+        laio_io_plug(bs, aio);
     }
 #endif
 }
@@ -1341,9 +1269,9 @@ static void raw_aio_plug(BlockDriverState *bs)
 static void raw_aio_unplug(BlockDriverState *bs)
 {
 #ifdef CONFIG_LINUX_AIO
-    BDRVRawState *s = bs->opaque;
-    if (s->use_aio) {
-        laio_io_unplug(bs, s->aio_ctx);
+    if (bs->open_flags & BDRV_O_NATIVE_AIO) {
+        LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
+        laio_io_unplug(bs, aio);
     }
 #endif
 }
@@ -1379,13 +1307,6 @@ static void raw_close(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
 
-    raw_detach_aio_context(bs);
-
-#ifdef CONFIG_LINUX_AIO
-    if (s->use_aio) {
-        laio_cleanup(s->aio_ctx);
-    }
-#endif
     if (s->fd >= 0) {
         qemu_close(s->fd);
         s->fd = -1;
@@ -1944,9 +1865,6 @@ BlockDriver bdrv_file = {
     .bdrv_get_allocated_file_size
                         = raw_get_allocated_file_size,
 
-    .bdrv_detach_aio_context = raw_detach_aio_context,
-    .bdrv_attach_aio_context = raw_attach_aio_context,
-
     .create_opts = &raw_create_opts,
 };
 
@@ -2308,9 +2226,6 @@ static BlockDriver bdrv_host_device = {
     .bdrv_probe_blocksizes = hdev_probe_blocksizes,
     .bdrv_probe_geometry = hdev_probe_geometry,
 
-    .bdrv_detach_aio_context = raw_detach_aio_context,
-    .bdrv_attach_aio_context = raw_attach_aio_context,
-
     /* generic scsi device */
 #ifdef __linux__
     .bdrv_aio_ioctl     = hdev_aio_ioctl,
@@ -2435,9 +2350,6 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_get_allocated_file_size
                         = raw_get_allocated_file_size,
 
-    .bdrv_detach_aio_context = raw_detach_aio_context,
-    .bdrv_attach_aio_context = raw_attach_aio_context,
-
     /* removable device support */
     .bdrv_is_inserted   = cdrom_is_inserted,
     .bdrv_eject         = cdrom_eject,
@@ -2570,9 +2482,6 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_get_allocated_file_size
                         = raw_get_allocated_file_size,
 
-    .bdrv_detach_aio_context = raw_detach_aio_context,
-    .bdrv_attach_aio_context = raw_attach_aio_context,
-
     /* removable device support */
     .bdrv_is_inserted   = cdrom_is_inserted,
     .bdrv_eject         = cdrom_eject,
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 21a6cb8..b085a5f 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -26,7 +26,7 @@
 #include "qemu/timer.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
-#include "raw-aio.h"
+#include "block/raw-aio.h"
 #include "trace.h"
 #include "block/thread-pool.h"
 #include "qemu/iov.h"
diff --git a/include/block/aio.h b/include/block/aio.h
index e086e3b..b87d146 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -48,6 +48,9 @@ typedef struct AioHandler AioHandler;
 typedef void QEMUBHFunc(void *opaque);
 typedef void IOHandler(void *opaque);
 
+struct ThreadPool;
+struct LinuxAioState;
+
 struct AioContext {
     GSource source;
 
@@ -120,6 +123,13 @@ struct AioContext {
     /* Thread pool for performing work and receiving completion callbacks */
     struct ThreadPool *thread_pool;
 
+#ifdef CONFIG_LINUX_AIO
+    /* State for native Linux AIO.  Uses aio_context_acquire/release for
+     * locking.
+     */
+    struct LinuxAioState *linux_aio;
+#endif
+
     /* TimerLists for calling timers - one per clock type */
     QEMUTimerListGroup tlg;
 
@@ -336,6 +346,9 @@ GSource *aio_get_g_source(AioContext *ctx);
 /* Return the ThreadPool bound to this AioContext */
 struct ThreadPool *aio_get_thread_pool(AioContext *ctx);
 
+/* Return the LinuxAioState bound to this AioContext */
+struct LinuxAioState *aio_get_linux_aio(AioContext *ctx);
+
 /**
  * aio_timer_new:
  * @ctx: the aio context
diff --git a/block/raw-aio.h b/include/block/raw-aio.h
similarity index 100%
rename from block/raw-aio.h
rename to include/block/raw-aio.h
-- 
1.8.3.1

Re: [Qemu-devel] [Qemu-block] [PATCH 2/7] block: make bdrv_start_throttled_reqs return void

[Alberto Garcia]

On Thu 24 Mar 2016 05:39:21 PM CET, Paolo Bonzini wrote:
> The return value is unused and I am not sure why it would be useful.

Yeah, this is not needed since 0b06ef3bdd177.

Reviewed-by: Alberto Garcia <berto@igalia.com>

>          while (qemu_co_enter_next(&bs->throttled_reqs[i])) {
> -            drained = true;
> +            ;
>          }

Is that semicolon necessary, though?

Berto

Re: [Qemu-devel] [Qemu-block] [PATCH 3/7] block: move restarting of throttled reqs to block/throttle-groups.c

[Alberto Garcia]

On Thu 24 Mar 2016 05:39:22 PM CET, Paolo Bonzini wrote:
> @@ -335,6 +346,11 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
>      }
>      throttle_config(ts, tt, cfg);
>      qemu_mutex_unlock(&tg->lock);
> +
> +    aio_context_acquire(bdrv_get_aio_context(bs));
> +    qemu_co_enter_next(&bs->throttled_reqs[0]);
> +    qemu_co_enter_next(&bs->throttled_reqs[1]);
> +    aio_context_release(bdrv_get_aio_context(bs));
>  }

None of the functions in throttle-groups.c acquire the AioContext
because they all assume that the caller is doing it, so we can do the
same here for consistency.

Berto

Re: [Qemu-devel] [Qemu-block] [PATCH 3/7] block: move restarting of throttled reqs to block/throttle-groups.c

[Paolo Bonzini]

On 29/03/2016 16:14, Alberto Garcia wrote:
> On Thu 24 Mar 2016 05:39:22 PM CET, Paolo Bonzini wrote:
>> @@ -335,6 +346,11 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
>>      }
>>      throttle_config(ts, tt, cfg);
>>      qemu_mutex_unlock(&tg->lock);
>> +
>> +    aio_context_acquire(bdrv_get_aio_context(bs));
>> +    qemu_co_enter_next(&bs->throttled_reqs[0]);
>> +    qemu_co_enter_next(&bs->throttled_reqs[1]);
>> +    aio_context_release(bdrv_get_aio_context(bs));
>>  }
> 
> None of the functions in throttle-groups.c acquire the AioContext
> because they all assume that the caller is doing it, so we can do the
> same here for consistency.

It turns out the caller (qmp_block_set_io_throttle) _is_ doing it, so we
can get rid of the aio_context_acquire/release pair here too.

Paolo

Re: [Qemu-devel] [Qemu-block] [PATCH 4/7] block: introduce bdrv_no_throttling_begin/end

[Alberto Garcia]

On Thu 24 Mar 2016 05:39:23 PM CET, Paolo Bonzini wrote:
> Extract the handling of throttling from bdrv_flush_io_queue.  These
> new functions will soon become BdrvChildRole callbacks, as they can
> be generalized to "beginning of drain" and "end of drain".
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

I like the idea, thanks for the patch!

Reviewed-by: Alberto Garcia <berto@igalia.com>

> +void bdrv_no_throttling_end(BlockDriverState *bs)
> +{
> +    --bs->io_limits_disabled;
>  }

Not very important, but you could assert that this doesn't go below 0.

Berto

Re: [Qemu-devel] [PATCH 4/7] block: introduce bdrv_no_throttling_begin/end

[Paolo Bonzini]

On 30/03/2016 14:24, Alberto Garcia wrote:
>> > +void bdrv_no_throttling_end(BlockDriverState *bs)
>> > +{
>> > +    --bs->io_limits_disabled;
>> >  }
> Not very important, but you could assert that this doesn't go below 0.

Sure, why not.

Paolo

Re: [Qemu-devel] [PATCH v3 0/7] bdrv_flush_io_queue removal, shared LinuxAioState

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]

On Thu, Mar 24, 2016 at 05:39:19PM +0100, Paolo Bonzini wrote:
> Patch 1 comes from Kevin's series to do BlockBackend throttling.
> 
> Patches 2-5 are from my bdrv_drain patches.
> 
> Patches 6-7 are new but based on Ming Lei's old submission.
> I'm including them here because they apply on top of patches 2-5.
> It would be nice to have them too in 2.6.
> 
> Paolo
> 
> Kevin Wolf (1):
>   block: Don't disable I/O throttling on sync requests
> 
> Paolo Bonzini (6):
>   block: make bdrv_start_throttled_reqs return void
>   block: move restarting of throttled reqs to block/throttle-groups.c
>   block: introduce bdrv_no_throttling_begin/end
>   block: plug whole tree at once, introduce bdrv_io_unplugged_begin/end
>   linux-aio: make it more type safe
>   linux-aio: share one LinuxAioState within an AioContext
> 
>  async.c                            |  23 +++++++
>  block.c                            |   1 -
>  block/block-backend.c              |   6 +-
>  block/io.c                         | 128 +++++++++++++++++++----------------
>  block/linux-aio.c                  |  60 +++++++----------
>  block/raw-posix.c                  | 133 ++++---------------------------------
>  block/raw-win32.c                  |   2 +-
>  block/throttle-groups.c            |  20 ++++++
>  include/block/aio.h                |  13 ++++
>  include/block/block.h              |   3 +-
>  include/block/block_int.h          |  14 ++--
>  {block => include/block}/raw-aio.h |  15 +++--
>  include/block/throttle-groups.h    |   1 +
>  13 files changed, 189 insertions(+), 230 deletions(-)
>  rename {block => include/block}/raw-aio.h (80%)

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 0/7] bdrv_flush_io_queue removal, shared LinuxAioState

[Paolo Bonzini]

On 30/03/2016 15:06, Stefan Hajnoczi wrote:
> On Thu, Mar 24, 2016 at 05:39:19PM +0100, Paolo Bonzini wrote:
>> Patch 1 comes from Kevin's series to do BlockBackend throttling.
>>
>> Patches 2-5 are from my bdrv_drain patches.
>>
>> Patches 6-7 are new but based on Ming Lei's old submission.
>> I'm including them here because they apply on top of patches 2-5.
>> It would be nice to have them too in 2.6.
>>
>> Paolo
>>
>> Kevin Wolf (1):
>>   block: Don't disable I/O throttling on sync requests
>>
>> Paolo Bonzini (6):
>>   block: make bdrv_start_throttled_reqs return void
>>   block: move restarting of throttled reqs to block/throttle-groups.c
>>   block: introduce bdrv_no_throttling_begin/end
>>   block: plug whole tree at once, introduce bdrv_io_unplugged_begin/end
>>   linux-aio: make it more type safe
>>   linux-aio: share one LinuxAioState within an AioContext
>>
>>  async.c                            |  23 +++++++
>>  block.c                            |   1 -
>>  block/block-backend.c              |   6 +-
>>  block/io.c                         | 128 +++++++++++++++++++----------------
>>  block/linux-aio.c                  |  60 +++++++----------
>>  block/raw-posix.c                  | 133 ++++---------------------------------
>>  block/raw-win32.c                  |   2 +-
>>  block/throttle-groups.c            |  20 ++++++
>>  include/block/aio.h                |  13 ++++
>>  include/block/block.h              |   3 +-
>>  include/block/block_int.h          |  14 ++--
>>  {block => include/block}/raw-aio.h |  15 +++--
>>  include/block/throttle-groups.h    |   1 +
>>  13 files changed, 189 insertions(+), 230 deletions(-)
>>  rename {block => include/block}/raw-aio.h (80%)
> 
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

Great, I'll send v4 as soon as possible for inclusion in the block-next
branch.

Paolo

Re: [Qemu-devel] [PATCH v3 0/7] bdrv_flush_io_queue removal, shared LinuxAioState

[Kevin Wolf]

Am 30.03.2016 um 15:17 hat Paolo Bonzini geschrieben:
> On 30/03/2016 15:06, Stefan Hajnoczi wrote:
> > Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Great, I'll send v4 as soon as possible for inclusion in the block-next
> branch.

Are you still working on v4 or did I miss it? All of the block-next
candidate series are waiting to be rebased on this one, and moving parts
at the bottom of a longish dependency chain are kind of nasty.

Kevin

Re: [Qemu-devel] [PATCH v3 0/7] bdrv_flush_io_queue removal, shared LinuxAioState

[Paolo Bonzini]

On 06/04/2016 20:19, Kevin Wolf wrote:
>> > 
>> > Great, I'll send v4 as soon as possible for inclusion in the block-next
>> > branch.
> Are you still working on v4 or did I miss it? All of the block-next
> candidate series are waiting to be rebased on this one, and moving parts
> at the bottom of a longish dependency chain are kind of nasty.

Yes, I am waiting for Fam's bdrv_co_drain patch, which is this one
depends on.

Paolo

Re: [Qemu-devel] [PATCH v3 0/7] bdrv_flush_io_queue removal, shared LinuxAioState

[Kevin Wolf]

Am 06.04.2016 um 20:28 hat Paolo Bonzini geschrieben:
> 
> 
> On 06/04/2016 20:19, Kevin Wolf wrote:
> >> > 
> >> > Great, I'll send v4 as soon as possible for inclusion in the block-next
> >> > branch.
> > Are you still working on v4 or did I miss it? All of the block-next
> > candidate series are waiting to be rebased on this one, and moving parts
> > at the bottom of a longish dependency chain are kind of nasty.
> 
> Yes, I am waiting for Fam's bdrv_co_drain patch, which is this one
> depends on.

Oh, I wasn't aware of that one. Thanks!

Kevin