On Wed, Jun 07, 2017 at 04:06:19PM +0300, Manos Pitsidianakis wrote: > +#define QEMU_OPT_IOPS_TOTAL "iops-total" > +#define QEMU_OPT_IOPS_TOTAL_MAX "iops-total-max" > +#define QEMU_OPT_IOPS_TOTAL_MAX_LENGTH "iops-total-max-length" > +#define QEMU_OPT_IOPS_READ "iops-read" > +#define QEMU_OPT_IOPS_READ_MAX "iops-read-max" > +#define QEMU_OPT_IOPS_READ_MAX_LENGTH "iops-read-max-length" > +#define QEMU_OPT_IOPS_WRITE "iops-write" > +#define QEMU_OPT_IOPS_WRITE_MAX "iops-write-max" > +#define QEMU_OPT_IOPS_WRITE_MAX_LENGTH "iops-write-max-length" > +#define QEMU_OPT_BPS_TOTAL "bps-total" > +#define QEMU_OPT_BPS_TOTAL_MAX "bps-total-max" > +#define QEMU_OPT_BPS_TOTAL_MAX_LENGTH "bps-total-max-length" > +#define QEMU_OPT_BPS_READ "bps-read" > +#define QEMU_OPT_BPS_READ_MAX "bps-read-max" > +#define QEMU_OPT_BPS_READ_MAX_LENGTH "bps-read-max-length" > +#define QEMU_OPT_BPS_WRITE "bps-write" > +#define QEMU_OPT_BPS_WRITE_MAX "bps-write-max" > +#define QEMU_OPT_BPS_WRITE_MAX_LENGTH "bps-write-max-length" > +#define QEMU_OPT_IOPS_SIZE "iops-size" > +#define QEMU_OPT_THROTTLE_GROUP_NAME "throttle-group"
Please reuse include/qemu/throttle-options.h.
> +static QemuMutex throttle_groups_lock;
> +static QTAILQ_HEAD(, ThrottleGroup) throttle_groups =
> + QTAILQ_HEAD_INITIALIZER(throttle_groups);
Please reuse block/throttle-groups.c. I think throttle-groups.c doesn't
actually need BlockBackend so you can make the code more general.
Define the necessary state:
typedef struct ThrottleGroupMember {
/* Protected by AioContext lock. */
CoQueue throttled_reqs[2];
/* Nonzero if the I/O limits are currently being ignored; generally
* it is zero. */
unsigned int io_limits_disabled;
/* The following fields are protected by the ThrottleGroup lock.
* See the ThrottleGroup documentation for details.
* throttle_state tells us if I/O limits are configured. */
ThrottleState *throttle_state;
ThrottleTimers throttle_timers;
unsigned pending_reqs[2];
QLIST_ENTRY(ThrottleGroupMember) round_robin;
} ThrottleGroupMember;
Then refactor throttle-groups.c to use ThrottleGroupMember instead of
BlockBackend(Public).
> diff --git a/include/block/throttle.h b/include/block/throttle.h
> new file mode 100644
> index 0000000000..fed24f02a6
> --- /dev/null
> +++ b/include/block/throttle.h
What is the purpose of this header?
Header files in include/* are "public" APIs that other parts of QEMU can
use. There is no user other than block/throttle.c so please keep it
private in the .c file.
> @@ -0,0 +1,59 @@
> +/* The ThrottleGroup structure (with its ThrottleState) is shared
> + * among different BlockDriverStates and it's independent from
> + * AioContext, so in order to use it from different threads it needs
> + * its own locking.
> + *
> + * This locking is however handled internally in block/throttle.c so it's
> + * transparent to outside users.
> + *
> + * The whole ThrottleGroup structure is private and invisible to
> + * outside users, that only use it through its ThrottleState.
> + *
> + * In addition to the ThrottleGroup structure, BlockDriverState has
> + * fields that need to be accessed by other members of the group and
> + * therefore also need to be protected by this lock. Once a
> + * BlockDriverState is registered in a group those fields can be accessed
> + * by other threads any time.
> + *
> + * Again, all this is handled internally in block/throttle.c and is mostly
> + * transparent to the outside. The 'throttle_timers' field however has an
> + * additional constraint because it may be temporarily invalid (see for
> example
> + * bdrv_set_aio_context()). Therefore block/throttle.c will access some
> + * other BlockDriverState's timers only after verifying that that BDS has
> + * throttled requests in the queue.
> + */
> +
> +typedef struct ThrottleGroup {
> + char *name; /* This is constant during the lifetime of the group */
> +
> + QemuMutex lock; /* This lock protects the following four fields */
> + ThrottleState ts;
> + QLIST_HEAD(, BDRVThrottleNodeState) head;
> + struct BDRVThrottleNodeState *tokens[2];
> + bool any_timer_armed[2];
> +
> + /* These two are protected by the global throttle_groups_lock */
> + unsigned refcount;
> + QTAILQ_ENTRY(ThrottleGroup) list;
> +} ThrottleGroup;
> +
> +typedef struct BDRVThrottleNodeState {
> + ThrottleGroup *throttle_group;
> +
> + /* I/O throttling has its own locking, but also some fields are
> + * protected by the AioContext lock.
> + */
> +
> + /* Protected by AioContext lock. */
> + CoQueue throttled_reqs[2];
> +
> + /* Nonzero if the I/O limits are currently being ignored; generally
> + * it is zero. */
> + unsigned int io_limits_disabled;
> +
> + ThrottleState *throttle_state;
> + ThrottleTimers throttle_timers;
> + unsigned pending_reqs[2];
> + QLIST_ENTRY(BDRVThrottleNodeState) round_robin;
> +
> +} BDRVThrottleNodeState;
> --
> 2.11.0
>
signature.asc
Description: PGP signature
