From patchwork Thu May 19 15:21:49 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 9127813 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1FA0360221 for ; Thu, 19 May 2016 15:27:32 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 130F8281BB for ; Thu, 19 May 2016 15:27:32 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 081F5281C3; Thu, 19 May 2016 15:27:32 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 469CD281BB for ; Thu, 19 May 2016 15:27:31 +0000 (UTC) Received: from localhost ([::1]:50946 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3PrG-0001Qx-B4 for patchwork-qemu-devel@patchwork.kernel.org; Thu, 19 May 2016 11:27:30 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41445) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3Pmb-0005KC-Vx for qemu-devel@nongnu.org; Thu, 19 May 2016 11:22:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b3Pma-0000IZ-F5 for qemu-devel@nongnu.org; Thu, 19 May 2016 11:22:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60249) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3PmN-0008Su-3Q; Thu, 19 May 2016 11:22:27 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9C2517DCD1; Thu, 19 May 2016 15:22:26 +0000 (UTC) Received: from noname.str.redhat.com. (dhcp-192-197.str.redhat.com [10.33.192.197]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4JFMEB0025145; Thu, 19 May 2016 11:22:25 -0400 From: Kevin Wolf To: qemu-block@nongnu.org Date: Thu, 19 May 2016 17:21:49 +0200 Message-Id: <1463671329-22655-12-git-send-email-kwolf@redhat.com> In-Reply-To: <1463671329-22655-1-git-send-email-kwolf@redhat.com> References: <1463671329-22655-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 19 May 2016 15:22:26 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 11/31] block: Decouple throttling from BlockDriverState X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 Reviewed-by: Alberto Garcia Acked-by: Stefan Hajnoczi --- 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 b5c02c4..98226fe 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 89768ab..93ddc27 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 a1f8488..db2b4bc 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -500,9 +500,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)