On Tue, 10/07 15:24, Benoît Canet wrote:
> The throttle group support use a cooperative round robin scheduling algorithm.
>
> The principle of the algorithm are simple:
s/principle/principles/
> - Each BDS of the group is used as a token in a circular way.
> - The active BDS compute if a wait must be done and arm the right timer.
> - If a wait must be done the token timer will be armed so the token will
> become
> the next active BDS.
>
> Signed-off-by: Benoit Canet <benoit.ca...@nodalink.com>
> ---
> block.c | 191
> ++++++++++++++++++++++++++++++++++++++++------
> block/qapi.c | 7 +-
> block/throttle-groups.c | 2 +-
> blockdev.c | 19 ++++-
> hmp.c | 4 +-
> include/block/block.h | 3 +-
> include/block/block_int.h | 9 ++-
> qapi/block-core.json | 5 +-
> qemu-options.hx | 1 +
> qmp-commands.hx | 3 +-
> 10 files changed, 209 insertions(+), 35 deletions(-)
>
> diff --git a/block.c b/block.c
> index 527ea48..e7e5607 100644
> --- a/block.c
> +++ b/block.c
> @@ -36,6 +36,7 @@
> #include "qmp-commands.h"
> #include "qemu/timer.h"
> #include "qapi-event.h"
> +#include "block/throttle-groups.h"
>
> #ifdef CONFIG_BSD
> #include <sys/types.h>
> @@ -129,7 +130,9 @@ void bdrv_set_io_limits(BlockDriverState *bs,
> {
> int i;
>
> - throttle_config(&bs->throttle_state, &bs->throttle_timers, cfg);
> + throttle_group_lock(bs->throttle_state);
> + throttle_config(bs->throttle_state, &bs->throttle_timers, cfg);
> + throttle_group_unlock(bs->throttle_state);
>
> for (i = 0; i < 2; i++) {
> qemu_co_enter_next(&bs->throttled_reqs[i]);
> @@ -156,34 +159,99 @@ static bool bdrv_start_throttled_reqs(BlockDriverState
> *bs)
> return drained;
> }
>
> +static void bdrv_throttle_group_add(BlockDriverState *bs)
> +{
> + int i;
> + BlockDriverState *token;
> +
> + for (i = 0; i < 2; i++) {
> + /* Get the BlockDriverState having the round robin token */
> + token = throttle_group_token(bs->throttle_state, i);
> +
> + /* If the ThrottleGroup is new set the current BlockDriverState as
> + * token
> + */
> + if (!token) {
> + throttle_group_set_token(bs->throttle_state, bs, i);
> + }
> +
> + }
> +
> + throttle_group_register_bs(bs->throttle_state, bs);
> +}
> +
> +static void bdrv_throttle_group_remove(BlockDriverState *bs)
> +{
> + BlockDriverState *token;
> + int i;
> +
> + for (i = 0; i < 2; i++) {
> + /* Get the BlockDriverState having the round robin token */
> + token = throttle_group_token(bs->throttle_state, i);
> + /* if this bs is the current token set the next bs as token */
> + if (token == bs) {
> + token = throttle_group_next_bs(token);
> + /* take care of the case where bs is the only bs of the group */
> + if (token == bs) {
> + token = NULL;
> + }
> + throttle_group_set_token(bs->throttle_state, token, i);
> + }
> + }
> +
> + /* remove the current bs from the list */
> + QLIST_REMOVE(bs, round_robin);
> +}
> +
> void bdrv_io_limits_disable(BlockDriverState *bs)
> {
> +
> + throttle_group_lock(bs->throttle_state);
> bs->io_limits_enabled = false;
> + throttle_group_unlock(bs->throttle_state);
>
> bdrv_start_throttled_reqs(bs);
>
> + throttle_group_lock(bs->throttle_state);
> + bdrv_throttle_group_remove(bs);
> + throttle_group_unlock(bs->throttle_state);
> +
> + throttle_group_unref(bs->throttle_state);
> + bs->throttle_state = NULL;
> +
> throttle_timers_destroy(&bs->throttle_timers);
> }
>
> static void bdrv_throttle_read_timer_cb(void *opaque)
> {
> BlockDriverState *bs = opaque;
> - throttle_timer_fired(&bs->throttle_state, false);
> +
> + throttle_group_lock(bs->throttle_state);
> + throttle_timer_fired(bs->throttle_state, false);
> + throttle_group_unlock(bs->throttle_state);
> +
> qemu_co_enter_next(&bs->throttled_reqs[0]);
> }
>
> static void bdrv_throttle_write_timer_cb(void *opaque)
> {
> BlockDriverState *bs = opaque;
> - throttle_timer_fired(&bs->throttle_state, true);
> +
> + throttle_group_lock(bs->throttle_state);
> + throttle_timer_fired(bs->throttle_state, true);
> + throttle_group_unlock(bs->throttle_state);
> +
> qemu_co_enter_next(&bs->throttled_reqs[1]);
> }
>
> /* should be called before bdrv_set_io_limits if a limit is set */
> -void bdrv_io_limits_enable(BlockDriverState *bs)
> +void bdrv_io_limits_enable(BlockDriverState *bs, const char *group)
Does this mean that after this series, all the throttle_states must be
contained inside its own throttle group? If so, we could embed ThrottleGroup
fields in ThrottleState.
It's weird when a function called throttle_group_compare takes a parameter of
ThrottleState pointer, and cast it back to ThrottleGroup with container_of.
Fam
