From patchwork Thu Aug 24 13:24:46 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 805489 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=igalia.com header.i=@igalia.com header.b="H8rTnXjX"; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xdQ7B6tcyz9sRW for ; Thu, 24 Aug 2017 23:26:46 +1000 (AEST) Received: from localhost ([::1]:48635 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dks9k-0003KX-Pr for incoming@patchwork.ozlabs.org; Thu, 24 Aug 2017 09:26:44 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41976) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dks90-0003JT-GH for qemu-devel@nongnu.org; Thu, 24 Aug 2017 09:25:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dks8w-0003Tp-Eu for qemu-devel@nongnu.org; Thu, 24 Aug 2017 09:25:58 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:53884) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dks8w-00032y-1T; Thu, 24 Aug 2017 09:25:54 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=1HRMARnXJOjzafJeuMIL/6Ux2YfytMrdsEEih3I13SU=; b=H8rTnXjXf6vvDKTixwmcHfo/bRCrN+Wc1bCAfrLhHBPC5+62OM3YCllSnRSYXzFi8cF9jqL821pzjZicEjCxXlSlEAdPeX7cg27Fbn2nUgoBWnzU7J5NfBYfC0Dlwuf4t+XMfbmQWSD8Vwl+ZvuinGKT+lZWteWXgdRWDLIgi7EgudNAd8kgf01VwbHnAqF/iF5xDurJSDpNyN4zXXUVoCIdiIfITGICRS6ZCqlaiIBtw8iqsD4N8XVJ7a80+SlVsBaYx5R7W0bqT09O7t07diHz2gSmizm8NzuZDLBBAwigFP4C00WZKQbauY3kguRRKm1kxvOY5q9HQjIpi944KA==; Received: from [194.100.51.2] (helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1dks8H-0002oQ-Sl; Thu, 24 Aug 2017 15:25:14 +0200 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1dks80-0007cw-Qm; Thu, 24 Aug 2017 16:24:56 +0300 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 24 Aug 2017 16:24:46 +0300 Message-Id: <5b0b9e1ac6eb208d709eddc7b09e7669a523bff3.1503580370.git.berto@igalia.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH v2 4/7] throttle: Remove throttle_fix_bucket() / throttle_unfix_bucket() 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: Alberto Garcia , Markus Armbruster , qemu-block@nongnu.org, Manos Pitsidianakis Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" The throttling code can change internally the value of bkt->max if it hasn't been set by the user. The problem with this is that if we want to retrieve the original value we have to undo this change first. This is ugly and unnecessary: this patch removes the throttle_fix_bucket() and throttle_unfix_bucket() functions completely and moves the logic to throttle_compute_wait(). Signed-off-by: Alberto Garcia Reviewed-by: Manos Pitsidianakis --- util/throttle.c | 62 +++++++++++++++++++++------------------------------------ 1 file changed, 23 insertions(+), 39 deletions(-) diff --git a/util/throttle.c b/util/throttle.c index bde56fe3de..4e80a7ea54 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -95,23 +95,36 @@ static int64_t throttle_do_compute_wait(double limit, double extra) int64_t throttle_compute_wait(LeakyBucket *bkt) { double extra; /* the number of extra units blocking the io */ + double bucket_size; /* I/O before throttling to bkt->avg */ + double burst_bucket_size; /* Before throttling to bkt->max */ if (!bkt->avg) { return 0; } - /* If the bucket is full then we have to wait */ - extra = bkt->level - bkt->max * bkt->burst_length; + if (!bkt->max) { + /* If bkt->max is 0 we still want to allow short bursts of I/O + * from the guest, otherwise every other request will be throttled + * and performance will suffer considerably. */ + bucket_size = bkt->avg / 10; + burst_bucket_size = 0; + } else { + /* If we have a burst limit then we have to wait until all I/O + * at burst rate has finished before throttling to bkt->avg */ + bucket_size = bkt->max * bkt->burst_length; + burst_bucket_size = bkt->max / 10; + } + + /* If the main bucket is full then we have to wait */ + extra = bkt->level - bucket_size; if (extra > 0) { return throttle_do_compute_wait(bkt->avg, extra); } - /* If the bucket is not full yet we have to make sure that we - * fulfill the goal of bkt->max units per second. */ + /* If the main bucket is not full yet we still have to check the + * burst bucket in order to enforce the burst limit */ if (bkt->burst_length > 1) { - /* We use 1/10 of the max value to smooth the throttling. - * See throttle_fix_bucket() for more details. */ - extra = bkt->burst_level - bkt->max / 10; + extra = bkt->burst_level - burst_bucket_size; if (extra > 0) { return throttle_do_compute_wait(bkt->max, extra); } @@ -357,31 +370,6 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp) return true; } -/* fix bucket parameters */ -static void throttle_fix_bucket(LeakyBucket *bkt) -{ - double min; - - /* zero bucket level */ - bkt->level = bkt->burst_level = 0; - - /* If bkt->max is 0 we still want to allow short bursts of I/O - * from the guest, otherwise every other request will be throttled - * and performance will suffer considerably. */ - min = bkt->avg / 10; - if (bkt->avg && !bkt->max) { - bkt->max = min; - } -} - -/* undo internal bucket parameter changes (see throttle_fix_bucket()) */ -static void throttle_unfix_bucket(LeakyBucket *bkt) -{ - if (bkt->max < bkt->avg) { - bkt->max = 0; - } -} - /* Used to configure the throttle * * @ts: the throttle state we are working on @@ -396,8 +384,10 @@ void throttle_config(ThrottleState *ts, ts->cfg = *cfg; + /* Zero bucket level */ for (i = 0; i < BUCKETS_COUNT; i++) { - throttle_fix_bucket(&ts->cfg.buckets[i]); + ts->cfg.buckets[i].level = 0; + ts->cfg.buckets[i].burst_level = 0; } ts->previous_leak = qemu_clock_get_ns(clock_type); @@ -410,13 +400,7 @@ void throttle_config(ThrottleState *ts, */ void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg) { - int i; - *cfg = ts->cfg; - - for (i = 0; i < BUCKETS_COUNT; i++) { - throttle_unfix_bucket(&cfg->buckets[i]); - } }