* [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend
@ 2016-05-11 13:24 Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 01/14] block: Make sure throttled BDSes always have a BB Kevin Wolf
` (14 more replies)
0 siblings, 15 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
This is another feature that was "logically" part of the BlockBackend, but
implemented as a BlockDriverState feature. It was always kept on top using
swap_feature_fields().
This series moves it to be actually implemented in the BlockBackend, removing
another obstacle for removing bs->blk and allowing multiple BBs per BDS.
Applies to block-next.
v3:
- Patch 5: 'block: Move throttling fields from BDS to BB'
Fixed a few commentions still mentioning BDS instead of BB [Berto]
- Patch 9: 'block: Drain throttling queue with BdrvChild'
Documented the semantics of BdrvChildRole.drained_begin/end [Stefan]
- Patch 10 (new): 'block/io: Quiesce parents between drained_begin/end'
Fix that parent requests were allowed again too early [Stefan]
- Patch 14 (was 13): 'block: Don't check throttled reqs in
bdrv_requests_pending()'
Explained more cases in the commit message [Berto]
v2:
- Rebased on top of Paolo's 'bdrv_flush_io_queue removal, shared LinuxAioState'
Most notable this includes a complete rewrite of patch 9 (was 10): 'block:
Drain throttling queue with BdrvChild'. Instead of a single drain_queue()
callback we now have a drained_begin/end() pair.
- Patch 2 (was 3): 'block: Introduce BlockBackendPublic'
Add int dummy to yet empty struct BlockBackendPublic [Eric]
- Patch 11: 'block: Remove bdrv_move_feature_fields()'
After the rebase, the function ended up empty, we can remove it now
- Patch 12: 'Revert "block: Forbid I/O throttling on nodes with
multiple parents for 2.6"'
This was committed to master after v1 had been posted, so this one is new as
well. The reason for forbidding this was that patch 6 ('block: Move actual
I/O throttling to BlockBackend') would change the behaviour of the non-BB
parents. Now that the final behaviour is implemented, we can allow the setup.
Kevin Wolf (14):
block: Make sure throttled BDSes always have a BB
block: Introduce BlockBackendPublic
block: throttle-groups: Use BlockBackend pointers internally
block: Convert throttle_group_get_name() to BlockBackend
block: Move throttling fields from BDS to BB
block: Move actual I/O throttling to BlockBackend
block: Move I/O throttling configuration functions to BlockBackend
block: Introduce BdrvChild.opaque
block: Drain throttling queue with BdrvChild callback
block/io: Quiesce parents between drained_begin/end
block: Decouple throttling from BlockDriverState
block: Remove bdrv_move_feature_fields()
Revert "block: Forbid I/O throttling on nodes with multiple parents
for 2.6"
block: Don't check throttled reqs in bdrv_requests_pending()
block.c | 58 +---------
block/block-backend.c | 135 ++++++++++++++++++----
block/io.c | 86 ++++----------
block/qapi.c | 6 +-
block/throttle-groups.c | 244 +++++++++++++++++++++-------------------
blockdev.c | 50 +++-----
include/block/block.h | 4 -
include/block/block_int.h | 35 ++----
include/block/throttle-groups.h | 14 +--
include/sysemu/block-backend.h | 27 +++++
tests/test-throttle.c | 62 +++++-----
11 files changed, 359 insertions(+), 362 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 01/14] block: Make sure throttled BDSes always have a BB
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 02/14] block: Introduce BlockBackendPublic Kevin Wolf
` (13 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
It was already true in principle that a throttled BDS always has a BB
attached, except that the order of operations while attaching or
detaching a BDS to/from a BB wasn't careful enough.
This commit breaks graph manipulations while I/O throttling is enabled.
It would have been possible to keep things working with some temporary
hacks, but quite cumbersome, so it's not worth the hassle. We'll fix
things again in a minute.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block.c | 14 ++++++++++++++
block/block-backend.c | 3 +++
blockdev.c | 4 ++--
tests/test-throttle.c | 11 ++++++++---
4 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index 724b3b2..9bad665 100644
--- a/block.c
+++ b/block.c
@@ -2259,8 +2259,22 @@ static void swap_feature_fields(BlockDriverState *bs_top,
assert(!bs_new->throttle_state);
if (bs_top->throttle_state) {
+ /*
+ * FIXME Need to break I/O throttling with graph manipulations
+ * temporarily because of conflicting invariants (3. will go away when
+ * throttling is fully converted to work on BlockBackends):
+ *
+ * 1. Every BlockBackend has a single root BDS
+ * 2. I/O throttling functions require an attached BlockBackend
+ * 3. We need to first enable throttling on the new BDS and then
+ * disable it on the old one (because of throttle group refcounts)
+ */
+#if 0
bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top));
bdrv_io_limits_disable(bs_top);
+#else
+ abort();
+#endif
}
}
diff --git a/block/block-backend.c b/block/block-backend.c
index a1e2c7f..53fd9d2 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -420,6 +420,9 @@ void blk_remove_bs(BlockBackend *blk)
notifier_list_notify(&blk->remove_bs_notifiers, blk);
blk_update_root_state(blk);
+ if (blk->root->bs->throttle_state) {
+ bdrv_io_limits_disable(blk->root->bs);
+ }
blk->root->bs->blk = NULL;
bdrv_root_unref_child(blk->root);
diff --git a/blockdev.c b/blockdev.c
index f74eb43..585a28a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2570,8 +2570,6 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
goto fail;
}
- blk_apply_root_state(blk, medium_bs);
-
bdrv_add_key(medium_bs, NULL, &err);
if (err) {
error_propagate(errp, err);
@@ -2596,6 +2594,8 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
goto fail;
}
+ blk_apply_root_state(blk, medium_bs);
+
qmp_blockdev_close_tray(device, errp);
fail:
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 744a524..77b95d6 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -574,11 +574,16 @@ static void test_accounting(void)
static void test_groups(void)
{
ThrottleConfig cfg1, cfg2;
+ BlockBackend *blk1, *blk2, *blk3;
BlockDriverState *bdrv1, *bdrv2, *bdrv3;
- bdrv1 = bdrv_new();
- bdrv2 = bdrv_new();
- bdrv3 = bdrv_new();
+ blk1 = blk_new_with_bs(&error_abort);
+ blk2 = blk_new_with_bs(&error_abort);
+ blk3 = blk_new_with_bs(&error_abort);
+
+ bdrv1 = blk_bs(blk1);
+ bdrv2 = blk_bs(blk2);
+ bdrv3 = blk_bs(blk3);
g_assert(bdrv1->throttle_state == NULL);
g_assert(bdrv2->throttle_state == NULL);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 02/14] block: Introduce BlockBackendPublic
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 01/14] block: Make sure throttled BDSes always have a BB Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 03/14] block: throttle-groups: Use BlockBackend pointers internally Kevin Wolf
` (12 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
Some features, like I/O throttling, are implemented outside
block-backend.c, but still want to keep information in BlockBackend,
e.g. list entries that allow keeping a list of BlockBackends.
In order to avoid exposing the whole struct layout in the public header
file, this patch introduces an embedded public struct where such
information can be added and a pair of functions to convert between
BlockBackend and BlockBackendPublic.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/block-backend.c | 17 +++++++++++++++++
include/sysemu/block-backend.h | 10 ++++++++++
2 files changed, 27 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index 53fd9d2..2f8acbd 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -34,6 +34,7 @@ struct BlockBackend {
DriveInfo *legacy_dinfo; /* null unless created by drive_new() */
QTAILQ_ENTRY(BlockBackend) link; /* for block_backends */
QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends */
+ BlockBackendPublic public;
void *dev; /* attached device model, if any */
/* TODO change to DeviceState when all users are qdevified */
@@ -411,6 +412,22 @@ BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo)
}
/*
+ * Returns a pointer to the publicly accessible fields of @blk.
+ */
+BlockBackendPublic *blk_get_public(BlockBackend *blk)
+{
+ return &blk->public;
+}
+
+/*
+ * Returns a BlockBackend given the associated @public fields.
+ */
+BlockBackend *blk_by_public(BlockBackendPublic *public)
+{
+ return container_of(public, BlockBackend, public);
+}
+
+/*
* Disassociates the currently associated BlockDriverState from @blk.
*/
void blk_remove_bs(BlockBackend *blk)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 26736ed..a771603 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -59,6 +59,13 @@ typedef struct BlockDevOps {
void (*resize_cb)(void *opaque);
} BlockDevOps;
+/* This struct is embedded in (the private) BlockBackend struct and contains
+ * fields that must be public. This is in particular for QLIST_ENTRY() and
+ * friends so that BlockBackends can be kept in lists outside block-backend.c */
+typedef struct BlockBackendPublic {
+ int dummy; /* empty structs are illegal */
+} BlockBackendPublic;
+
BlockBackend *blk_new(Error **errp);
BlockBackend *blk_new_with_bs(Error **errp);
BlockBackend *blk_new_open(const char *filename, const char *reference,
@@ -74,6 +81,9 @@ BlockDriverState *blk_next_root_bs(BlockDriverState *bs);
bool monitor_add_blk(BlockBackend *blk, const char *name, Error **errp);
void monitor_remove_blk(BlockBackend *blk);
+BlockBackendPublic *blk_get_public(BlockBackend *blk);
+BlockBackend *blk_by_public(BlockBackendPublic *public);
+
BlockDriverState *blk_bs(BlockBackend *blk);
void blk_remove_bs(BlockBackend *blk);
void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 03/14] block: throttle-groups: Use BlockBackend pointers internally
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 01/14] block: Make sure throttled BDSes always have a BB Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 02/14] block: Introduce BlockBackendPublic Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 04/14] block: Convert throttle_group_get_name() to BlockBackend Kevin Wolf
` (11 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
As a first step towards moving I/O throttling to the BlockBackend level,
this patch changes all pointers in struct ThrottleGroup from referencing
a BlockDriverState to referencing a BlockBackend.
This change is valid because we made sure that throttling can only be
enabled on BDSes which have a BB attached.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/io.c | 4 +-
block/throttle-groups.c | 134 ++++++++++++++++++++--------------------
include/block/block_int.h | 1 -
include/block/throttle-groups.h | 4 +-
include/sysemu/block-backend.h | 5 +-
tests/test-throttle.c | 13 ++--
6 files changed, 83 insertions(+), 78 deletions(-)
diff --git a/block/io.c b/block/io.c
index cd6d71a..ede74c5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -70,7 +70,7 @@ void bdrv_io_limits_disable(BlockDriverState *bs)
{
assert(bs->throttle_state);
bdrv_no_throttling_begin(bs);
- throttle_group_unregister_bs(bs);
+ throttle_group_unregister_blk(bs->blk);
bdrv_no_throttling_end(bs);
}
@@ -78,7 +78,7 @@ void bdrv_io_limits_disable(BlockDriverState *bs)
void bdrv_io_limits_enable(BlockDriverState *bs, const char *group)
{
assert(!bs->throttle_state);
- throttle_group_register_bs(bs, group);
+ throttle_group_register_blk(bs->blk, group);
}
void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 9ac063a..1e6e2e5 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -23,6 +23,7 @@
*/
#include "qemu/osdep.h"
+#include "sysemu/block-backend.h"
#include "block/throttle-groups.h"
#include "qemu/queue.h"
#include "qemu/thread.h"
@@ -57,8 +58,8 @@ typedef struct ThrottleGroup {
QemuMutex lock; /* This lock protects the following four fields */
ThrottleState ts;
- QLIST_HEAD(, BlockDriverState) head;
- BlockDriverState *tokens[2];
+ QLIST_HEAD(, BlockBackendPublic) head;
+ BlockBackend *tokens[2];
bool any_timer_armed[2];
/* These two are protected by the global throttle_groups_lock */
@@ -145,81 +146,81 @@ const char *throttle_group_get_name(BlockDriverState *bs)
return tg->name;
}
-/* Return the next BlockDriverState in the round-robin sequence,
- * simulating a circular list.
+/* Return the next BlockBackend in the round-robin sequence, simulating a
+ * circular list.
*
* This assumes that tg->lock is held.
*
- * @bs: the current BlockDriverState
- * @ret: the next BlockDriverState in the sequence
+ * @blk: the current BlockBackend
+ * @ret: the next BlockBackend in the sequence
*/
-static BlockDriverState *throttle_group_next_bs(BlockDriverState *bs)
+static BlockBackend *throttle_group_next_blk(BlockBackend *blk)
{
+ BlockDriverState *bs = blk_bs(blk);
ThrottleState *ts = bs->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
- BlockDriverState *next = QLIST_NEXT(bs, round_robin);
+ BlockBackendPublic *next = QLIST_NEXT(blk_get_public(blk), round_robin);
if (!next) {
- return QLIST_FIRST(&tg->head);
+ next = QLIST_FIRST(&tg->head);
}
- return next;
+ return blk_by_public(next);
}
-/* Return the next BlockDriverState in the round-robin sequence with
- * pending I/O requests.
+/* Return the next BlockBackend in the round-robin sequence with pending I/O
+ * requests.
*
* This assumes that tg->lock is held.
*
- * @bs: the current BlockDriverState
+ * @blk: the current BlockBackend
* @is_write: the type of operation (read/write)
- * @ret: the next BlockDriverState with pending requests, or bs
- * if there is none.
+ * @ret: the next BlockBackend with pending requests, or blk if there is
+ * none.
*/
-static BlockDriverState *next_throttle_token(BlockDriverState *bs,
- bool is_write)
+static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write)
{
- ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
- BlockDriverState *token, *start;
+ ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
+ ThrottleGroup, ts);
+ BlockBackend *token, *start;
start = token = tg->tokens[is_write];
/* get next bs round in round robin style */
- token = throttle_group_next_bs(token);
- while (token != start && !token->pending_reqs[is_write]) {
- token = throttle_group_next_bs(token);
+ token = throttle_group_next_blk(token);
+ while (token != start && !blk_bs(token)->pending_reqs[is_write]) {
+ token = throttle_group_next_blk(token);
}
/* If no IO are queued for scheduling on the next round robin token
* then decide the token is the current bs because chances are
* the current bs get the current request queued.
*/
- if (token == start && !token->pending_reqs[is_write]) {
- token = bs;
+ if (token == start && !blk_bs(token)->pending_reqs[is_write]) {
+ token = blk;
}
return token;
}
-/* Check if the next I/O request for a BlockDriverState needs to be
- * throttled or not. If there's no timer set in this group, set one
- * and update the token accordingly.
+/* Check if the next I/O request for a BlockBackend needs to be throttled or
+ * not. If there's no timer set in this group, set one and update the token
+ * accordingly.
*
* This assumes that tg->lock is held.
*
- * @bs: the current BlockDriverState
+ * @blk: the current BlockBackend
* @is_write: the type of operation (read/write)
* @ret: whether the I/O request needs to be throttled or not
*/
-static bool throttle_group_schedule_timer(BlockDriverState *bs,
- bool is_write)
+static bool throttle_group_schedule_timer(BlockBackend *blk, bool is_write)
{
- ThrottleState *ts = bs->throttle_state;
- ThrottleTimers *tt = &bs->throttle_timers;
+ ThrottleState *ts = blk_bs(blk)->throttle_state;
+ ThrottleTimers *tt = &blk_bs(blk)->throttle_timers;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
bool must_wait;
- if (bs->io_limits_disabled) {
+ if (blk_bs(blk)->io_limits_disabled) {
return false;
}
@@ -230,9 +231,9 @@ static bool throttle_group_schedule_timer(BlockDriverState *bs,
must_wait = throttle_schedule_timer(ts, tt, is_write);
- /* If a timer just got armed, set bs as the current token */
+ /* If a timer just got armed, set blk as the current token */
if (must_wait) {
- tg->tokens[is_write] = bs;
+ tg->tokens[is_write] = blk;
tg->any_timer_armed[is_write] = true;
}
@@ -243,18 +244,19 @@ static bool throttle_group_schedule_timer(BlockDriverState *bs,
*
* This assumes that tg->lock is held.
*
- * @bs: the current BlockDriverState
+ * @blk: the current BlockBackend
* @is_write: the type of operation (read/write)
*/
-static void schedule_next_request(BlockDriverState *bs, bool is_write)
+static void schedule_next_request(BlockBackend *blk, bool is_write)
{
+ BlockDriverState *bs = blk_bs(blk);
ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
bool must_wait;
- BlockDriverState *token;
+ BlockBackend *token;
/* Check if there's any pending request to schedule next */
- token = next_throttle_token(bs, is_write);
- if (!token->pending_reqs[is_write]) {
+ token = next_throttle_token(blk, is_write);
+ if (!blk_bs(token)->pending_reqs[is_write]) {
return;
}
@@ -266,9 +268,9 @@ static void schedule_next_request(BlockDriverState *bs, bool is_write)
/* Give preference to requests from the current bs */
if (qemu_in_coroutine() &&
qemu_co_queue_next(&bs->throttled_reqs[is_write])) {
- token = bs;
+ token = blk;
} else {
- ThrottleTimers *tt = &token->throttle_timers;
+ ThrottleTimers *tt = &blk_bs(token)->throttle_timers;
int64_t now = qemu_clock_get_ns(tt->clock_type);
timer_mod(tt->timers[is_write], now + 1);
tg->any_timer_armed[is_write] = true;
@@ -290,13 +292,13 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
bool is_write)
{
bool must_wait;
- BlockDriverState *token;
+ BlockBackend *token;
ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
/* First we check if this I/O has to be throttled. */
- token = next_throttle_token(bs, is_write);
+ token = next_throttle_token(bs->blk, is_write);
must_wait = throttle_group_schedule_timer(token, is_write);
/* Wait if there's a timer set or queued requests of this type */
@@ -312,7 +314,7 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
throttle_account(bs->throttle_state, is_write, bytes);
/* Schedule the next request */
- schedule_next_request(bs, is_write);
+ schedule_next_request(bs->blk, is_write);
qemu_mutex_unlock(&tg->lock);
}
@@ -395,7 +397,7 @@ static void timer_cb(BlockDriverState *bs, bool is_write)
* scheduling the next one */
if (empty_queue) {
qemu_mutex_lock(&tg->lock);
- schedule_next_request(bs, is_write);
+ schedule_next_request(bs->blk, is_write);
qemu_mutex_unlock(&tg->lock);
}
}
@@ -410,17 +412,17 @@ static void write_timer_cb(void *opaque)
timer_cb(opaque, true);
}
-/* Register a BlockDriverState in the throttling group, also
- * initializing its timers and updating its throttle_state pointer to
- * point to it. If a throttling group with that name does not exist
- * yet, it will be created.
+/* Register a BlockBackend in the throttling group, also initializing its
+ * timers and updating its throttle_state pointer to point to it. If a
+ * throttling group with that name does not exist yet, it will be created.
*
- * @bs: the BlockDriverState to insert
+ * @blk: the BlockBackend to insert
* @groupname: the name of the group
*/
-void throttle_group_register_bs(BlockDriverState *bs, const char *groupname)
+void throttle_group_register_blk(BlockBackend *blk, const char *groupname)
{
int i;
+ BlockDriverState *bs = blk_bs(blk);
ThrottleState *ts = throttle_group_incref(groupname);
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
int clock_type = QEMU_CLOCK_REALTIME;
@@ -433,14 +435,14 @@ void throttle_group_register_bs(BlockDriverState *bs, const char *groupname)
bs->throttle_state = ts;
qemu_mutex_lock(&tg->lock);
- /* If the ThrottleGroup is new set this BlockDriverState as the token */
+ /* If the ThrottleGroup is new set this BlockBackend as the token */
for (i = 0; i < 2; i++) {
if (!tg->tokens[i]) {
- tg->tokens[i] = bs;
+ tg->tokens[i] = blk;
}
}
- QLIST_INSERT_HEAD(&tg->head, bs, round_robin);
+ QLIST_INSERT_HEAD(&tg->head, blk_get_public(blk), round_robin);
throttle_timers_init(&bs->throttle_timers,
bdrv_get_aio_context(bs),
@@ -452,19 +454,19 @@ void throttle_group_register_bs(BlockDriverState *bs, const char *groupname)
qemu_mutex_unlock(&tg->lock);
}
-/* Unregister a BlockDriverState from its group, removing it from the
- * list, destroying the timers and setting the throttle_state pointer
- * to NULL.
+/* Unregister a BlockBackend from its group, removing it from the list,
+ * destroying the timers and setting the throttle_state pointer to NULL.
*
- * The BlockDriverState must not have pending throttled requests, so
- * the caller has to drain them first.
+ * The BlockBackend must not have pending throttled requests, so the caller has
+ * to drain them first.
*
* The group will be destroyed if it's empty after this operation.
*
- * @bs: the BlockDriverState to remove
+ * @blk: the BlockBackend to remove
*/
-void throttle_group_unregister_bs(BlockDriverState *bs)
+void throttle_group_unregister_blk(BlockBackend *blk)
{
+ BlockDriverState *bs = blk_bs(blk);
ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
int i;
@@ -474,10 +476,10 @@ void throttle_group_unregister_bs(BlockDriverState *bs)
qemu_mutex_lock(&tg->lock);
for (i = 0; i < 2; i++) {
- if (tg->tokens[i] == bs) {
- BlockDriverState *token = throttle_group_next_bs(bs);
+ if (tg->tokens[i] == blk) {
+ BlockBackend *token = throttle_group_next_blk(blk);
/* Take care of the case where this is the last bs in the group */
- if (token == bs) {
+ if (token == blk) {
token = NULL;
}
tg->tokens[i] = token;
@@ -485,7 +487,7 @@ void throttle_group_unregister_bs(BlockDriverState *bs)
}
/* remove the current bs from the list */
- QLIST_REMOVE(bs, round_robin);
+ QLIST_REMOVE(blk_get_public(blk), round_robin);
throttle_timers_destroy(&bs->throttle_timers);
qemu_mutex_unlock(&tg->lock);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2709488..a51f317 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -430,7 +430,6 @@ struct BlockDriverState {
ThrottleState *throttle_state;
ThrottleTimers throttle_timers;
unsigned pending_reqs[2];
- QLIST_ENTRY(BlockDriverState) round_robin;
/* Offset after the highest byte written to */
uint64_t wr_highest_offset;
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index 395f72d..b9114ee 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -36,8 +36,8 @@ void throttle_group_unref(ThrottleState *ts);
void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg);
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_register_blk(BlockBackend *blk, const char *groupname);
+void throttle_group_unregister_blk(BlockBackend *blk);
void throttle_group_restart_bs(BlockDriverState *bs);
void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index a771603..1dcd70e 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -63,7 +63,10 @@ typedef struct BlockDevOps {
* fields that must be public. This is in particular for QLIST_ENTRY() and
* friends so that BlockBackends can be kept in lists outside block-backend.c */
typedef struct BlockBackendPublic {
- int dummy; /* empty structs are illegal */
+ /* I/O throttling */
+ /* The following fields are protected by the ThrottleGroup lock.
+ * See the ThrottleGroup documentation for details. */
+ QLIST_ENTRY(BlockBackendPublic) round_robin;
} BlockBackendPublic;
BlockBackend *blk_new(Error **errp);
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 77b95d6..beaa2a3 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -20,6 +20,7 @@
#include "qemu/throttle.h"
#include "qemu/error-report.h"
#include "block/throttle-groups.h"
+#include "sysemu/block-backend.h"
static AioContext *ctx;
static LeakyBucket bkt;
@@ -589,9 +590,9 @@ static void test_groups(void)
g_assert(bdrv2->throttle_state == NULL);
g_assert(bdrv3->throttle_state == NULL);
- throttle_group_register_bs(bdrv1, "bar");
- throttle_group_register_bs(bdrv2, "foo");
- throttle_group_register_bs(bdrv3, "bar");
+ throttle_group_register_blk(blk1, "bar");
+ throttle_group_register_blk(blk2, "foo");
+ throttle_group_register_blk(blk3, "bar");
g_assert(bdrv1->throttle_state != NULL);
g_assert(bdrv2->throttle_state != NULL);
@@ -623,9 +624,9 @@ static void test_groups(void)
throttle_group_get_config(bdrv3, &cfg2);
g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1)));
- throttle_group_unregister_bs(bdrv1);
- throttle_group_unregister_bs(bdrv2);
- throttle_group_unregister_bs(bdrv3);
+ throttle_group_unregister_blk(blk1);
+ throttle_group_unregister_blk(blk2);
+ throttle_group_unregister_blk(blk3);
g_assert(bdrv1->throttle_state == NULL);
g_assert(bdrv2->throttle_state == NULL);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 04/14] block: Convert throttle_group_get_name() to BlockBackend
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (2 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 03/14] block: throttle-groups: Use BlockBackend pointers internally Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 05/14] block: Move throttling fields from BDS to BB Kevin Wolf
` (10 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/block-backend.c | 2 +-
block/io.c | 2 +-
block/qapi.c | 2 +-
block/throttle-groups.c | 12 ++++++------
include/block/throttle-groups.h | 2 +-
tests/test-throttle.c | 4 ++--
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 2f8acbd..964a205 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1525,7 +1525,7 @@ void blk_update_root_state(BlockBackend *blk)
throttle_group_unref(blk->root_state.throttle_state);
}
if (blk->root->bs->throttle_state) {
- const char *name = throttle_group_get_name(blk->root->bs);
+ const char *name = throttle_group_get_name(blk);
blk->root_state.throttle_group = g_strdup(name);
blk->root_state.throttle_state = throttle_group_incref(name);
} else {
diff --git a/block/io.c b/block/io.c
index ede74c5..f6fb868 100644
--- a/block/io.c
+++ b/block/io.c
@@ -89,7 +89,7 @@ void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
}
/* this bs is a part of the same group than the one we want */
- if (!g_strcmp0(throttle_group_get_name(bs), group)) {
+ if (!g_strcmp0(throttle_group_get_name(bs->blk), group)) {
return;
}
diff --git a/block/qapi.c b/block/qapi.c
index c5f6ba6..a3e514d 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -118,7 +118,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
info->iops_size = cfg.op_size;
info->has_group = true;
- info->group = g_strdup(throttle_group_get_name(bs));
+ info->group = g_strdup(throttle_group_get_name(bs->blk));
}
info->write_threshold = bdrv_write_threshold_get(bs);
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 1e6e2e5..e50ccaa 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -133,16 +133,16 @@ void throttle_group_unref(ThrottleState *ts)
qemu_mutex_unlock(&throttle_groups_lock);
}
-/* Get the name from a BlockDriverState's ThrottleGroup. The name (and
- * the pointer) is guaranteed to remain constant during the lifetime
- * of the group.
+/* Get the name from a BlockBackend's ThrottleGroup. The name (and the pointer)
+ * is guaranteed to remain constant during the lifetime of the group.
*
- * @bs: a BlockDriverState that is member of a throttling group
+ * @blk: a BlockBackend that is member of a throttling group
* @ret: the name of the group.
*/
-const char *throttle_group_get_name(BlockDriverState *bs)
+const char *throttle_group_get_name(BlockBackend *blk)
{
- ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
+ ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
+ ThrottleGroup, ts);
return tg->name;
}
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index b9114ee..bd55a34 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -28,7 +28,7 @@
#include "qemu/throttle.h"
#include "block/block_int.h"
-const char *throttle_group_get_name(BlockDriverState *bs);
+const char *throttle_group_get_name(BlockBackend *blk);
ThrottleState *throttle_group_incref(const char *name);
void throttle_group_unref(ThrottleState *ts);
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index beaa2a3..1a322f1 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -598,8 +598,8 @@ static void test_groups(void)
g_assert(bdrv2->throttle_state != NULL);
g_assert(bdrv3->throttle_state != NULL);
- g_assert(!strcmp(throttle_group_get_name(bdrv1), "bar"));
- g_assert(!strcmp(throttle_group_get_name(bdrv2), "foo"));
+ g_assert(!strcmp(throttle_group_get_name(blk1), "bar"));
+ g_assert(!strcmp(throttle_group_get_name(blk2), "foo"));
g_assert(bdrv1->throttle_state == bdrv3->throttle_state);
/* Setting the config of a group member affects the whole group */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 05/14] block: Move throttling fields from BDS to BB
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (3 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 04/14] block: Convert throttle_group_get_name() to BlockBackend Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 06/14] block: Move actual I/O throttling to BlockBackend Kevin Wolf
` (9 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
This patch changes where the throttling state is stored (used to be the
BlockDriverState, now it is the BlockBackend), but it doesn't actually
make it a BB level feature yet. For example, throttling is still
disabled when the BDS is detached from the BB.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 22 +++----
block/block-backend.c | 13 ++--
block/io.c | 36 +++++++----
block/qapi.c | 2 +-
block/throttle-groups.c | 129 +++++++++++++++++++++-------------------
blockdev.c | 4 +-
include/block/block_int.h | 13 ----
include/block/throttle-groups.h | 2 +-
include/sysemu/block-backend.h | 11 +++-
tests/test-throttle.c | 28 +++++----
10 files changed, 142 insertions(+), 118 deletions(-)
diff --git a/block.c b/block.c
index 9bad665..0dc9a3e 100644
--- a/block.c
+++ b/block.c
@@ -237,8 +237,6 @@ BlockDriverState *bdrv_new(void)
QLIST_INIT(&bs->op_blockers[i]);
}
notifier_with_return_list_init(&bs->before_write_notifiers);
- qemu_co_queue_init(&bs->throttled_reqs[0]);
- qemu_co_queue_init(&bs->throttled_reqs[1]);
bs->refcnt = 1;
bs->aio_context = qemu_get_aio_context();
@@ -1525,7 +1523,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
return -ENODEV;
}
- if (bs->throttle_state) {
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
error_setg(errp, "Cannot reference an existing block device for "
"which I/O throttling is enabled");
return -EINVAL;
@@ -2124,7 +2122,7 @@ static void bdrv_close(BlockDriverState *bs)
assert(!bs->job);
/* Disable I/O limits and drain all pending throttled requests */
- if (bs->throttle_state) {
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
bdrv_io_limits_disable(bs);
}
@@ -2257,8 +2255,8 @@ static void swap_feature_fields(BlockDriverState *bs_top,
bdrv_move_feature_fields(bs_top, bs_new);
bdrv_move_feature_fields(bs_new, &tmp);
- assert(!bs_new->throttle_state);
- if (bs_top->throttle_state) {
+ assert(!bs_new->blk);
+ if (bs_top->blk && blk_get_public(bs_top->blk)->throttle_state) {
/*
* FIXME Need to break I/O throttling with graph manipulations
* temporarily because of conflicting invariants (3. will go away when
@@ -2300,11 +2298,11 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
assert(!bdrv_requests_pending(bs_new));
bdrv_ref(bs_top);
- change_parent_backing_link(bs_top, bs_new);
/* Some fields always stay on top of the backing file chain */
swap_feature_fields(bs_top, bs_new);
+ change_parent_backing_link(bs_top, bs_new);
bdrv_set_backing_hd(bs_new, bs_top);
bdrv_unref(bs_top);
@@ -3676,8 +3674,9 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
baf->detach_aio_context(baf->opaque);
}
- if (bs->throttle_state) {
- throttle_timers_detach_aio_context(&bs->throttle_timers);
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
+ throttle_timers_detach_aio_context(
+ &blk_get_public(bs->blk)->throttle_timers);
}
if (bs->drv->bdrv_detach_aio_context) {
bs->drv->bdrv_detach_aio_context(bs);
@@ -3712,8 +3711,9 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
if (bs->drv->bdrv_attach_aio_context) {
bs->drv->bdrv_attach_aio_context(bs, new_context);
}
- if (bs->throttle_state) {
- throttle_timers_attach_aio_context(&bs->throttle_timers, new_context);
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
+ throttle_timers_attach_aio_context(
+ &blk_get_public(bs->blk)->throttle_timers, new_context);
}
QLIST_FOREACH(ban, &bs->aio_notifiers, list) {
diff --git a/block/block-backend.c b/block/block-backend.c
index 964a205..6880659 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -107,8 +107,12 @@ BlockBackend *blk_new(Error **errp)
blk = g_new0(BlockBackend, 1);
blk->refcnt = 1;
+ qemu_co_queue_init(&blk->public.throttled_reqs[0]);
+ qemu_co_queue_init(&blk->public.throttled_reqs[1]);
+
notifier_list_init(&blk->remove_bs_notifiers);
notifier_list_init(&blk->insert_bs_notifiers);
+
QTAILQ_INSERT_TAIL(&block_backends, blk, link);
return blk;
}
@@ -437,7 +441,7 @@ void blk_remove_bs(BlockBackend *blk)
notifier_list_notify(&blk->remove_bs_notifiers, blk);
blk_update_root_state(blk);
- if (blk->root->bs->throttle_state) {
+ if (blk->public.throttle_state) {
bdrv_io_limits_disable(blk->root->bs);
}
@@ -795,7 +799,6 @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
int count)
{
- BlockDriverState *bs = blk_bs(blk);
int ret;
ret = blk_check_byte_request(blk, offset, count);
@@ -803,9 +806,9 @@ int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
return ret;
}
- bdrv_no_throttling_begin(bs);
+ bdrv_no_throttling_begin(blk_bs(blk));
ret = blk_pread(blk, offset, buf, count);
- bdrv_no_throttling_end(bs);
+ bdrv_no_throttling_end(blk_bs(blk));
return ret;
}
@@ -1524,7 +1527,7 @@ void blk_update_root_state(BlockBackend *blk)
g_free(blk->root_state.throttle_group);
throttle_group_unref(blk->root_state.throttle_state);
}
- if (blk->root->bs->throttle_state) {
+ if (blk->public.throttle_state) {
const char *name = throttle_group_get_name(blk);
blk->root_state.throttle_group = g_strdup(name);
blk->root_state.throttle_state = throttle_group_incref(name);
diff --git a/block/io.c b/block/io.c
index f6fb868..bdbaa1c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -55,20 +55,31 @@ void bdrv_set_io_limits(BlockDriverState *bs,
void bdrv_no_throttling_begin(BlockDriverState *bs)
{
- if (bs->io_limits_disabled++ == 0) {
- throttle_group_restart_bs(bs);
+ if (!bs->blk) {
+ return;
+ }
+
+ if (blk_get_public(bs->blk)->io_limits_disabled++ == 0) {
+ throttle_group_restart_blk(bs->blk);
}
}
void bdrv_no_throttling_end(BlockDriverState *bs)
{
- assert(bs->io_limits_disabled);
- --bs->io_limits_disabled;
+ BlockBackendPublic *blkp;
+
+ if (!bs->blk) {
+ return;
+ }
+
+ blkp = blk_get_public(bs->blk);
+ assert(blkp->io_limits_disabled);
+ --blkp->io_limits_disabled;
}
void bdrv_io_limits_disable(BlockDriverState *bs)
{
- assert(bs->throttle_state);
+ assert(blk_get_public(bs->blk)->throttle_state);
bdrv_no_throttling_begin(bs);
throttle_group_unregister_blk(bs->blk);
bdrv_no_throttling_end(bs);
@@ -77,14 +88,16 @@ void bdrv_io_limits_disable(BlockDriverState *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->throttle_state);
+ BlockBackendPublic *blkp = blk_get_public(bs->blk);
+
+ assert(!blkp->throttle_state);
throttle_group_register_blk(bs->blk, group);
}
void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
{
/* this bs is not part of any group */
- if (!bs->throttle_state) {
+ if (!blk_get_public(bs->blk)->throttle_state) {
return;
}
@@ -178,14 +191,15 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
bool bdrv_requests_pending(BlockDriverState *bs)
{
BdrvChild *child;
+ BlockBackendPublic *blkp = bs->blk ? blk_get_public(bs->blk) : NULL;
if (!QLIST_EMPTY(&bs->tracked_requests)) {
return true;
}
- if (!qemu_co_queue_empty(&bs->throttled_reqs[0])) {
+ if (blkp && !qemu_co_queue_empty(&blkp->throttled_reqs[0])) {
return true;
}
- if (!qemu_co_queue_empty(&bs->throttled_reqs[1])) {
+ if (blkp && !qemu_co_queue_empty(&blkp->throttled_reqs[1])) {
return true;
}
@@ -1070,7 +1084,7 @@ int coroutine_fn bdrv_co_preadv(BlockDriverState *bs,
}
/* throttling disk I/O */
- if (bs->throttle_state) {
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
throttle_group_co_io_limits_intercept(bs, bytes, false);
}
@@ -1431,7 +1445,7 @@ int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs,
}
/* throttling disk I/O */
- if (bs->throttle_state) {
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
throttle_group_co_io_limits_intercept(bs, bytes, true);
}
diff --git a/block/qapi.c b/block/qapi.c
index a3e514d..1e4bb8a 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -67,7 +67,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
info->backing_file_depth = bdrv_get_backing_file_depth(bs);
info->detect_zeroes = bs->detect_zeroes;
- if (bs->throttle_state) {
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
ThrottleConfig cfg;
throttle_group_get_config(bs, &cfg);
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index e50ccaa..56dc311 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -30,7 +30,7 @@
#include "sysemu/qtest.h"
/* The ThrottleGroup structure (with its ThrottleState) is shared
- * among different BlockDriverState and it's independent from
+ * among different BlockBackends and it's independent from
* AioContext, so in order to use it from different threads it needs
* its own locking.
*
@@ -40,18 +40,18 @@
* The whole ThrottleGroup structure is private and invisible to
* outside users, that only use it through its ThrottleState.
*
- * In addition to the ThrottleGroup structure, BlockDriverState has
+ * In addition to the ThrottleGroup structure, BlockBackendPublic has
* fields that need to be accessed by other members of the group and
- * therefore also need to be protected by this lock. Once a BDS is
- * registered in a group those fields can be accessed by other threads
- * any time.
+ * therefore also need to be protected by this lock. Once a
+ * BlockBackend is registered in a group those fields can be accessed
+ * by other threads any time.
*
* Again, all this is handled internally and is mostly transparent to
* the outside. The 'throttle_timers' field however has an additional
* constraint because it may be temporarily invalid (see for example
* bdrv_set_aio_context()). Therefore in this file a thread will
- * access some other BDS's timers only after verifying that that BDS
- * has throttled requests in the queue.
+ * access some other BlockBackend's timers only after verifying that
+ * that BlockBackend has throttled requests in the queue.
*/
typedef struct ThrottleGroup {
char *name; /* This is constant during the lifetime of the group */
@@ -141,8 +141,8 @@ void throttle_group_unref(ThrottleState *ts)
*/
const char *throttle_group_get_name(BlockBackend *blk)
{
- ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
- ThrottleGroup, ts);
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
return tg->name;
}
@@ -156,10 +156,10 @@ const char *throttle_group_get_name(BlockBackend *blk)
*/
static BlockBackend *throttle_group_next_blk(BlockBackend *blk)
{
- BlockDriverState *bs = blk_bs(blk);
- ThrottleState *ts = bs->throttle_state;
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
- BlockBackendPublic *next = QLIST_NEXT(blk_get_public(blk), round_robin);
+ BlockBackendPublic *next = QLIST_NEXT(blkp, round_robin);
if (!next) {
next = QLIST_FIRST(&tg->head);
@@ -180,15 +180,15 @@ static BlockBackend *throttle_group_next_blk(BlockBackend *blk)
*/
static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write)
{
- ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
- ThrottleGroup, ts);
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
BlockBackend *token, *start;
start = token = tg->tokens[is_write];
/* get next bs round in round robin style */
token = throttle_group_next_blk(token);
- while (token != start && !blk_bs(token)->pending_reqs[is_write]) {
+ while (token != start && !blkp->pending_reqs[is_write]) {
token = throttle_group_next_blk(token);
}
@@ -196,7 +196,7 @@ static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write)
* then decide the token is the current bs because chances are
* the current bs get the current request queued.
*/
- if (token == start && !blk_bs(token)->pending_reqs[is_write]) {
+ if (token == start && !blkp->pending_reqs[is_write]) {
token = blk;
}
@@ -215,12 +215,13 @@ static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write)
*/
static bool throttle_group_schedule_timer(BlockBackend *blk, bool is_write)
{
- ThrottleState *ts = blk_bs(blk)->throttle_state;
- ThrottleTimers *tt = &blk_bs(blk)->throttle_timers;
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleState *ts = blkp->throttle_state;
+ ThrottleTimers *tt = &blkp->throttle_timers;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
bool must_wait;
- if (blk_bs(blk)->io_limits_disabled) {
+ if (blkp->io_limits_disabled) {
return false;
}
@@ -249,14 +250,14 @@ static bool throttle_group_schedule_timer(BlockBackend *blk, bool is_write)
*/
static void schedule_next_request(BlockBackend *blk, bool is_write)
{
- BlockDriverState *bs = blk_bs(blk);
- ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
bool must_wait;
BlockBackend *token;
/* Check if there's any pending request to schedule next */
token = next_throttle_token(blk, is_write);
- if (!blk_bs(token)->pending_reqs[is_write]) {
+ if (!blkp->pending_reqs[is_write]) {
return;
}
@@ -265,12 +266,12 @@ static void schedule_next_request(BlockBackend *blk, bool is_write)
/* If it doesn't have to wait, queue it for immediate execution */
if (!must_wait) {
- /* Give preference to requests from the current bs */
+ /* Give preference to requests from the current blk */
if (qemu_in_coroutine() &&
- qemu_co_queue_next(&bs->throttled_reqs[is_write])) {
+ qemu_co_queue_next(&blkp->throttled_reqs[is_write])) {
token = blk;
} else {
- ThrottleTimers *tt = &blk_bs(token)->throttle_timers;
+ ThrottleTimers *tt = &blkp->throttle_timers;
int64_t now = qemu_clock_get_ns(tt->clock_type);
timer_mod(tt->timers[is_write], now + 1);
tg->any_timer_armed[is_write] = true;
@@ -294,37 +295,40 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
bool must_wait;
BlockBackend *token;
- ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
+ BlockBackend *blk = bs->blk;
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
/* First we check if this I/O has to be throttled. */
- token = next_throttle_token(bs->blk, is_write);
+ token = next_throttle_token(blk, is_write);
must_wait = throttle_group_schedule_timer(token, is_write);
/* Wait if there's a timer set or queued requests of this type */
- if (must_wait || bs->pending_reqs[is_write]) {
- bs->pending_reqs[is_write]++;
+ if (must_wait || blkp->pending_reqs[is_write]) {
+ blkp->pending_reqs[is_write]++;
qemu_mutex_unlock(&tg->lock);
- qemu_co_queue_wait(&bs->throttled_reqs[is_write]);
+ qemu_co_queue_wait(&blkp->throttled_reqs[is_write]);
qemu_mutex_lock(&tg->lock);
- bs->pending_reqs[is_write]--;
+ blkp->pending_reqs[is_write]--;
}
/* The I/O will be executed, so do the accounting */
- throttle_account(bs->throttle_state, is_write, bytes);
+ throttle_account(blkp->throttle_state, is_write, bytes);
/* Schedule the next request */
- schedule_next_request(bs->blk, is_write);
+ schedule_next_request(blk, is_write);
qemu_mutex_unlock(&tg->lock);
}
-void throttle_group_restart_bs(BlockDriverState *bs)
+void throttle_group_restart_blk(BlockBackend *blk)
{
+ BlockBackendPublic *blkp = blk_get_public(blk);
int i;
for (i = 0; i < 2; i++) {
- while (qemu_co_enter_next(&bs->throttled_reqs[i])) {
+ while (qemu_co_enter_next(&blkp->throttled_reqs[i])) {
;
}
}
@@ -339,8 +343,9 @@ void throttle_group_restart_bs(BlockDriverState *bs)
*/
void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
{
- ThrottleTimers *tt = &bs->throttle_timers;
- ThrottleState *ts = bs->throttle_state;
+ BlockBackendPublic *blkp = blk_get_public(bs->blk);
+ ThrottleTimers *tt = &blkp->throttle_timers;
+ ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
/* throttle_config() cancels the timers */
@@ -353,8 +358,8 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
throttle_config(ts, tt, cfg);
qemu_mutex_unlock(&tg->lock);
- qemu_co_enter_next(&bs->throttled_reqs[0]);
- qemu_co_enter_next(&bs->throttled_reqs[1]);
+ qemu_co_enter_next(&blkp->throttled_reqs[0]);
+ qemu_co_enter_next(&blkp->throttled_reqs[1]);
}
/* Get the throttle configuration from a particular group. Similar to
@@ -366,7 +371,8 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
*/
void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg)
{
- ThrottleState *ts = bs->throttle_state;
+ BlockBackendPublic *blkp = blk_get_public(bs->blk);
+ ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
throttle_get_config(ts, cfg);
@@ -376,12 +382,13 @@ void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg)
/* ThrottleTimers callback. This wakes up a request that was waiting
* because it had been throttled.
*
- * @bs: the BlockDriverState whose request had been throttled
+ * @blk: the BlockBackend whose request had been throttled
* @is_write: the type of operation (read/write)
*/
-static void timer_cb(BlockDriverState *bs, bool is_write)
+static void timer_cb(BlockBackend *blk, bool is_write)
{
- ThrottleState *ts = bs->throttle_state;
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
bool empty_queue;
@@ -391,13 +398,13 @@ static void timer_cb(BlockDriverState *bs, bool is_write)
qemu_mutex_unlock(&tg->lock);
/* Run the request that was waiting for this timer */
- empty_queue = !qemu_co_enter_next(&bs->throttled_reqs[is_write]);
+ empty_queue = !qemu_co_enter_next(&blkp->throttled_reqs[is_write]);
/* If the request queue was empty then we have to take care of
* scheduling the next one */
if (empty_queue) {
qemu_mutex_lock(&tg->lock);
- schedule_next_request(bs->blk, is_write);
+ schedule_next_request(blk, is_write);
qemu_mutex_unlock(&tg->lock);
}
}
@@ -422,7 +429,7 @@ static void write_timer_cb(void *opaque)
void throttle_group_register_blk(BlockBackend *blk, const char *groupname)
{
int i;
- BlockDriverState *bs = blk_bs(blk);
+ BlockBackendPublic *blkp = blk_get_public(blk);
ThrottleState *ts = throttle_group_incref(groupname);
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
int clock_type = QEMU_CLOCK_REALTIME;
@@ -432,7 +439,7 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname)
clock_type = QEMU_CLOCK_VIRTUAL;
}
- bs->throttle_state = ts;
+ blkp->throttle_state = ts;
qemu_mutex_lock(&tg->lock);
/* If the ThrottleGroup is new set this BlockBackend as the token */
@@ -442,14 +449,14 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname)
}
}
- QLIST_INSERT_HEAD(&tg->head, blk_get_public(blk), round_robin);
+ QLIST_INSERT_HEAD(&tg->head, blkp, round_robin);
- throttle_timers_init(&bs->throttle_timers,
- bdrv_get_aio_context(bs),
+ throttle_timers_init(&blkp->throttle_timers,
+ blk_get_aio_context(blk),
clock_type,
read_timer_cb,
write_timer_cb,
- bs);
+ blk);
qemu_mutex_unlock(&tg->lock);
}
@@ -466,19 +473,19 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname)
*/
void throttle_group_unregister_blk(BlockBackend *blk)
{
- BlockDriverState *bs = blk_bs(blk);
- ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
int i;
- assert(bs->pending_reqs[0] == 0 && bs->pending_reqs[1] == 0);
- assert(qemu_co_queue_empty(&bs->throttled_reqs[0]));
- assert(qemu_co_queue_empty(&bs->throttled_reqs[1]));
+ assert(blkp->pending_reqs[0] == 0 && blkp->pending_reqs[1] == 0);
+ assert(qemu_co_queue_empty(&blkp->throttled_reqs[0]));
+ assert(qemu_co_queue_empty(&blkp->throttled_reqs[1]));
qemu_mutex_lock(&tg->lock);
for (i = 0; i < 2; i++) {
if (tg->tokens[i] == blk) {
BlockBackend *token = throttle_group_next_blk(blk);
- /* Take care of the case where this is the last bs in the group */
+ /* Take care of the case where this is the last blk in the group */
if (token == blk) {
token = NULL;
}
@@ -486,13 +493,13 @@ void throttle_group_unregister_blk(BlockBackend *blk)
}
}
- /* remove the current bs from the list */
- QLIST_REMOVE(blk_get_public(blk), round_robin);
- throttle_timers_destroy(&bs->throttle_timers);
+ /* remove the current blk from the list */
+ QLIST_REMOVE(blkp, round_robin);
+ throttle_timers_destroy(&blkp->throttle_timers);
qemu_mutex_unlock(&tg->lock);
throttle_group_unref(&tg->ts);
- bs->throttle_state = NULL;
+ blkp->throttle_state = NULL;
}
static void throttle_groups_init(void)
diff --git a/blockdev.c b/blockdev.c
index 585a28a..878c697 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2726,14 +2726,14 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
if (throttle_enabled(&cfg)) {
/* Enable I/O limits if they're not enabled yet, otherwise
* just update the throttling group. */
- if (!bs->throttle_state) {
+ if (!blk_get_public(bs->blk)->throttle_state) {
bdrv_io_limits_enable(bs, has_group ? group : device);
} else if (has_group) {
bdrv_io_limits_update_group(bs, group);
}
/* Set the new throttling configuration */
bdrv_set_io_limits(bs, &cfg);
- } else if (bs->throttle_state) {
+ } else if (blk_get_public(bs->blk)->throttle_state) {
/* If all throttling settings are set to 0, disable I/O limits */
bdrv_io_limits_disable(bs);
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a51f317..ddd8371 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -26,7 +26,6 @@
#include "block/accounting.h"
#include "block/block.h"
-#include "block/throttle-groups.h"
#include "qemu/option.h"
#include "qemu/queue.h"
#include "qemu/coroutine.h"
@@ -419,18 +418,6 @@ struct BlockDriverState {
/* number of in-flight serialising requests */
unsigned int serialising_in_flight;
- /* I/O throttling.
- * throttle_state tells us if this BDS has I/O limits configured.
- * io_limits_disabled tells us if they are currently being enforced */
- CoQueue throttled_reqs[2];
- unsigned int io_limits_disabled;
-
- /* The following fields are protected by the ThrottleGroup lock.
- * See the ThrottleGroup documentation for details. */
- ThrottleState *throttle_state;
- ThrottleTimers throttle_timers;
- unsigned pending_reqs[2];
-
/* Offset after the highest byte written to */
uint64_t wr_highest_offset;
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index bd55a34..840ba44 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -38,7 +38,7 @@ void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg);
void throttle_group_register_blk(BlockBackend *blk, const char *groupname);
void throttle_group_unregister_blk(BlockBackend *blk);
-void throttle_group_restart_bs(BlockDriverState *bs);
+void throttle_group_restart_blk(BlockBackend *blk);
void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
unsigned int bytes,
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 1dcd70e..08d27a8 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -14,6 +14,7 @@
#define BLOCK_BACKEND_H
#include "qemu/iov.h"
+#include "block/throttle-groups.h"
/*
* TODO Have to include block/block.h for a bunch of block layer
@@ -63,9 +64,17 @@ typedef struct BlockDevOps {
* fields that must be public. This is in particular for QLIST_ENTRY() and
* friends so that BlockBackends can be kept in lists outside block-backend.c */
typedef struct BlockBackendPublic {
- /* I/O throttling */
+ /* I/O throttling.
+ * throttle_state tells us if this BlockBackend has I/O limits configured.
+ * io_limits_disabled tells us if they are currently being enforced */
+ CoQueue throttled_reqs[2];
+ unsigned int io_limits_disabled;
+
/* The following fields are protected by the ThrottleGroup lock.
* See the ThrottleGroup documentation for details. */
+ ThrottleState *throttle_state;
+ ThrottleTimers throttle_timers;
+ unsigned pending_reqs[2];
QLIST_ENTRY(BlockBackendPublic) round_robin;
} BlockBackendPublic;
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 1a322f1..a020068 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -576,31 +576,35 @@ static void test_groups(void)
{
ThrottleConfig cfg1, cfg2;
BlockBackend *blk1, *blk2, *blk3;
- BlockDriverState *bdrv1, *bdrv2, *bdrv3;
+ BlockBackendPublic *blkp1, *blkp2, *blkp3;
+ BlockDriverState *bdrv1, *bdrv3;
blk1 = blk_new_with_bs(&error_abort);
blk2 = blk_new_with_bs(&error_abort);
blk3 = blk_new_with_bs(&error_abort);
bdrv1 = blk_bs(blk1);
- bdrv2 = blk_bs(blk2);
bdrv3 = blk_bs(blk3);
- g_assert(bdrv1->throttle_state == NULL);
- g_assert(bdrv2->throttle_state == NULL);
- g_assert(bdrv3->throttle_state == NULL);
+ blkp1 = blk_get_public(blk1);
+ blkp2 = blk_get_public(blk2);
+ blkp3 = blk_get_public(blk3);
+
+ g_assert(blkp1->throttle_state == NULL);
+ g_assert(blkp2->throttle_state == NULL);
+ g_assert(blkp3->throttle_state == NULL);
throttle_group_register_blk(blk1, "bar");
throttle_group_register_blk(blk2, "foo");
throttle_group_register_blk(blk3, "bar");
- g_assert(bdrv1->throttle_state != NULL);
- g_assert(bdrv2->throttle_state != NULL);
- g_assert(bdrv3->throttle_state != NULL);
+ g_assert(blkp1->throttle_state != NULL);
+ g_assert(blkp2->throttle_state != NULL);
+ g_assert(blkp3->throttle_state != NULL);
g_assert(!strcmp(throttle_group_get_name(blk1), "bar"));
g_assert(!strcmp(throttle_group_get_name(blk2), "foo"));
- g_assert(bdrv1->throttle_state == bdrv3->throttle_state);
+ g_assert(blkp1->throttle_state == blkp3->throttle_state);
/* Setting the config of a group member affects the whole group */
throttle_config_init(&cfg1);
@@ -628,9 +632,9 @@ static void test_groups(void)
throttle_group_unregister_blk(blk2);
throttle_group_unregister_blk(blk3);
- g_assert(bdrv1->throttle_state == NULL);
- g_assert(bdrv2->throttle_state == NULL);
- g_assert(bdrv3->throttle_state == NULL);
+ g_assert(blkp1->throttle_state == NULL);
+ g_assert(blkp2->throttle_state == NULL);
+ g_assert(blkp3->throttle_state == NULL);
}
int main(int argc, char **argv)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 06/14] block: Move actual I/O throttling to BlockBackend
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (4 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 05/14] block: Move throttling fields from BDS to BB Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 07/14] block: Move I/O throttling configuration functions " Kevin Wolf
` (8 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/block-backend.c | 10 ++++++++++
block/io.c | 10 ----------
block/throttle-groups.c | 5 ++---
include/block/throttle-groups.h | 2 +-
4 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 6880659..730b8a9 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -716,6 +716,11 @@ static int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
return ret;
}
+ /* throttling disk I/O */
+ if (blk->public.throttle_state) {
+ throttle_group_co_io_limits_intercept(blk, bytes, false);
+ }
+
return bdrv_co_preadv(blk_bs(blk), offset, bytes, qiov, flags);
}
@@ -730,6 +735,11 @@ static int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
return ret;
}
+ /* throttling disk I/O */
+ if (blk->public.throttle_state) {
+ throttle_group_co_io_limits_intercept(blk, bytes, true);
+ }
+
if (!blk->enable_write_cache) {
flags |= BDRV_REQ_FUA;
}
diff --git a/block/io.c b/block/io.c
index bdbaa1c..cf2ac4c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1083,11 +1083,6 @@ int coroutine_fn bdrv_co_preadv(BlockDriverState *bs,
flags |= BDRV_REQ_COPY_ON_READ;
}
- /* throttling disk I/O */
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- throttle_group_co_io_limits_intercept(bs, bytes, false);
- }
-
/* Align read if necessary by padding qiov */
if (offset & (align - 1)) {
head_buf = qemu_blockalign(bs, align);
@@ -1444,11 +1439,6 @@ int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs,
return ret;
}
- /* throttling disk I/O */
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- throttle_group_co_io_limits_intercept(bs, bytes, true);
- }
-
/*
* Align write if necessary by performing a read-modify-write cycle.
* Pad qiov with the read parts and be sure to have a tracked request not
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 56dc311..3db8cf7 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -284,18 +284,17 @@ static void schedule_next_request(BlockBackend *blk, bool is_write)
* if necessary, and schedule the next request using a round robin
* algorithm.
*
- * @bs: the current BlockDriverState
+ * @blk: the current BlockBackend
* @bytes: the number of bytes for this I/O
* @is_write: the type of operation (read/write)
*/
-void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
+void coroutine_fn throttle_group_co_io_limits_intercept(BlockBackend *blk,
unsigned int bytes,
bool is_write)
{
bool must_wait;
BlockBackend *token;
- BlockBackend *blk = bs->blk;
BlockBackendPublic *blkp = blk_get_public(blk);
ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index 840ba44..ac42248 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -40,7 +40,7 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname);
void throttle_group_unregister_blk(BlockBackend *blk);
void throttle_group_restart_blk(BlockBackend *blk);
-void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
+void coroutine_fn throttle_group_co_io_limits_intercept(BlockBackend *blk,
unsigned int bytes,
bool is_write);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 07/14] block: Move I/O throttling configuration functions to BlockBackend
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (5 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 06/14] block: Move actual I/O throttling to BlockBackend Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 08/14] block: Introduce BdrvChild.opaque Kevin Wolf
` (7 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block.c | 2 +-
block/block-backend.c | 43 +++++++++++++++++++++++++++++++++++++++--
block/io.c | 41 ---------------------------------------
block/qapi.c | 2 +-
block/throttle-groups.c | 12 ++++++------
blockdev.c | 16 +++++++--------
include/block/block.h | 4 ----
include/block/block_int.h | 3 +--
include/block/throttle-groups.h | 4 ++--
include/sysemu/block-backend.h | 5 +++++
tests/test-throttle.c | 16 ++++++---------
11 files changed, 71 insertions(+), 77 deletions(-)
diff --git a/block.c b/block.c
index 0dc9a3e..18dfe97 100644
--- a/block.c
+++ b/block.c
@@ -2123,7 +2123,7 @@ static void bdrv_close(BlockDriverState *bs)
/* Disable I/O limits and drain all pending throttled requests */
if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- bdrv_io_limits_disable(bs);
+ blk_io_limits_disable(bs->blk);
}
bdrv_drained_begin(bs); /* complete I/O */
diff --git a/block/block-backend.c b/block/block-backend.c
index 730b8a9..b9aaa60 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -442,7 +442,7 @@ void blk_remove_bs(BlockBackend *blk)
blk_update_root_state(blk);
if (blk->public.throttle_state) {
- bdrv_io_limits_disable(blk->root->bs);
+ blk_io_limits_disable(blk);
}
blk->root->bs->blk = NULL;
@@ -1556,7 +1556,7 @@ void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs)
{
bs->detect_zeroes = blk->root_state.detect_zeroes;
if (blk->root_state.throttle_group) {
- bdrv_io_limits_enable(bs, blk->root_state.throttle_group);
+ blk_io_limits_enable(blk, blk->root_state.throttle_group);
}
}
@@ -1620,3 +1620,42 @@ int blk_flush_all(void)
return result;
}
+
+
+/* throttling disk I/O limits */
+void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg)
+{
+ throttle_group_config(blk, cfg);
+}
+
+void blk_io_limits_disable(BlockBackend *blk)
+{
+ assert(blk->public.throttle_state);
+ bdrv_no_throttling_begin(blk_bs(blk));
+ throttle_group_unregister_blk(blk);
+ bdrv_no_throttling_end(blk_bs(blk));
+}
+
+/* should be called before blk_set_io_limits if a limit is set */
+void blk_io_limits_enable(BlockBackend *blk, const char *group)
+{
+ assert(!blk->public.throttle_state);
+ throttle_group_register_blk(blk, group);
+}
+
+void blk_io_limits_update_group(BlockBackend *blk, const char *group)
+{
+ /* this BB is not part of any group */
+ if (!blk->public.throttle_state) {
+ return;
+ }
+
+ /* this BB is a part of the same group than the one we want */
+ if (!g_strcmp0(throttle_group_get_name(blk), group)) {
+ return;
+ }
+
+ /* need to change the group this bs belong to */
+ blk_io_limits_disable(blk);
+ blk_io_limits_enable(blk, group);
+}
diff --git a/block/io.c b/block/io.c
index cf2ac4c..1699f1e 100644
--- a/block/io.c
+++ b/block/io.c
@@ -46,13 +46,6 @@ static void coroutine_fn bdrv_co_do_rw(void *opaque);
static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, BdrvRequestFlags flags);
-/* throttling disk I/O limits */
-void bdrv_set_io_limits(BlockDriverState *bs,
- ThrottleConfig *cfg)
-{
- throttle_group_config(bs, cfg);
-}
-
void bdrv_no_throttling_begin(BlockDriverState *bs)
{
if (!bs->blk) {
@@ -77,40 +70,6 @@ void bdrv_no_throttling_end(BlockDriverState *bs)
--blkp->io_limits_disabled;
}
-void bdrv_io_limits_disable(BlockDriverState *bs)
-{
- assert(blk_get_public(bs->blk)->throttle_state);
- bdrv_no_throttling_begin(bs);
- throttle_group_unregister_blk(bs->blk);
- 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)
-{
- BlockBackendPublic *blkp = blk_get_public(bs->blk);
-
- assert(!blkp->throttle_state);
- throttle_group_register_blk(bs->blk, group);
-}
-
-void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
-{
- /* this bs is not part of any group */
- if (!blk_get_public(bs->blk)->throttle_state) {
- return;
- }
-
- /* this bs is a part of the same group than the one we want */
- if (!g_strcmp0(throttle_group_get_name(bs->blk), group)) {
- return;
- }
-
- /* need to change the group this bs belong to */
- bdrv_io_limits_disable(bs);
- bdrv_io_limits_enable(bs, group);
-}
-
void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
{
BlockDriver *drv = bs->drv;
diff --git a/block/qapi.c b/block/qapi.c
index 1e4bb8a..b0d8d6b 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -70,7 +70,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
ThrottleConfig cfg;
- throttle_group_get_config(bs, &cfg);
+ throttle_group_get_config(bs->blk, &cfg);
info->bps = cfg.buckets[THROTTLE_BPS_TOTAL].avg;
info->bps_rd = cfg.buckets[THROTTLE_BPS_READ].avg;
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 3db8cf7..59545e2 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -337,12 +337,12 @@ void throttle_group_restart_blk(BlockBackend *blk)
* to throttle_config(), but guarantees atomicity within the
* throttling group.
*
- * @bs: a BlockDriverState that is member of the group
+ * @blk: a BlockBackend that is a member of the group
* @cfg: the configuration to set
*/
-void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
+void throttle_group_config(BlockBackend *blk, ThrottleConfig *cfg)
{
- BlockBackendPublic *blkp = blk_get_public(bs->blk);
+ BlockBackendPublic *blkp = blk_get_public(blk);
ThrottleTimers *tt = &blkp->throttle_timers;
ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
@@ -365,12 +365,12 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
* throttle_get_config(), but guarantees atomicity within the
* throttling group.
*
- * @bs: a BlockDriverState that is member of the group
+ * @blk: a BlockBackend that is a member of the group
* @cfg: the configuration will be written here
*/
-void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg)
+void throttle_group_get_config(BlockBackend *blk, ThrottleConfig *cfg)
{
- BlockBackendPublic *blkp = blk_get_public(bs->blk);
+ BlockBackendPublic *blkp = blk_get_public(blk);
ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
diff --git a/blockdev.c b/blockdev.c
index 878c697..e383710 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -616,8 +616,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
if (!throttling_group) {
throttling_group = blk_name(blk);
}
- bdrv_io_limits_enable(bs, throttling_group);
- bdrv_set_io_limits(bs, &cfg);
+ blk_io_limits_enable(blk, throttling_group);
+ blk_set_io_limits(blk, &cfg);
}
if (bdrv_key_required(bs)) {
@@ -2726,16 +2726,16 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
if (throttle_enabled(&cfg)) {
/* Enable I/O limits if they're not enabled yet, otherwise
* just update the throttling group. */
- if (!blk_get_public(bs->blk)->throttle_state) {
- bdrv_io_limits_enable(bs, has_group ? group : device);
+ if (!blk_get_public(blk)->throttle_state) {
+ blk_io_limits_enable(blk, has_group ? group : device);
} else if (has_group) {
- bdrv_io_limits_update_group(bs, group);
+ blk_io_limits_update_group(blk, group);
}
/* Set the new throttling configuration */
- bdrv_set_io_limits(bs, &cfg);
- } else if (blk_get_public(bs->blk)->throttle_state) {
+ blk_set_io_limits(blk, &cfg);
+ } else if (blk_get_public(blk)->throttle_state) {
/* If all throttling settings are set to 0, disable I/O limits */
- bdrv_io_limits_disable(bs);
+ blk_io_limits_disable(blk);
}
out:
diff --git a/include/block/block.h b/include/block/block.h
index 0e8b4d1..4b04550 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -187,10 +187,6 @@ void bdrv_stats_print(Monitor *mon, const QObject *data);
void bdrv_info_stats(Monitor *mon, QObject **ret_data);
/* disk I/O throttling */
-void bdrv_io_limits_enable(BlockDriverState *bs, const char *group);
-void bdrv_io_limits_disable(BlockDriverState *bs);
-void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group);
-
void bdrv_init(void);
void bdrv_init_with_whitelist(void);
bool bdrv_uses_whitelist(void);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ddd8371..65f716d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -520,8 +520,7 @@ int get_tmp_filename(char *filename, int size);
BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
const char *filename);
-void bdrv_set_io_limits(BlockDriverState *bs,
- ThrottleConfig *cfg);
+bool bdrv_start_throttled_reqs(BlockDriverState *bs);
/**
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index ac42248..d983d34 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -33,8 +33,8 @@ const char *throttle_group_get_name(BlockBackend *blk);
ThrottleState *throttle_group_incref(const char *name);
void throttle_group_unref(ThrottleState *ts);
-void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg);
-void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg);
+void throttle_group_config(BlockBackend *blk, ThrottleConfig *cfg);
+void throttle_group_get_config(BlockBackend *blk, ThrottleConfig *cfg);
void throttle_group_register_blk(BlockBackend *blk, const char *groupname);
void throttle_group_unregister_blk(BlockBackend *blk);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 08d27a8..dd9c8ca 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -212,4 +212,9 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk,
BlockCompletionFunc *cb,
void *opaque, int ret);
+void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg);
+void blk_io_limits_disable(BlockBackend *blk);
+void blk_io_limits_enable(BlockBackend *blk, const char *group);
+void blk_io_limits_update_group(BlockBackend *blk, const char *group);
+
#endif
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index a020068..5ec966c 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -577,15 +577,11 @@ static void test_groups(void)
ThrottleConfig cfg1, cfg2;
BlockBackend *blk1, *blk2, *blk3;
BlockBackendPublic *blkp1, *blkp2, *blkp3;
- BlockDriverState *bdrv1, *bdrv3;
blk1 = blk_new_with_bs(&error_abort);
blk2 = blk_new_with_bs(&error_abort);
blk3 = blk_new_with_bs(&error_abort);
- bdrv1 = blk_bs(blk1);
- bdrv3 = blk_bs(blk3);
-
blkp1 = blk_get_public(blk1);
blkp2 = blk_get_public(blk2);
blkp3 = blk_get_public(blk3);
@@ -612,20 +608,20 @@ static void test_groups(void)
cfg1.buckets[THROTTLE_BPS_WRITE].avg = 285000;
cfg1.buckets[THROTTLE_OPS_READ].avg = 20000;
cfg1.buckets[THROTTLE_OPS_WRITE].avg = 12000;
- throttle_group_config(bdrv1, &cfg1);
+ throttle_group_config(blk1, &cfg1);
- throttle_group_get_config(bdrv1, &cfg1);
- throttle_group_get_config(bdrv3, &cfg2);
+ throttle_group_get_config(blk1, &cfg1);
+ throttle_group_get_config(blk3, &cfg2);
g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1)));
cfg2.buckets[THROTTLE_BPS_READ].avg = 4547;
cfg2.buckets[THROTTLE_BPS_WRITE].avg = 1349;
cfg2.buckets[THROTTLE_OPS_READ].avg = 123;
cfg2.buckets[THROTTLE_OPS_WRITE].avg = 86;
- throttle_group_config(bdrv3, &cfg1);
+ throttle_group_config(blk3, &cfg1);
- throttle_group_get_config(bdrv1, &cfg1);
- throttle_group_get_config(bdrv3, &cfg2);
+ throttle_group_get_config(blk1, &cfg1);
+ throttle_group_get_config(blk3, &cfg2);
g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1)));
throttle_group_unregister_blk(blk1);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 08/14] block: Introduce BdrvChild.opaque
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (6 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 07/14] block: Move I/O throttling configuration functions " Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 09/14] block: Drain throttling queue with BdrvChild callback Kevin Wolf
` (6 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
BlockBackends use it to get a back pointer from BdrvChild to
BlockBackend in any BdrvChildRole callbacks.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/block-backend.c | 2 ++
include/block/block_int.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index b9aaa60..52b7b92 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -133,6 +133,7 @@ BlockBackend *blk_new_with_bs(Error **errp)
bs = bdrv_new_root();
blk->root = bdrv_root_attach_child(bs, "root", &child_root);
+ blk->root->opaque = blk;
bs->blk = blk;
return blk;
}
@@ -458,6 +459,7 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
assert(!blk->root && !bs->blk);
bdrv_ref(bs);
blk->root = bdrv_root_attach_child(bs, "root", &child_root);
+ blk->root->opaque = blk;
bs->blk = blk;
notifier_list_notify(&blk->insert_bs_notifiers, blk);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 65f716d..6af541e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -368,6 +368,7 @@ struct BdrvChild {
BlockDriverState *bs;
char *name;
const BdrvChildRole *role;
+ void *opaque;
QLIST_ENTRY(BdrvChild) next;
QLIST_ENTRY(BdrvChild) next_parent;
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 09/14] block: Drain throttling queue with BdrvChild callback
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (7 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 08/14] block: Introduce BdrvChild.opaque Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-16 22:45 ` Stefan Hajnoczi
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 10/14] block/io: Quiesce parents between drained_begin/end Kevin Wolf
` (5 subsequent siblings)
14 siblings, 1 reply; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
This removes the last part of I/O throttling from block/io.c and moves
it to the BlockBackend.
Instead of having knowledge about throttling inside io.c, we can call a
BdrvChild callback .drained_begin/end, which happens to drain the
throttled requests for BlockBackend parents.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/block-backend.c | 32 +++++++++++++++++++++++++++-----
block/io.c | 39 ++++++++++++++++++---------------------
include/block/block_int.h | 16 +++++++++++-----
3 files changed, 56 insertions(+), 31 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 52b7b92..74429ae 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -91,9 +91,14 @@ static void blk_root_inherit_options(int *child_flags, QDict *child_options,
/* We're not supposed to call this function for root nodes */
abort();
}
+static void blk_root_drained_begin(BdrvChild *child);
+static void blk_root_drained_end(BdrvChild *child);
static const BdrvChildRole child_root = {
- .inherit_options = blk_root_inherit_options,
+ .inherit_options = blk_root_inherit_options,
+
+ .drained_begin = blk_root_drained_begin,
+ .drained_end = blk_root_drained_end,
};
/*
@@ -818,9 +823,9 @@ int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
return ret;
}
- bdrv_no_throttling_begin(blk_bs(blk));
+ blk_root_drained_begin(blk->root);
ret = blk_pread(blk, offset, buf, count);
- bdrv_no_throttling_end(blk_bs(blk));
+ blk_root_drained_end(blk->root);
return ret;
}
@@ -1633,9 +1638,9 @@ void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg)
void blk_io_limits_disable(BlockBackend *blk)
{
assert(blk->public.throttle_state);
- bdrv_no_throttling_begin(blk_bs(blk));
+ bdrv_drained_begin(blk_bs(blk));
throttle_group_unregister_blk(blk);
- bdrv_no_throttling_end(blk_bs(blk));
+ bdrv_drained_end(blk_bs(blk));
}
/* should be called before blk_set_io_limits if a limit is set */
@@ -1661,3 +1666,20 @@ void blk_io_limits_update_group(BlockBackend *blk, const char *group)
blk_io_limits_disable(blk);
blk_io_limits_enable(blk, group);
}
+
+static void blk_root_drained_begin(BdrvChild *child)
+{
+ BlockBackend *blk = child->opaque;
+
+ if (blk->public.io_limits_disabled++ == 0) {
+ throttle_group_restart_blk(blk);
+ }
+}
+
+static void blk_root_drained_end(BdrvChild *child)
+{
+ BlockBackend *blk = child->opaque;
+
+ assert(blk->public.io_limits_disabled);
+ --blk->public.io_limits_disabled;
+}
diff --git a/block/io.c b/block/io.c
index 1699f1e..7c213ec 100644
--- a/block/io.c
+++ b/block/io.c
@@ -27,7 +27,6 @@
#include "sysemu/block-backend.h"
#include "block/blockjob.h"
#include "block/block_int.h"
-#include "block/throttle-groups.h"
#include "qemu/cutils.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
@@ -46,28 +45,26 @@ static void coroutine_fn bdrv_co_do_rw(void *opaque);
static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, BdrvRequestFlags flags);
-void bdrv_no_throttling_begin(BlockDriverState *bs)
+static void bdrv_parent_drained_begin(BlockDriverState *bs)
{
- if (!bs->blk) {
- return;
- }
+ BdrvChild *c;
- if (blk_get_public(bs->blk)->io_limits_disabled++ == 0) {
- throttle_group_restart_blk(bs->blk);
+ QLIST_FOREACH(c, &bs->parents, next_parent) {
+ if (c->role->drained_begin) {
+ c->role->drained_begin(c);
+ }
}
}
-void bdrv_no_throttling_end(BlockDriverState *bs)
+static void bdrv_parent_drained_end(BlockDriverState *bs)
{
- BlockBackendPublic *blkp;
+ BdrvChild *c;
- if (!bs->blk) {
- return;
+ QLIST_FOREACH(c, &bs->parents, next_parent) {
+ if (c->role->drained_end) {
+ c->role->drained_end(c);
+ }
}
-
- blkp = blk_get_public(bs->blk);
- assert(blkp->io_limits_disabled);
- --blkp->io_limits_disabled;
}
void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
@@ -248,17 +245,17 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
*/
void coroutine_fn bdrv_co_drain(BlockDriverState *bs)
{
- bdrv_no_throttling_begin(bs);
+ bdrv_parent_drained_begin(bs);
bdrv_io_unplugged_begin(bs);
bdrv_drain_recurse(bs);
bdrv_co_yield_to_drain(bs);
bdrv_io_unplugged_end(bs);
- bdrv_no_throttling_end(bs);
+ bdrv_parent_drained_end(bs);
}
void bdrv_drain(BlockDriverState *bs)
{
- bdrv_no_throttling_begin(bs);
+ bdrv_parent_drained_begin(bs);
bdrv_io_unplugged_begin(bs);
bdrv_drain_recurse(bs);
if (qemu_in_coroutine()) {
@@ -267,7 +264,7 @@ void bdrv_drain(BlockDriverState *bs)
bdrv_drain_poll(bs);
}
bdrv_io_unplugged_end(bs);
- bdrv_no_throttling_end(bs);
+ bdrv_parent_drained_end(bs);
}
/*
@@ -290,7 +287,7 @@ void bdrv_drain_all(void)
if (bs->job) {
block_job_pause(bs->job);
}
- bdrv_no_throttling_begin(bs);
+ bdrv_parent_drained_begin(bs);
bdrv_io_unplugged_begin(bs);
bdrv_drain_recurse(bs);
aio_context_release(aio_context);
@@ -333,7 +330,7 @@ void bdrv_drain_all(void)
aio_context_acquire(aio_context);
bdrv_io_unplugged_end(bs);
- bdrv_no_throttling_end(bs);
+ bdrv_parent_drained_end(bs);
if (bs->job) {
block_job_resume(bs->job);
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 6af541e..461583b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -359,6 +359,17 @@ typedef struct BdrvAioNotifier {
struct BdrvChildRole {
void (*inherit_options)(int *child_flags, QDict *child_options,
int parent_flags, QDict *parent_options);
+
+ /*
+ * If this pair of functions is implemented, the parent doesn't issue new
+ * requests after returning from .drained_being() until .drained_end() is
+ * called.
+ *
+ * Note that this can be nested. If drained_begin() was called twice, new
+ * I/O is allowed only after drained_end() was called twice, too.
+ */
+ void (*drained_begin)(BdrvChild *child);
+ void (*drained_end)(BdrvChild *child);
};
extern const BdrvChildRole child_file;
@@ -521,8 +532,6 @@ int get_tmp_filename(char *filename, int size);
BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
const char *filename);
-bool bdrv_start_throttled_reqs(BlockDriverState *bs);
-
/**
* bdrv_add_before_write_notifier:
@@ -705,9 +714,6 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
const BdrvChildRole *child_role);
void bdrv_root_unref_child(BdrvChild *child);
-void bdrv_no_throttling_begin(BlockDriverState *bs);
-void bdrv_no_throttling_end(BlockDriverState *bs);
-
void blk_dev_change_media_cb(BlockBackend *blk, bool load);
bool blk_dev_has_removable_media(BlockBackend *blk);
bool blk_dev_has_tray(BlockBackend *blk);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 10/14] block/io: Quiesce parents between drained_begin/end
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (8 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 09/14] block: Drain throttling queue with BdrvChild callback Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-16 22:46 ` Stefan Hajnoczi
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 11/14] block: Decouple throttling from BlockDriverState Kevin Wolf
` (4 subsequent siblings)
14 siblings, 1 reply; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
So far, bdrv_parent_drained_begin/end() was called for the duration of
the actual bdrv_drain() at the beginning of a drained section, but we
really should keep parents quiesced until the end of the drained
section.
This does not actually change behaviour at this point because the only
user of the .drained_begin/end BdrvChildRole callback is I/O throttling,
which already doesn't send any new requests after flushing its queue in
.drained_being. The patch merely removes a trap for future users.
Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/block/io.c b/block/io.c
index 7c213ec..23abbc5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2749,11 +2749,14 @@ void bdrv_drained_begin(BlockDriverState *bs)
if (!bs->quiesce_counter++) {
aio_disable_external(bdrv_get_aio_context(bs));
}
+ bdrv_parent_drained_begin(bs);
bdrv_drain(bs);
}
void bdrv_drained_end(BlockDriverState *bs)
{
+ bdrv_parent_drained_end(bs);
+
assert(bs->quiesce_counter > 0);
if (--bs->quiesce_counter > 0) {
return;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 11/14] block: Decouple throttling from BlockDriverState
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (9 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 10/14] block/io: Quiesce parents between drained_begin/end Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 12/14] block: Remove bdrv_move_feature_fields() Kevin Wolf
` (3 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
This moves the throttling related part of the BDS life cycle management
to BlockBackend. The throttling group reference is now kept even when no
medium is inserted.
With this commit, throttling isn't disabled and then re-enabled any more
during graph reconfiguration. This fixes the temporary breakage of I/O
throttling when used with live snapshots or block jobs that manipulate
the graph.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block.c | 34 ----------------------------------
block/block-backend.c | 37 ++++++++++++++-----------------------
blockdev.c | 27 +++++++++------------------
include/block/block_int.h | 3 ---
4 files changed, 23 insertions(+), 78 deletions(-)
diff --git a/block.c b/block.c
index 18dfe97..e628356 100644
--- a/block.c
+++ b/block.c
@@ -38,7 +38,6 @@
#include "qmp-commands.h"
#include "qemu/timer.h"
#include "qapi-event.h"
-#include "block/throttle-groups.h"
#include "qemu/cutils.h"
#include "qemu/id.h"
@@ -2121,11 +2120,6 @@ static void bdrv_close(BlockDriverState *bs)
assert(!bs->job);
- /* Disable I/O limits and drain all pending throttled requests */
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- blk_io_limits_disable(bs->blk);
- }
-
bdrv_drained_begin(bs); /* complete I/O */
bdrv_flush(bs);
bdrv_drain(bs); /* in case flush left pending I/O */
@@ -2254,26 +2248,6 @@ static void swap_feature_fields(BlockDriverState *bs_top,
bdrv_move_feature_fields(&tmp, bs_top);
bdrv_move_feature_fields(bs_top, bs_new);
bdrv_move_feature_fields(bs_new, &tmp);
-
- assert(!bs_new->blk);
- if (bs_top->blk && blk_get_public(bs_top->blk)->throttle_state) {
- /*
- * FIXME Need to break I/O throttling with graph manipulations
- * temporarily because of conflicting invariants (3. will go away when
- * throttling is fully converted to work on BlockBackends):
- *
- * 1. Every BlockBackend has a single root BDS
- * 2. I/O throttling functions require an attached BlockBackend
- * 3. We need to first enable throttling on the new BDS and then
- * disable it on the old one (because of throttle group refcounts)
- */
-#if 0
- bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top));
- bdrv_io_limits_disable(bs_top);
-#else
- abort();
-#endif
- }
}
/*
@@ -3674,10 +3648,6 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
baf->detach_aio_context(baf->opaque);
}
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- throttle_timers_detach_aio_context(
- &blk_get_public(bs->blk)->throttle_timers);
- }
if (bs->drv->bdrv_detach_aio_context) {
bs->drv->bdrv_detach_aio_context(bs);
}
@@ -3711,10 +3681,6 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
if (bs->drv->bdrv_attach_aio_context) {
bs->drv->bdrv_attach_aio_context(bs, new_context);
}
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- throttle_timers_attach_aio_context(
- &blk_get_public(bs->blk)->throttle_timers, new_context);
- }
QLIST_FOREACH(ban, &bs->aio_notifiers, list) {
ban->attached_aio_context(new_context, ban->opaque);
diff --git a/block/block-backend.c b/block/block-backend.c
index 74429ae..a71b54d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -188,10 +188,6 @@ static void blk_delete(BlockBackend *blk)
}
assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers));
assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers));
- if (blk->root_state.throttle_state) {
- g_free(blk->root_state.throttle_group);
- throttle_group_unref(blk->root_state.throttle_state);
- }
QTAILQ_REMOVE(&block_backends, blk, link);
drive_info_del(blk->legacy_dinfo);
block_acct_cleanup(&blk->stats);
@@ -445,12 +441,12 @@ void blk_remove_bs(BlockBackend *blk)
assert(blk->root->bs->blk == blk);
notifier_list_notify(&blk->remove_bs_notifiers, blk);
-
- blk_update_root_state(blk);
if (blk->public.throttle_state) {
- blk_io_limits_disable(blk);
+ throttle_timers_detach_aio_context(&blk->public.throttle_timers);
}
+ blk_update_root_state(blk);
+
blk->root->bs->blk = NULL;
bdrv_root_unref_child(blk->root);
blk->root = NULL;
@@ -468,6 +464,10 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
bs->blk = blk;
notifier_list_notify(&blk->insert_bs_notifiers, blk);
+ if (blk->public.throttle_state) {
+ throttle_timers_attach_aio_context(
+ &blk->public.throttle_timers, bdrv_get_aio_context(bs));
+ }
}
/*
@@ -1374,7 +1374,14 @@ void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
BlockDriverState *bs = blk_bs(blk);
if (bs) {
+ if (blk->public.throttle_state) {
+ throttle_timers_detach_aio_context(&blk->public.throttle_timers);
+ }
bdrv_set_aio_context(bs, new_context);
+ if (blk->public.throttle_state) {
+ throttle_timers_attach_aio_context(&blk->public.throttle_timers,
+ new_context);
+ }
}
}
@@ -1539,19 +1546,6 @@ void blk_update_root_state(BlockBackend *blk)
blk->root_state.open_flags = blk->root->bs->open_flags;
blk->root_state.read_only = blk->root->bs->read_only;
blk->root_state.detect_zeroes = blk->root->bs->detect_zeroes;
-
- if (blk->root_state.throttle_group) {
- g_free(blk->root_state.throttle_group);
- throttle_group_unref(blk->root_state.throttle_state);
- }
- if (blk->public.throttle_state) {
- const char *name = throttle_group_get_name(blk);
- blk->root_state.throttle_group = g_strdup(name);
- blk->root_state.throttle_state = throttle_group_incref(name);
- } else {
- blk->root_state.throttle_group = NULL;
- blk->root_state.throttle_state = NULL;
- }
}
/*
@@ -1562,9 +1556,6 @@ void blk_update_root_state(BlockBackend *blk)
void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs)
{
bs->detect_zeroes = blk->root_state.detect_zeroes;
- if (blk->root_state.throttle_group) {
- blk_io_limits_enable(blk, blk->root_state.throttle_group);
- }
}
/*
diff --git a/blockdev.c b/blockdev.c
index e383710..6870d66 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -577,15 +577,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
blk_rs->read_only = !(bdrv_flags & BDRV_O_RDWR);
blk_rs->detect_zeroes = detect_zeroes;
- if (throttle_enabled(&cfg)) {
- if (!throttling_group) {
- throttling_group = blk_name(blk);
- }
- blk_rs->throttle_group = g_strdup(throttling_group);
- blk_rs->throttle_state = throttle_group_incref(throttling_group);
- blk_rs->throttle_state->cfg = cfg;
- }
-
QDECREF(bs_opts);
} else {
if (file && !*file) {
@@ -611,15 +602,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
bs->detect_zeroes = detect_zeroes;
- /* disk I/O throttling */
- if (throttle_enabled(&cfg)) {
- if (!throttling_group) {
- throttling_group = blk_name(blk);
- }
- blk_io_limits_enable(blk, throttling_group);
- blk_set_io_limits(blk, &cfg);
- }
-
if (bdrv_key_required(bs)) {
autostart = 0;
}
@@ -633,6 +615,15 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
}
}
+ /* disk I/O throttling */
+ if (throttle_enabled(&cfg)) {
+ if (!throttling_group) {
+ throttling_group = blk_name(blk);
+ }
+ blk_io_limits_enable(blk, throttling_group);
+ blk_set_io_limits(blk, &cfg);
+ }
+
blk_set_enable_write_cache(blk, !writethrough);
blk_set_on_error(blk, on_read_error, on_write_error);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 461583b..be59dba 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -495,9 +495,6 @@ struct BlockBackendRootState {
int open_flags;
bool read_only;
BlockdevDetectZeroesOptions detect_zeroes;
-
- char *throttle_group;
- ThrottleState *throttle_state;
};
static inline BlockDriverState *backing_bs(BlockDriverState *bs)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 12/14] block: Remove bdrv_move_feature_fields()
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (10 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 11/14] block: Decouple throttling from BlockDriverState Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 13/14] Revert "block: Forbid I/O throttling on nodes with multiple parents for 2.6" Kevin Wolf
` (2 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
bdrv_move_feature_fields() and swap_feature_fields() are empty now, they
can be removed.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block.c | 30 ------------------------------
1 file changed, 30 deletions(-)
diff --git a/block.c b/block.c
index e628356..b2f4a6b 100644
--- a/block.c
+++ b/block.c
@@ -2210,13 +2210,6 @@ void bdrv_close_all(void)
}
}
-/* Fields that need to stay with the top-level BDS */
-static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
- BlockDriverState *bs_src)
-{
- /* move some fields that need to stay attached to the device */
-}
-
static void change_parent_backing_link(BlockDriverState *from,
BlockDriverState *to)
{
@@ -2240,16 +2233,6 @@ static void change_parent_backing_link(BlockDriverState *from,
}
}
-static void swap_feature_fields(BlockDriverState *bs_top,
- BlockDriverState *bs_new)
-{
- BlockDriverState tmp;
-
- bdrv_move_feature_fields(&tmp, bs_top);
- bdrv_move_feature_fields(bs_top, bs_new);
- bdrv_move_feature_fields(bs_new, &tmp);
-}
-
/*
* Add new bs contents at the top of an image chain while the chain is
* live, while keeping required fields on the top layer.
@@ -2273,9 +2256,6 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
bdrv_ref(bs_top);
- /* Some fields always stay on top of the backing file chain */
- swap_feature_fields(bs_top, bs_new);
-
change_parent_backing_link(bs_top, bs_new);
bdrv_set_backing_hd(bs_new, bs_top);
bdrv_unref(bs_top);
@@ -2292,16 +2272,6 @@ void bdrv_replace_in_backing_chain(BlockDriverState *old, BlockDriverState *new)
bdrv_ref(old);
- if (old->blk) {
- /* As long as these fields aren't in BlockBackend, but in the top-level
- * BlockDriverState, it's not possible for a BDS to have two BBs.
- *
- * We really want to copy the fields from old to new, but we go for a
- * swap instead so that pointers aren't duplicated and cause trouble.
- * (Also, bdrv_swap() used to do the same.) */
- assert(!new->blk);
- swap_feature_fields(old, new);
- }
change_parent_backing_link(old, new);
/* Change backing files if a previously independent node is added to the
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 13/14] Revert "block: Forbid I/O throttling on nodes with multiple parents for 2.6"
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (11 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 12/14] block: Remove bdrv_move_feature_fields() Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 14/14] block: Don't check throttled reqs in bdrv_requests_pending() Kevin Wolf
2016-05-16 22:46 ` [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Stefan Hajnoczi
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
This reverts commit 76b223200ef4fb09dd87f0e213159795eb68e7a5.
Now that I/O throttling is fully done on the BlockBackend level, there
is no reason any more to block I/O throttling for nodes with multiple
parents as the parents don't influence each other any more.
Conflicts:
block.c
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block.c | 6 ------
blockdev.c | 7 -------
2 files changed, 13 deletions(-)
diff --git a/block.c b/block.c
index b2f4a6b..4a143d5 100644
--- a/block.c
+++ b/block.c
@@ -1522,12 +1522,6 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
return -ENODEV;
}
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- error_setg(errp, "Cannot reference an existing block device for "
- "which I/O throttling is enabled");
- return -EINVAL;
- }
-
bdrv_ref(bs);
*pbs = bs;
return 0;
diff --git a/blockdev.c b/blockdev.c
index 6870d66..fe20754 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2652,13 +2652,6 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
goto out;
}
- /* The BlockBackend must be the only parent */
- assert(QLIST_FIRST(&bs->parents));
- if (QLIST_NEXT(QLIST_FIRST(&bs->parents), next_parent)) {
- error_setg(errp, "Cannot throttle device with multiple parents");
- goto out;
- }
-
throttle_config_init(&cfg);
cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps;
cfg.buckets[THROTTLE_BPS_READ].avg = bps_rd;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 14/14] block: Don't check throttled reqs in bdrv_requests_pending()
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (12 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 13/14] Revert "block: Forbid I/O throttling on nodes with multiple parents for 2.6" Kevin Wolf
@ 2016-05-11 13:24 ` Kevin Wolf
2016-05-16 22:46 ` [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Stefan Hajnoczi
14 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-11 13:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, berto, pbonzini, mreitz, eblake, qemu-devel
Checking whether there are throttled requests requires going to the
associated BlockBackend, which we want to avoid.
All users of bdrv_requests_pending() in block/io.c already call
bdrv_parent_drained_begin() first, which restarts all throttled
requests, so no throttled requests can be left here and this is removal
of dead code.
The remaining users (assertions during graph manipulation in block.c)
don't care about requests that are still queued in the BlockBackend and
haven't been issued for a BlockDriverState yet.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/block/io.c b/block/io.c
index 23abbc5..6e90805 100644
--- a/block/io.c
+++ b/block/io.c
@@ -147,17 +147,10 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
bool bdrv_requests_pending(BlockDriverState *bs)
{
BdrvChild *child;
- BlockBackendPublic *blkp = bs->blk ? blk_get_public(bs->blk) : NULL;
if (!QLIST_EMPTY(&bs->tracked_requests)) {
return true;
}
- if (blkp && !qemu_co_queue_empty(&blkp->throttled_reqs[0])) {
- return true;
- }
- if (blkp && !qemu_co_queue_empty(&blkp->throttled_reqs[1])) {
- return true;
- }
QLIST_FOREACH(child, &bs->children, next) {
if (bdrv_requests_pending(child->bs)) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/14] block: Drain throttling queue with BdrvChild callback
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 09/14] block: Drain throttling queue with BdrvChild callback Kevin Wolf
@ 2016-05-16 22:45 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2016-05-16 22:45 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, berto, pbonzini, mreitz, eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 647 bytes --]
On Wed, May 11, 2016 at 03:24:20PM +0200, Kevin Wolf wrote:
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 6af541e..461583b 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -359,6 +359,17 @@ typedef struct BdrvAioNotifier {
> struct BdrvChildRole {
> void (*inherit_options)(int *child_flags, QDict *child_options,
> int parent_flags, QDict *parent_options);
> +
> + /*
> + * If this pair of functions is implemented, the parent doesn't issue new
> + * requests after returning from .drained_being() until .drained_end() is
s/being/begin/
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/14] block/io: Quiesce parents between drained_begin/end
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 10/14] block/io: Quiesce parents between drained_begin/end Kevin Wolf
@ 2016-05-16 22:46 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2016-05-16 22:46 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, berto, pbonzini, mreitz, eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]
On Wed, May 11, 2016 at 03:24:21PM +0200, Kevin Wolf wrote:
> So far, bdrv_parent_drained_begin/end() was called for the duration of
> the actual bdrv_drain() at the beginning of a drained section, but we
> really should keep parents quiesced until the end of the drained
> section.
>
> This does not actually change behaviour at this point because the only
> user of the .drained_begin/end BdrvChildRole callback is I/O throttling,
> which already doesn't send any new requests after flushing its queue in
> .drained_being. The patch merely removes a trap for future users.
s/being/begin/
>
> Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/block/io.c b/block/io.c
> index 7c213ec..23abbc5 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2749,11 +2749,14 @@ void bdrv_drained_begin(BlockDriverState *bs)
> if (!bs->quiesce_counter++) {
> aio_disable_external(bdrv_get_aio_context(bs));
> }
> + bdrv_parent_drained_begin(bs);
> bdrv_drain(bs);
> }
>
> void bdrv_drained_end(BlockDriverState *bs)
> {
> + bdrv_parent_drained_end(bs);
> +
> assert(bs->quiesce_counter > 0);
> if (--bs->quiesce_counter > 0) {
> return;
> --
> 1.8.3.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
` (13 preceding siblings ...)
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 14/14] block: Don't check throttled reqs in bdrv_requests_pending() Kevin Wolf
@ 2016-05-16 22:46 ` Stefan Hajnoczi
2016-05-17 13:39 ` Kevin Wolf
14 siblings, 1 reply; 19+ messages in thread
From: Stefan Hajnoczi @ 2016-05-16 22:46 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, berto, pbonzini, mreitz, eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]
On Wed, May 11, 2016 at 03:24:11PM +0200, Kevin Wolf wrote:
> This is another feature that was "logically" part of the BlockBackend, but
> implemented as a BlockDriverState feature. It was always kept on top using
> swap_feature_fields().
>
> This series moves it to be actually implemented in the BlockBackend, removing
> another obstacle for removing bs->blk and allowing multiple BBs per BDS.
>
> Applies to block-next.
>
>
> v3:
> - Patch 5: 'block: Move throttling fields from BDS to BB'
> Fixed a few commentions still mentioning BDS instead of BB [Berto]
>
> - Patch 9: 'block: Drain throttling queue with BdrvChild'
> Documented the semantics of BdrvChildRole.drained_begin/end [Stefan]
>
> - Patch 10 (new): 'block/io: Quiesce parents between drained_begin/end'
> Fix that parent requests were allowed again too early [Stefan]
>
> - Patch 14 (was 13): 'block: Don't check throttled reqs in
> bdrv_requests_pending()'
> Explained more cases in the commit message [Berto]
>
>
> v2:
> - Rebased on top of Paolo's 'bdrv_flush_io_queue removal, shared LinuxAioState'
> Most notable this includes a complete rewrite of patch 9 (was 10): 'block:
> Drain throttling queue with BdrvChild'. Instead of a single drain_queue()
> callback we now have a drained_begin/end() pair.
>
> - Patch 2 (was 3): 'block: Introduce BlockBackendPublic'
> Add int dummy to yet empty struct BlockBackendPublic [Eric]
>
> - Patch 11: 'block: Remove bdrv_move_feature_fields()'
> After the rebase, the function ended up empty, we can remove it now
>
> - Patch 12: 'Revert "block: Forbid I/O throttling on nodes with
> multiple parents for 2.6"'
> This was committed to master after v1 had been posted, so this one is new as
> well. The reason for forbidding this was that patch 6 ('block: Move actual
> I/O throttling to BlockBackend') would change the behaviour of the non-BB
> parents. Now that the final behaviour is implemented, we can allow the setup.
>
> Kevin Wolf (14):
> block: Make sure throttled BDSes always have a BB
> block: Introduce BlockBackendPublic
> block: throttle-groups: Use BlockBackend pointers internally
> block: Convert throttle_group_get_name() to BlockBackend
> block: Move throttling fields from BDS to BB
> block: Move actual I/O throttling to BlockBackend
> block: Move I/O throttling configuration functions to BlockBackend
> block: Introduce BdrvChild.opaque
> block: Drain throttling queue with BdrvChild callback
> block/io: Quiesce parents between drained_begin/end
> block: Decouple throttling from BlockDriverState
> block: Remove bdrv_move_feature_fields()
> Revert "block: Forbid I/O throttling on nodes with multiple parents
> for 2.6"
> block: Don't check throttled reqs in bdrv_requests_pending()
>
> block.c | 58 +---------
> block/block-backend.c | 135 ++++++++++++++++++----
> block/io.c | 86 ++++----------
> block/qapi.c | 6 +-
> block/throttle-groups.c | 244 +++++++++++++++++++++-------------------
> blockdev.c | 50 +++-----
> include/block/block.h | 4 -
> include/block/block_int.h | 35 ++----
> include/block/throttle-groups.h | 14 +--
> include/sysemu/block-backend.h | 27 +++++
> tests/test-throttle.c | 62 +++++-----
> 11 files changed, 359 insertions(+), 362 deletions(-)
I'm happy with this series modulo the typos I mentioned in replies.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend
2016-05-16 22:46 ` [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Stefan Hajnoczi
@ 2016-05-17 13:39 ` Kevin Wolf
0 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-05-17 13:39 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, berto, pbonzini, mreitz, eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 675 bytes --]
Am 17.05.2016 um 00:46 hat Stefan Hajnoczi geschrieben:
> On Wed, May 11, 2016 at 03:24:11PM +0200, Kevin Wolf wrote:
> > This is another feature that was "logically" part of the BlockBackend, but
> > implemented as a BlockDriverState feature. It was always kept on top using
> > swap_feature_fields().
> >
> > This series moves it to be actually implemented in the BlockBackend, removing
> > another obstacle for removing bs->blk and allowing multiple BBs per BDS.
> >
> > Applies to block-next.
> > [...]
>
> I'm happy with this series modulo the typos I mentioned in replies.
Thanks, fixed the typos and applied the series to the block branch.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-05-17 13:39 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-11 13:24 [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 01/14] block: Make sure throttled BDSes always have a BB Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 02/14] block: Introduce BlockBackendPublic Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 03/14] block: throttle-groups: Use BlockBackend pointers internally Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 04/14] block: Convert throttle_group_get_name() to BlockBackend Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 05/14] block: Move throttling fields from BDS to BB Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 06/14] block: Move actual I/O throttling to BlockBackend Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 07/14] block: Move I/O throttling configuration functions " Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 08/14] block: Introduce BdrvChild.opaque Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 09/14] block: Drain throttling queue with BdrvChild callback Kevin Wolf
2016-05-16 22:45 ` Stefan Hajnoczi
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 10/14] block/io: Quiesce parents between drained_begin/end Kevin Wolf
2016-05-16 22:46 ` Stefan Hajnoczi
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 11/14] block: Decouple throttling from BlockDriverState Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 12/14] block: Remove bdrv_move_feature_fields() Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 13/14] Revert "block: Forbid I/O throttling on nodes with multiple parents for 2.6" Kevin Wolf
2016-05-11 13:24 ` [Qemu-devel] [PATCH v3 14/14] block: Don't check throttled reqs in bdrv_requests_pending() Kevin Wolf
2016-05-16 22:46 ` [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend Stefan Hajnoczi
2016-05-17 13:39 ` Kevin Wolf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.