From patchwork Wed Aug 9 14:02:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Manos Pitsidianakis X-Patchwork-Id: 9890703 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 A937460384 for ; Wed, 9 Aug 2017 14:11:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 804DC28A99 for ; Wed, 9 Aug 2017 14:11:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7529028AA3; Wed, 9 Aug 2017 14:11:09 +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 5FE3A28A99 for ; Wed, 9 Aug 2017 14:11:08 +0000 (UTC) Received: from localhost ([::1]:47718 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dfRhT-0000ke-Ov for patchwork-qemu-devel@patchwork.kernel.org; Wed, 09 Aug 2017 10:11:07 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49867) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dfRat-0003US-CC for qemu-devel@nongnu.org; Wed, 09 Aug 2017 10:04:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dfRar-0003dR-5C for qemu-devel@nongnu.org; Wed, 09 Aug 2017 10:04:19 -0400 Received: from smtp1.ntua.gr ([2001:648:2000:de::183]:49072) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dfRal-0003Wk-P3; Wed, 09 Aug 2017 10:04:12 -0400 Received: from mail.ntua.gr ([IPv6:2a02:587:8030:4e00:989e:98bf:1f16:f20b]) (authenticated bits=0) by smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v79E3U66083141 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 9 Aug 2017 17:03:30 +0300 (EEST) (envelope-from el13635@mail.ntua.gr) X-Authentication-Warning: smtp1.ntua.gr: Host [IPv6:2a02:587:8030:4e00:989e:98bf:1f16:f20b] claimed to be mail.ntua.gr From: Manos Pitsidianakis To: qemu-devel Date: Wed, 9 Aug 2017 17:02:54 +0300 Message-Id: <20170809140256.25584-5-el13635@mail.ntua.gr> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170809140256.25584-1-el13635@mail.ntua.gr> References: <20170809140256.25584-1-el13635@mail.ntua.gr> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:648:2000:de::183 Subject: [Qemu-devel] [PATCH v2 4/6] block: remove legacy I/O throttling 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: Kevin Wolf , Alberto Garcia , Stefan Hajnoczi , qemu-block Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP This commit removes all I/O throttling from block/block-backend.c. In order to support the existing interface, it is changed to use the block/throttle.c filter driver. The throttle filter node that is created by the legacy interface is stored in a 'throttle_node' field in the BlockBackendPublic of the device. The legacy throttle node is managed by the legacy interface completely. More advanced configurations with the filter drive are possible using the QMP API, but these will be ignored by the legacy interface. Signed-off-by: Manos Pitsidianakis --- block/block-backend.c | 133 ++++++++++++++++++++++++---------------- block/qapi.c | 10 +-- block/throttle.c | 8 +++ blockdev.c | 49 +++++++++++---- include/block/throttle-groups.h | 1 + include/sysemu/block-backend.h | 6 +- tests/test-throttle.c | 19 +++--- 7 files changed, 146 insertions(+), 80 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index df0200fc49..61983b7393 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -15,6 +15,7 @@ #include "block/block_int.h" #include "block/blockjob.h" #include "block/throttle-groups.h" +#include "qemu/throttle-options.h" #include "sysemu/blockdev.h" #include "sysemu/sysemu.h" #include "qapi-event.h" @@ -282,7 +283,7 @@ static void blk_delete(BlockBackend *blk) assert(!blk->refcnt); assert(!blk->name); assert(!blk->dev); - if (blk->public.throttle_group_member.throttle_state) { + if (blk->public.throttle_node) { blk_io_limits_disable(blk); } if (blk->root) { @@ -593,13 +594,7 @@ BlockBackend *blk_by_public(BlockBackendPublic *public) */ void blk_remove_bs(BlockBackend *blk) { - ThrottleTimers *tt; - notifier_list_notify(&blk->remove_bs_notifiers, blk); - if (blk->public.throttle_group_member.throttle_state) { - tt = &blk->public.throttle_group_member.throttle_timers; - throttle_timers_detach_aio_context(tt); - } blk_update_root_state(blk); @@ -620,12 +615,6 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp) bdrv_ref(bs); notifier_list_notify(&blk->insert_bs_notifiers, blk); - if (blk->public.throttle_group_member.throttle_state) { - throttle_timers_attach_aio_context( - &blk->public.throttle_group_member.throttle_timers, - bdrv_get_aio_context(bs)); - } - return 0; } @@ -983,13 +972,6 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset, } bdrv_inc_in_flight(bs); - - /* throttling disk I/O */ - if (blk->public.throttle_group_member.throttle_state) { - throttle_group_co_io_limits_intercept(&blk->public.throttle_group_member, - bytes, false); - } - ret = bdrv_co_preadv(blk->root, offset, bytes, qiov, flags); bdrv_dec_in_flight(bs); return ret; @@ -1010,11 +992,6 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset, } bdrv_inc_in_flight(bs); - /* throttling disk I/O */ - if (blk->public.throttle_group_member.throttle_state) { - throttle_group_co_io_limits_intercept(&blk->public.throttle_group_member, - bytes, true); - } if (!blk->enable_write_cache) { flags |= BDRV_REQ_FUA; @@ -1682,16 +1659,9 @@ static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb) void blk_set_aio_context(BlockBackend *blk, AioContext *new_context) { BlockDriverState *bs = blk_bs(blk); - ThrottleGroupMember *tgm = &blk->public.throttle_group_member; if (bs) { - if (tgm->throttle_state) { - throttle_group_detach_aio_context(tgm); - } bdrv_set_aio_context(bs, new_context); - if (tgm->throttle_state) { - throttle_group_attach_aio_context(tgm, new_context); - } } } @@ -1909,45 +1879,98 @@ int blk_commit_all(void) /* throttling disk I/O limits */ void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg) { - throttle_group_config(&blk->public.throttle_group_member, cfg); + assert(blk->public.throttle_node); + throttle_group_config(throttle_get_tgm(blk->public.throttle_node), cfg); } void blk_io_limits_disable(BlockBackend *blk) { - assert(blk->public.throttle_group_member.throttle_state); - bdrv_drained_begin(blk_bs(blk)); - throttle_group_unregister_tgm(&blk->public.throttle_group_member); - bdrv_drained_end(blk_bs(blk)); + BlockDriverState *bs, *throttle_node; + + throttle_node = blk_get_public(blk)->throttle_node; + + assert(throttle_node); + + bs = throttle_node->file->bs; + bdrv_drained_begin(bs); + + /* Ref throttle_node's child bs to ensure it won't go away */ + bdrv_ref(bs); + + bdrv_child_try_set_perm(throttle_node->file, 0, BLK_PERM_ALL, + &error_abort); + /* Replace throttle_node with bs. While throttle_node was inserted under + * blk, at this point it might have more than one parent, so use + * bdrv_replace_node(). This destroys throttle_node */ + bdrv_replace_node(throttle_node, bs, &error_abort); + blk_get_public(blk)->throttle_node = NULL; + + bdrv_unref(bs); + bdrv_drained_end(bs); + } /* should be called before blk_set_io_limits if a limit is set */ -void blk_io_limits_enable(BlockBackend *blk, const char *group) +void blk_io_limits_enable(BlockBackend *blk, const char *group, Error **errp) { - assert(!blk->public.throttle_group_member.throttle_state); - throttle_group_register_tgm(&blk->public.throttle_group_member, - group, blk_get_aio_context(blk)); + BlockDriverState *bs = blk_bs(blk), *throttle_node; + QDict *options = qdict_new(); + Error *local_err = NULL; + + bdrv_drained_begin(bs); + + qdict_set_default_str(options, "file", bs->node_name); + qdict_set_default_str(options, QEMU_OPT_THROTTLE_GROUP_NAME, group); + throttle_node = bdrv_new_open_driver(bdrv_find_format("throttle"), NULL, + bdrv_get_flags(bs), options, errp); + if (!throttle_node) { + goto end; + } + throttle_node->implicit = true; + + blk_remove_bs(blk); + blk_insert_bs(blk, throttle_node, &local_err); + if (local_err) { + error_propagate(errp, local_err); + blk_insert_bs(blk, bs, &error_abort); + bdrv_unref(throttle_node); + throttle_node = NULL; + goto end; + } + bdrv_unref(throttle_node); + + assert(throttle_node->file->bs == bs); + assert(throttle_node->refcnt == 1); + +end: + bdrv_drained_end(bs); + blk_get_public(blk)->throttle_node = throttle_node; } -void blk_io_limits_update_group(BlockBackend *blk, const char *group) +void blk_io_limits_update_group(BlockBackend *blk, const char *group, Error **errp) { + ThrottleGroupMember *tgm; + /* this BB is not part of any group */ - if (!blk->public.throttle_group_member.throttle_state) { + if (!blk->public.throttle_node) { return; } + tgm = throttle_get_tgm(blk->public.throttle_node); /* this BB is a part of the same group than the one we want */ - if (!g_strcmp0(throttle_group_get_name(&blk->public.throttle_group_member), - group)) { + if (!g_strcmp0(throttle_group_get_name(tgm), + group)) { return; } - /* need to change the group this bs belong to */ + /* need to change the group this bs belongs to */ blk_io_limits_disable(blk); - blk_io_limits_enable(blk, group); + blk_io_limits_enable(blk, group, errp); } static void blk_root_drained_begin(BdrvChild *child) { + ThrottleGroupMember *tgm; BlockBackend *blk = child->opaque; if (++blk->quiesce_counter == 1) { @@ -1958,19 +1981,25 @@ static void blk_root_drained_begin(BdrvChild *child) /* Note that blk->root may not be accessible here yet if we are just * attaching to a BlockDriverState that is drained. Use child instead. */ - - if (atomic_fetch_inc(&blk->public.throttle_group_member.io_limits_disabled) == 0) { - throttle_group_restart_tgm(&blk->public.throttle_group_member); + if (blk->public.throttle_node) { + tgm = throttle_get_tgm(blk->public.throttle_node); + if (atomic_fetch_inc(&tgm->io_limits_disabled) == 0) { + throttle_group_restart_tgm(tgm); + } } } static void blk_root_drained_end(BdrvChild *child) { + ThrottleGroupMember *tgm; BlockBackend *blk = child->opaque; assert(blk->quiesce_counter); - assert(blk->public.throttle_group_member.io_limits_disabled); - atomic_dec(&blk->public.throttle_group_member.io_limits_disabled); + if (blk->public.throttle_node) { + tgm = throttle_get_tgm(blk->public.throttle_node); + assert(tgm->io_limits_disabled); + atomic_dec(&tgm->io_limits_disabled); + } if (--blk->quiesce_counter == 0) { if (blk->dev_ops && blk->dev_ops->drained_end) { diff --git a/block/qapi.c b/block/qapi.c index 847b044d13..ab55db7134 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -66,11 +66,12 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, info->detect_zeroes = bs->detect_zeroes; - if (blk && blk_get_public(blk)->throttle_group_member.throttle_state) { + if (blk && blk_get_public(blk)->throttle_node) { ThrottleConfig cfg; - BlockBackendPublic *blkp = blk_get_public(blk); + BlockDriverState *throttle_node = blk_get_public(blk)->throttle_node; + ThrottleGroupMember *tgm = throttle_get_tgm(throttle_node); - throttle_group_get_config(&blkp->throttle_group_member, &cfg); + throttle_group_get_config(tgm, &cfg); info->bps = cfg.buckets[THROTTLE_BPS_TOTAL].avg; info->bps_rd = cfg.buckets[THROTTLE_BPS_READ].avg; @@ -118,8 +119,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(&blkp->throttle_group_member)); + info->group = g_strdup(throttle_group_get_name(tgm)); } info->write_threshold = bdrv_write_threshold_get(bs); diff --git a/block/throttle.c b/block/throttle.c index 3e6cb1de7b..16861b57ff 100644 --- a/block/throttle.c +++ b/block/throttle.c @@ -38,6 +38,14 @@ static QemuOptsList throttle_opts = { }, }; +static BlockDriver bdrv_throttle; + +ThrottleGroupMember *throttle_get_tgm(BlockDriverState *bs) +{ + assert(bs->drv == &bdrv_throttle); + return (ThrottleGroupMember *)bs->opaque; +} + /* Extract ThrottleConfig options. Assumes cfg is initialized and will be * checked for validity. * diff --git a/blockdev.c b/blockdev.c index 6ffa5b0b04..20e5513f87 100644 --- a/blockdev.c +++ b/blockdev.c @@ -607,7 +607,14 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, if (!throttling_group) { throttling_group = id; } - blk_io_limits_enable(blk, throttling_group); + blk_io_limits_enable(blk, throttling_group, &error); + if (error) { + error_propagate(errp, error); + blk_unref(blk); + blk = NULL; + goto err_no_bs_opts; + + } blk_set_io_limits(blk, &cfg); } @@ -2629,6 +2636,9 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) BlockDriverState *bs; BlockBackend *blk; AioContext *aio_context; + BlockDriverState *throttle_node = NULL; + ThrottleGroupMember *tgm; + Error *local_err = NULL; blk = qmp_get_blk(arg->has_device ? arg->device : NULL, arg->has_id ? arg->id : NULL, @@ -2704,18 +2714,33 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp) if (throttle_enabled(&cfg)) { /* Enable I/O limits if they're not enabled yet, otherwise * just update the throttling group. */ - if (!blk_get_public(blk)->throttle_group_member.throttle_state) { - blk_io_limits_enable(blk, - arg->has_group ? arg->group : - arg->has_device ? arg->device : - arg->id); - } else if (arg->has_group) { - blk_io_limits_update_group(blk, arg->group); + if (!blk_get_public(blk)->throttle_node) { + blk_io_limits_enable(blk, arg->has_group ? arg->group : + arg->has_device ? arg->device : arg->id, + &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto out; + } } - /* Set the new throttling configuration */ - blk_set_io_limits(blk, &cfg); - } else if (blk_get_public(blk)->throttle_group_member.throttle_state) { - /* If all throttling settings are set to 0, disable I/O limits */ + + if (arg->has_group) { + /* move throttle node membership to arg->group */ + blk_io_limits_update_group(blk, arg->group, &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto out; + } + } + + throttle_node = blk_get_public(blk)->throttle_node; + tgm = throttle_get_tgm(throttle_node); + throttle_group_config(tgm, &cfg); + } else if (blk_get_public(blk)->throttle_node) { + /* + * If all throttling settings are set to 0, disable I/O limits + * by deleting the legacy throttle node + * */ blk_io_limits_disable(blk); } diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h index 82f030523f..26a8e44b43 100644 --- a/include/block/throttle-groups.h +++ b/include/block/throttle-groups.h @@ -76,5 +76,6 @@ void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm void throttle_group_attach_aio_context(ThrottleGroupMember *tgm, AioContext *new_context); void throttle_group_detach_aio_context(ThrottleGroupMember *tgm); +ThrottleGroupMember *throttle_get_tgm(BlockDriverState *bs); #endif diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 0e0cda7521..4a7ca53685 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -73,7 +73,7 @@ typedef struct BlockDevOps { * friends so that BlockBackends can be kept in lists outside block-backend.c * */ typedef struct BlockBackendPublic { - ThrottleGroupMember throttle_group_member; + BlockDriverState *throttle_node; } BlockBackendPublic; BlockBackend *blk_new(uint64_t perm, uint64_t shared_perm); @@ -225,7 +225,7 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk, 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); +void blk_io_limits_enable(BlockBackend *blk, const char *group, Error **errp); +void blk_io_limits_update_group(BlockBackend *blk, const char *group, Error **errp); #endif diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 0ea9093eee..eef2b1c707 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -594,7 +594,6 @@ static void test_groups(void) { ThrottleConfig cfg1, cfg2; BlockBackend *blk1, *blk2, *blk3; - BlockBackendPublic *blkp1, *blkp2, *blkp3; ThrottleGroupMember *tgm1, *tgm2, *tgm3; /* No actual I/O is performed on these devices */ @@ -602,13 +601,9 @@ static void test_groups(void) blk2 = blk_new(0, BLK_PERM_ALL); blk3 = blk_new(0, BLK_PERM_ALL); - blkp1 = blk_get_public(blk1); - blkp2 = blk_get_public(blk2); - blkp3 = blk_get_public(blk3); - - tgm1 = &blkp1->throttle_group_member; - tgm2 = &blkp2->throttle_group_member; - tgm3 = &blkp3->throttle_group_member; + tgm1 = g_new0(ThrottleGroupMember, 1); + tgm2 = g_new0(ThrottleGroupMember, 1); + tgm3 = g_new0(ThrottleGroupMember, 1); g_assert(tgm1->throttle_state == NULL); g_assert(tgm2->throttle_state == NULL); @@ -655,6 +650,14 @@ static void test_groups(void) g_assert(tgm1->throttle_state == NULL); g_assert(tgm2->throttle_state == NULL); g_assert(tgm3->throttle_state == NULL); + + g_free(tgm1); + g_free(tgm2); + g_free(tgm3); + + blk_unref(blk1); + blk_unref(blk2); + blk_unref(blk3); } int main(int argc, char **argv)