From 3c9242f5ae8ca37bb5775747e34999fe8cdfee2f Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 18 Feb 2016 12:26:54 +0200 Subject: throttle: Make throttle_compute_timer() static This function is only used internally in throttle.c Signed-off-by: Alberto Garcia Reviewed-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- util/throttle.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'util/throttle.c') diff --git a/util/throttle.c b/util/throttle.c index 2f9b23d925..c21043a8d5 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -137,10 +137,10 @@ static int64_t throttle_compute_wait_for(ThrottleState *ts, * @next_timestamp: the resulting timer * @ret: true if a timer must be set */ -bool throttle_compute_timer(ThrottleState *ts, - bool is_write, - int64_t now, - int64_t *next_timestamp) +static bool throttle_compute_timer(ThrottleState *ts, + bool is_write, + int64_t now, + int64_t *next_timestamp) { int64_t wait; -- cgit 1.4.1 From 6921b1809507797752b039b2892fc33bf6bccb7e Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 18 Feb 2016 12:26:55 +0200 Subject: throttle: Make throttle_conflicting() set errp The caller does not need to set it, and this will allow us to refactor this function later. Signed-off-by: Alberto Garcia Reviewed-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- blockdev.c | 4 +--- include/qemu/throttle.h | 2 +- tests/test-throttle.c | 12 ++++++------ util/throttle.c | 11 +++++++++-- 4 files changed, 17 insertions(+), 12 deletions(-) (limited to 'util/throttle.c') diff --git a/blockdev.c b/blockdev.c index ed97d8a7ba..14e89dea17 100644 --- a/blockdev.c +++ b/blockdev.c @@ -345,9 +345,7 @@ static bool parse_stats_intervals(BlockAcctStats *stats, QList *intervals, static bool check_throttle_config(ThrottleConfig *cfg, Error **errp) { - if (throttle_conflicting(cfg)) { - error_setg(errp, "bps/iops/max total values and read/write values" - " cannot be used at the same time"); + if (throttle_conflicting(cfg, errp)) { return false; } diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index 52c98d9e61..69cf171b1a 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -106,7 +106,7 @@ bool throttle_timers_are_initialized(ThrottleTimers *tt); /* configuration */ bool throttle_enabled(ThrottleConfig *cfg); -bool throttle_conflicting(ThrottleConfig *cfg); +bool throttle_conflicting(ThrottleConfig *cfg, Error **errp); bool throttle_is_valid(ThrottleConfig *cfg); diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 858f1aa43f..579b8af87e 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -255,31 +255,31 @@ static void test_conflicts_for_one_set(bool is_max, int write) { memset(&cfg, 0, sizeof(cfg)); - g_assert(!throttle_conflicting(&cfg)); + g_assert(!throttle_conflicting(&cfg, NULL)); set_cfg_value(is_max, total, 1); set_cfg_value(is_max, read, 1); - g_assert(throttle_conflicting(&cfg)); + g_assert(throttle_conflicting(&cfg, NULL)); memset(&cfg, 0, sizeof(cfg)); set_cfg_value(is_max, total, 1); set_cfg_value(is_max, write, 1); - g_assert(throttle_conflicting(&cfg)); + g_assert(throttle_conflicting(&cfg, NULL)); memset(&cfg, 0, sizeof(cfg)); set_cfg_value(is_max, total, 1); set_cfg_value(is_max, read, 1); set_cfg_value(is_max, write, 1); - g_assert(throttle_conflicting(&cfg)); + g_assert(throttle_conflicting(&cfg, NULL)); memset(&cfg, 0, sizeof(cfg)); set_cfg_value(is_max, total, 1); - g_assert(!throttle_conflicting(&cfg)); + g_assert(!throttle_conflicting(&cfg, NULL)); memset(&cfg, 0, sizeof(cfg)); set_cfg_value(is_max, read, 1); set_cfg_value(is_max, write, 1); - g_assert(!throttle_conflicting(&cfg)); + g_assert(!throttle_conflicting(&cfg, NULL)); } static void test_conflicting_config(void) diff --git a/util/throttle.c b/util/throttle.c index c21043a8d5..564e13261e 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -252,8 +252,9 @@ bool throttle_enabled(ThrottleConfig *cfg) * * @cfg: the throttling configuration to inspect * @ret: true if any conflict detected else false + * @errp: error object */ -bool throttle_conflicting(ThrottleConfig *cfg) +bool throttle_conflicting(ThrottleConfig *cfg, Error **errp) { bool bps_flag, ops_flag; bool bps_max_flag, ops_max_flag; @@ -274,7 +275,13 @@ bool throttle_conflicting(ThrottleConfig *cfg) (cfg->buckets[THROTTLE_OPS_READ].max || cfg->buckets[THROTTLE_OPS_WRITE].max); - return bps_flag || ops_flag || bps_max_flag || ops_max_flag; + if (bps_flag || ops_flag || bps_max_flag || ops_max_flag) { + error_setg(errp, "bps/iops/max total values and read/write values" + " cannot be used at the same time"); + return true; + } + + return false; } /* check if a throttling configuration is valid -- cgit 1.4.1 From 45b2d418e05e8b3cce9c8370ef6daac4b073b57a Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 18 Feb 2016 12:26:56 +0200 Subject: throttle: Make throttle_max_is_missing_limit() set errp The caller does not need to set it, and this will allow us to refactor this function later. Signed-off-by: Alberto Garcia Reviewed-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- blockdev.c | 4 +--- include/qemu/throttle.h | 2 +- tests/test-throttle.c | 6 +++--- util/throttle.c | 5 ++++- 4 files changed, 9 insertions(+), 8 deletions(-) (limited to 'util/throttle.c') diff --git a/blockdev.c b/blockdev.c index 14e89dea17..52aabf7b90 100644 --- a/blockdev.c +++ b/blockdev.c @@ -355,9 +355,7 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp) return false; } - if (throttle_max_is_missing_limit(cfg)) { - error_setg(errp, "bps_max/iops_max require corresponding" - " bps/iops values"); + if (throttle_max_is_missing_limit(cfg, errp)) { return false; } diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index 69cf171b1a..03bdec07ea 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -110,7 +110,7 @@ bool throttle_conflicting(ThrottleConfig *cfg, Error **errp); bool throttle_is_valid(ThrottleConfig *cfg); -bool throttle_max_is_missing_limit(ThrottleConfig *cfg); +bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp); void throttle_config(ThrottleState *ts, ThrottleTimers *tt, diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 579b8af87e..49bd3bccaf 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -338,15 +338,15 @@ static void test_max_is_missing_limit(void) memset(&cfg, 0, sizeof(cfg)); cfg.buckets[i].max = 100; cfg.buckets[i].avg = 0; - g_assert(throttle_max_is_missing_limit(&cfg)); + g_assert(throttle_max_is_missing_limit(&cfg, NULL)); cfg.buckets[i].max = 0; cfg.buckets[i].avg = 0; - g_assert(!throttle_max_is_missing_limit(&cfg)); + g_assert(!throttle_max_is_missing_limit(&cfg, NULL)); cfg.buckets[i].max = 0; cfg.buckets[i].avg = 100; - g_assert(!throttle_max_is_missing_limit(&cfg)); + g_assert(!throttle_max_is_missing_limit(&cfg, NULL)); } } diff --git a/util/throttle.c b/util/throttle.c index 564e13261e..77010b4a1a 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -306,13 +306,16 @@ bool throttle_is_valid(ThrottleConfig *cfg) /* check if bps_max/iops_max is used without bps/iops * @cfg: the throttling configuration to inspect + * @errp: error object */ -bool throttle_max_is_missing_limit(ThrottleConfig *cfg) +bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp) { int i; for (i = 0; i < BUCKETS_COUNT; i++) { if (cfg->buckets[i].max && !cfg->buckets[i].avg) { + error_setg(errp, "bps_max/iops_max require corresponding" + " bps/iops values"); return true; } } -- cgit 1.4.1 From 03ba36c83d136a9d039b56f0a834e65676b58c22 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 18 Feb 2016 12:26:57 +0200 Subject: throttle: Make throttle_is_valid() set errp The caller does not need to set it, and this will allow us to refactor this function later. Signed-off-by: Alberto Garcia Reviewed-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- blockdev.c | 4 +--- include/qemu/throttle.h | 2 +- tests/test-throttle.c | 2 +- util/throttle.c | 5 ++++- 4 files changed, 7 insertions(+), 6 deletions(-) (limited to 'util/throttle.c') diff --git a/blockdev.c b/blockdev.c index 52aabf7b90..11a3139851 100644 --- a/blockdev.c +++ b/blockdev.c @@ -349,9 +349,7 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp) return false; } - if (!throttle_is_valid(cfg)) { - error_setg(errp, "bps/iops/max values must be within [0, %lld]", - THROTTLE_VALUE_MAX); + if (!throttle_is_valid(cfg, errp)) { return false; } diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index 03bdec07ea..ecae6212ff 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -108,7 +108,7 @@ bool throttle_enabled(ThrottleConfig *cfg); bool throttle_conflicting(ThrottleConfig *cfg, Error **errp); -bool throttle_is_valid(ThrottleConfig *cfg); +bool throttle_is_valid(ThrottleConfig *cfg, Error **errp); bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp); diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 49bd3bccaf..0e7c7e0f3f 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -315,7 +315,7 @@ static void test_is_valid_for_value(int value, bool should_be_valid) for (index = 0; index < BUCKETS_COUNT; index++) { memset(&cfg, 0, sizeof(cfg)); set_cfg_value(is_max, index, value); - g_assert(throttle_is_valid(&cfg) == should_be_valid); + g_assert(throttle_is_valid(&cfg, NULL) == should_be_valid); } } } diff --git a/util/throttle.c b/util/throttle.c index 77010b4a1a..9046dd8e36 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -287,8 +287,9 @@ bool throttle_conflicting(ThrottleConfig *cfg, Error **errp) /* check if a throttling configuration is valid * @cfg: the throttling configuration to inspect * @ret: true if valid else false + * @errp: error object */ -bool throttle_is_valid(ThrottleConfig *cfg) +bool throttle_is_valid(ThrottleConfig *cfg, Error **errp) { int i; @@ -297,6 +298,8 @@ bool throttle_is_valid(ThrottleConfig *cfg) cfg->buckets[i].max < 0 || cfg->buckets[i].avg > THROTTLE_VALUE_MAX || cfg->buckets[i].max > THROTTLE_VALUE_MAX) { + error_setg(errp, "bps/iops/max values must be within [0, %lld]", + THROTTLE_VALUE_MAX); return false; } } -- cgit 1.4.1 From d5851089a8a77d5c23e8d5fffb5b99265009ba62 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 18 Feb 2016 12:26:59 +0200 Subject: throttle: Merge all functions that check the configuration into one There's no need to keep throttle_conflicting(), throttle_is_valid() and throttle_max_is_missing_limit() as separate functions, so this patch merges all three into one. As a consequence, check_throttle_config() becomes redundant and can be replaced with throttle_is_valid(). Signed-off-by: Alberto Garcia Reviewed-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- blockdev.c | 21 ++------------------- include/qemu/throttle.h | 4 ---- tests/test-throttle.c | 18 +++++++++--------- util/throttle.c | 40 ++++++++-------------------------------- 4 files changed, 19 insertions(+), 64 deletions(-) (limited to 'util/throttle.c') diff --git a/blockdev.c b/blockdev.c index 11a3139851..73babeb14e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -343,23 +343,6 @@ static bool parse_stats_intervals(BlockAcctStats *stats, QList *intervals, return true; } -static bool check_throttle_config(ThrottleConfig *cfg, Error **errp) -{ - if (throttle_conflicting(cfg, errp)) { - return false; - } - - if (!throttle_is_valid(cfg, errp)) { - return false; - } - - if (throttle_max_is_missing_limit(cfg, errp)) { - return false; - } - - return true; -} - typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType; /* All parameters but @opts are optional and may be set to NULL. */ @@ -434,7 +417,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags, throttle_cfg->op_size = qemu_opt_get_number(opts, "throttling.iops-size", 0); - if (!check_throttle_config(throttle_cfg, errp)) { + if (!throttle_is_valid(throttle_cfg, errp)) { return; } } @@ -2660,7 +2643,7 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, cfg.op_size = iops_size; } - if (!check_throttle_config(&cfg, errp)) { + if (!throttle_is_valid(&cfg, errp)) { goto out; } diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index ecae6212ff..aec0785b39 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -106,12 +106,8 @@ bool throttle_timers_are_initialized(ThrottleTimers *tt); /* configuration */ bool throttle_enabled(ThrottleConfig *cfg); -bool throttle_conflicting(ThrottleConfig *cfg, Error **errp); - bool throttle_is_valid(ThrottleConfig *cfg, Error **errp); -bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp); - void throttle_config(ThrottleState *ts, ThrottleTimers *tt, ThrottleConfig *cfg); diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 3e208a8024..a0c17ac488 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -257,31 +257,31 @@ static void test_conflicts_for_one_set(bool is_max, int write) { memset(&cfg, 0, sizeof(cfg)); - g_assert(!throttle_conflicting(&cfg, NULL)); + g_assert(throttle_is_valid(&cfg, NULL)); set_cfg_value(is_max, total, 1); set_cfg_value(is_max, read, 1); - g_assert(throttle_conflicting(&cfg, NULL)); + g_assert(!throttle_is_valid(&cfg, NULL)); memset(&cfg, 0, sizeof(cfg)); set_cfg_value(is_max, total, 1); set_cfg_value(is_max, write, 1); - g_assert(throttle_conflicting(&cfg, NULL)); + g_assert(!throttle_is_valid(&cfg, NULL)); memset(&cfg, 0, sizeof(cfg)); set_cfg_value(is_max, total, 1); set_cfg_value(is_max, read, 1); set_cfg_value(is_max, write, 1); - g_assert(throttle_conflicting(&cfg, NULL)); + g_assert(!throttle_is_valid(&cfg, NULL)); memset(&cfg, 0, sizeof(cfg)); set_cfg_value(is_max, total, 1); - g_assert(!throttle_conflicting(&cfg, NULL)); + g_assert(throttle_is_valid(&cfg, NULL)); memset(&cfg, 0, sizeof(cfg)); set_cfg_value(is_max, read, 1); set_cfg_value(is_max, write, 1); - g_assert(!throttle_conflicting(&cfg, NULL)); + g_assert(throttle_is_valid(&cfg, NULL)); } static void test_conflicting_config(void) @@ -340,15 +340,15 @@ static void test_max_is_missing_limit(void) memset(&cfg, 0, sizeof(cfg)); cfg.buckets[i].max = 100; cfg.buckets[i].avg = 0; - g_assert(throttle_max_is_missing_limit(&cfg, NULL)); + g_assert(!throttle_is_valid(&cfg, NULL)); cfg.buckets[i].max = 0; cfg.buckets[i].avg = 0; - g_assert(!throttle_max_is_missing_limit(&cfg, NULL)); + g_assert(throttle_is_valid(&cfg, NULL)); cfg.buckets[i].max = 0; cfg.buckets[i].avg = 100; - g_assert(!throttle_max_is_missing_limit(&cfg, NULL)); + g_assert(throttle_is_valid(&cfg, NULL)); } } diff --git a/util/throttle.c b/util/throttle.c index 9046dd8e36..f8bf03c2e9 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -248,14 +248,14 @@ bool throttle_enabled(ThrottleConfig *cfg) return false; } -/* return true if any two throttling parameters conflicts - * +/* check if a throttling configuration is valid * @cfg: the throttling configuration to inspect - * @ret: true if any conflict detected else false + * @ret: true if valid else false * @errp: error object */ -bool throttle_conflicting(ThrottleConfig *cfg, Error **errp) +bool throttle_is_valid(ThrottleConfig *cfg, Error **errp) { + int i; bool bps_flag, ops_flag; bool bps_max_flag, ops_max_flag; @@ -278,21 +278,9 @@ bool throttle_conflicting(ThrottleConfig *cfg, Error **errp) if (bps_flag || ops_flag || bps_max_flag || ops_max_flag) { error_setg(errp, "bps/iops/max total values and read/write values" " cannot be used at the same time"); - return true; + return false; } - return false; -} - -/* check if a throttling configuration is valid - * @cfg: the throttling configuration to inspect - * @ret: true if valid else false - * @errp: error object - */ -bool throttle_is_valid(ThrottleConfig *cfg, Error **errp) -{ - int i; - for (i = 0; i < BUCKETS_COUNT; i++) { if (cfg->buckets[i].avg < 0 || cfg->buckets[i].max < 0 || @@ -302,27 +290,15 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp) THROTTLE_VALUE_MAX); return false; } - } - - return true; -} -/* check if bps_max/iops_max is used without bps/iops - * @cfg: the throttling configuration to inspect - * @errp: error object - */ -bool throttle_max_is_missing_limit(ThrottleConfig *cfg, Error **errp) -{ - int i; - - for (i = 0; i < BUCKETS_COUNT; i++) { if (cfg->buckets[i].max && !cfg->buckets[i].avg) { error_setg(errp, "bps_max/iops_max require corresponding" " bps/iops values"); - return true; + return false; } } - return false; + + return true; } /* fix bucket parameters */ -- cgit 1.4.1 From 1588ab5d0b96301b893d63aa342faad0e37a02ff Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 18 Feb 2016 12:27:00 +0200 Subject: throttle: Use throttle_config_init() to initialize ThrottleConfig We can currently initialize ThrottleConfig by zeroing all its fields, but this will change with the new fields to define the length of the burst periods. This patch introduces a new throttle_config_init() function and uses it to replace all memset() calls that initialize ThrottleConfig directly. Signed-off-by: Alberto Garcia Reviewed-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- blockdev.c | 4 ++-- include/qemu/throttle.h | 2 ++ tests/test-throttle.c | 28 +++++++++++++++++----------- util/throttle.c | 10 ++++++++++ 4 files changed, 31 insertions(+), 13 deletions(-) (limited to 'util/throttle.c') diff --git a/blockdev.c b/blockdev.c index 73babeb14e..e01486e896 100644 --- a/blockdev.c +++ b/blockdev.c @@ -387,7 +387,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags, } if (throttle_cfg) { - memset(throttle_cfg, 0, sizeof(*throttle_cfg)); + throttle_config_init(throttle_cfg); throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg = qemu_opt_get_number(opts, "throttling.bps-total", 0); throttle_cfg->buckets[THROTTLE_BPS_READ].avg = @@ -2611,7 +2611,7 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, goto out; } - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps; cfg.buckets[THROTTLE_BPS_READ].avg = bps_rd; cfg.buckets[THROTTLE_BPS_WRITE].avg = bps_wr; diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index aec0785b39..8ec8951225 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -114,6 +114,8 @@ void throttle_config(ThrottleState *ts, void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg); +void throttle_config_init(ThrottleConfig *cfg); + /* usage */ bool throttle_schedule_timer(ThrottleState *ts, ThrottleTimers *tt, diff --git a/tests/test-throttle.c b/tests/test-throttle.c index a0c17ac488..34f1f9efa1 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -35,6 +35,9 @@ static bool double_cmp(double x, double y) /* tests for single bucket operations */ static void test_leak_bucket(void) { + throttle_config_init(&cfg); + bkt = cfg.buckets[THROTTLE_BPS_TOTAL]; + /* set initial value */ bkt.avg = 150; bkt.max = 15; @@ -64,6 +67,9 @@ static void test_compute_wait(void) int64_t wait; int64_t result; + throttle_config_init(&cfg); + bkt = cfg.buckets[THROTTLE_BPS_TOTAL]; + /* no operation limit set */ bkt.avg = 0; bkt.max = 15; @@ -233,17 +239,17 @@ static void test_enabled(void) { int i; - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); g_assert(!throttle_enabled(&cfg)); for (i = 0; i < BUCKETS_COUNT; i++) { - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); set_cfg_value(false, i, 150); g_assert(throttle_enabled(&cfg)); } for (i = 0; i < BUCKETS_COUNT; i++) { - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); set_cfg_value(false, i, -150); g_assert(!throttle_enabled(&cfg)); } @@ -256,29 +262,29 @@ static void test_conflicts_for_one_set(bool is_max, int read, int write) { - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); g_assert(throttle_is_valid(&cfg, NULL)); set_cfg_value(is_max, total, 1); set_cfg_value(is_max, read, 1); g_assert(!throttle_is_valid(&cfg, NULL)); - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); set_cfg_value(is_max, total, 1); set_cfg_value(is_max, write, 1); g_assert(!throttle_is_valid(&cfg, NULL)); - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); set_cfg_value(is_max, total, 1); set_cfg_value(is_max, read, 1); set_cfg_value(is_max, write, 1); g_assert(!throttle_is_valid(&cfg, NULL)); - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); set_cfg_value(is_max, total, 1); g_assert(throttle_is_valid(&cfg, NULL)); - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); set_cfg_value(is_max, read, 1); set_cfg_value(is_max, write, 1); g_assert(throttle_is_valid(&cfg, NULL)); @@ -315,7 +321,7 @@ static void test_is_valid_for_value(int value, bool should_be_valid) int is_max, index; for (is_max = 0; is_max < 2; is_max++) { for (index = 0; index < BUCKETS_COUNT; index++) { - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); set_cfg_value(is_max, index, value); g_assert(throttle_is_valid(&cfg, NULL) == should_be_valid); } @@ -337,7 +343,7 @@ static void test_max_is_missing_limit(void) int i; for (i = 0; i < BUCKETS_COUNT; i++) { - memset(&cfg, 0, sizeof(cfg)); + throttle_config_init(&cfg); cfg.buckets[i].max = 100; cfg.buckets[i].avg = 0; g_assert(!throttle_is_valid(&cfg, NULL)); @@ -552,7 +558,7 @@ static void test_groups(void) g_assert(bdrv1->throttle_state == bdrv3->throttle_state); /* Setting the config of a group member affects the whole group */ - memset(&cfg1, 0, sizeof(cfg1)); + throttle_config_init(&cfg1); cfg1.buckets[THROTTLE_BPS_READ].avg = 500000; cfg1.buckets[THROTTLE_BPS_WRITE].avg = 285000; cfg1.buckets[THROTTLE_OPS_READ].avg = 20000; diff --git a/util/throttle.c b/util/throttle.c index f8bf03c2e9..6a01cee892 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -171,10 +171,20 @@ void throttle_timers_attach_aio_context(ThrottleTimers *tt, tt->write_timer_cb, tt->timer_opaque); } +/* + * Initialize the ThrottleConfig structure to a valid state + * @cfg: the config to initialize + */ +void throttle_config_init(ThrottleConfig *cfg) +{ + memset(cfg, 0, sizeof(*cfg)); +} + /* To be called first on the ThrottleState */ void throttle_init(ThrottleState *ts) { memset(ts, 0, sizeof(ThrottleState)); + throttle_config_init(&ts->cfg); } /* To be called first on the ThrottleTimers */ -- cgit 1.4.1 From 100f8f26086ad85a9361f2883edd55bc337ce594 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 18 Feb 2016 12:27:01 +0200 Subject: throttle: Add support for burst periods This patch adds support for burst periods to the throttling code. With this feature the user can keep performing bursts as defined by the LeakyBucket.max rate for a configurable period of time. Signed-off-by: Alberto Garcia Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- include/qemu/throttle.h | 41 +++++++++++++++++++++++---- util/throttle.c | 73 ++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 96 insertions(+), 18 deletions(-) (limited to 'util/throttle.c') diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index 8ec8951225..63df69070a 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -2,7 +2,7 @@ * QEMU throttling infrastructure * * Copyright (C) Nodalink, EURL. 2013-2014 - * Copyright (C) Igalia, S.L. 2015 + * Copyright (C) Igalia, S.L. 2015-2016 * * Authors: * Benoît Canet @@ -42,16 +42,47 @@ typedef enum { } BucketType; /* - * The max parameter of the leaky bucket throttling algorithm can be used to - * allow the guest to do bursts. - * The max value is a pool of I/O that the guest can use without being throttled - * at all. Throttling is triggered once this pool is empty. + * This module implements I/O limits using the leaky bucket + * algorithm. The code is independent of the I/O units, but it is + * currently used for bytes per second and operations per second. + * + * Three parameters can be set by the user: + * + * - avg: the desired I/O limits in units per second. + * - max: the limit during bursts, also in units per second. + * - burst_length: the maximum length of the burst period, in seconds. + * + * Here's how it works: + * + * - The bucket level (number of performed I/O units) is kept in + * bkt.level and leaks at a rate of bkt.avg units per second. + * + * - The size of the bucket is bkt.max * bkt.burst_length. Once the + * bucket is full no more I/O is performed until the bucket leaks + * again. This is what makes the I/O rate bkt.avg. + * + * - The bkt.avg rate does not apply until the bucket is full, + * allowing the user to do bursts until then. The I/O limit during + * bursts is bkt.max. To enforce this limit we keep an additional + * bucket in bkt.burst_length that leaks at a rate of bkt.max units + * per second. + * + * - Because of all of the above, the user can perform I/O at a + * maximum of bkt.max units per second for at most bkt.burst_length + * seconds in a row. After that the bucket will be full and the I/O + * rate will go down to bkt.avg. + * + * - Since the bucket always leaks at a rate of bkt.avg, this also + * determines how much the user needs to wait before being able to + * do bursts again. */ typedef struct LeakyBucket { double avg; /* average goal in units per second */ double max; /* leaky bucket max burst in units */ double level; /* bucket level in units */ + double burst_level; /* bucket level in units (for computing bursts) */ + unsigned burst_length; /* max length of the burst period, in seconds */ } LeakyBucket; /* The following structure is used to configure a ThrottleState diff --git a/util/throttle.c b/util/throttle.c index 6a01cee892..371c769455 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -41,6 +41,14 @@ void throttle_leak_bucket(LeakyBucket *bkt, int64_t delta_ns) /* make the bucket leak */ bkt->level = MAX(bkt->level - leak, 0); + + /* if we allow bursts for more than one second we also need to + * keep track of bkt->burst_level so the bkt->max goal per second + * is attained */ + if (bkt->burst_length > 1) { + leak = (bkt->max * (double) delta_ns) / NANOSECONDS_PER_SECOND; + bkt->burst_level = MAX(bkt->burst_level - leak, 0); + } } /* Calculate the time delta since last leak and make proportionals leaks @@ -91,13 +99,24 @@ int64_t throttle_compute_wait(LeakyBucket *bkt) return 0; } - extra = bkt->level - bkt->max; + /* If the bucket is full then we have to wait */ + extra = bkt->level - bkt->max * bkt->burst_length; + if (extra > 0) { + return throttle_do_compute_wait(bkt->avg, extra); + } - if (extra <= 0) { - return 0; + /* If the bucket is not full yet we have to make sure that we + * fulfill the goal of bkt->max units per second. */ + 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; + if (extra > 0) { + return throttle_do_compute_wait(bkt->max, extra); + } } - return throttle_do_compute_wait(bkt->avg, extra); + return 0; } /* This function compute the time that must be waited while this IO @@ -177,7 +196,11 @@ void throttle_timers_attach_aio_context(ThrottleTimers *tt, */ void throttle_config_init(ThrottleConfig *cfg) { + unsigned i; memset(cfg, 0, sizeof(*cfg)); + for (i = 0; i < BUCKETS_COUNT; i++) { + cfg->buckets[i].burst_length = 1; + } } /* To be called first on the ThrottleState */ @@ -301,6 +324,16 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp) return false; } + if (!cfg->buckets[i].burst_length) { + error_setg(errp, "the burst length cannot be 0"); + return false; + } + + if (cfg->buckets[i].burst_length > 1 && !cfg->buckets[i].max) { + error_setg(errp, "burst length set without burst rate"); + return false; + } + if (cfg->buckets[i].max && !cfg->buckets[i].avg) { error_setg(errp, "bps_max/iops_max require corresponding" " bps/iops values"); @@ -317,7 +350,7 @@ static void throttle_fix_bucket(LeakyBucket *bkt) double min; /* zero bucket level */ - bkt->level = 0; + bkt->level = bkt->burst_level = 0; /* The following is done to cope with the Linux CFQ block scheduler * which regroup reads and writes by block of 100ms in the guest. @@ -420,22 +453,36 @@ bool throttle_schedule_timer(ThrottleState *ts, */ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size) { + const BucketType bucket_types_size[2][2] = { + { THROTTLE_BPS_TOTAL, THROTTLE_BPS_READ }, + { THROTTLE_BPS_TOTAL, THROTTLE_BPS_WRITE } + }; + const BucketType bucket_types_units[2][2] = { + { THROTTLE_OPS_TOTAL, THROTTLE_OPS_READ }, + { THROTTLE_OPS_TOTAL, THROTTLE_OPS_WRITE } + }; double units = 1.0; + unsigned i; /* if cfg.op_size is defined and smaller than size we compute unit count */ if (ts->cfg.op_size && size > ts->cfg.op_size) { units = (double) size / ts->cfg.op_size; } - ts->cfg.buckets[THROTTLE_BPS_TOTAL].level += size; - ts->cfg.buckets[THROTTLE_OPS_TOTAL].level += units; + for (i = 0; i < 2; i++) { + LeakyBucket *bkt; + + bkt = &ts->cfg.buckets[bucket_types_size[is_write][i]]; + bkt->level += size; + if (bkt->burst_length > 1) { + bkt->burst_level += size; + } - if (is_write) { - ts->cfg.buckets[THROTTLE_BPS_WRITE].level += size; - ts->cfg.buckets[THROTTLE_OPS_WRITE].level += units; - } else { - ts->cfg.buckets[THROTTLE_BPS_READ].level += size; - ts->cfg.buckets[THROTTLE_OPS_READ].level += units; + bkt = &ts->cfg.buckets[bucket_types_units[is_write][i]]; + bkt->level += units; + if (bkt->burst_length > 1) { + bkt->burst_level += units; + } } } -- cgit 1.4.1