On Thu, 22 Sep 2016 07:59:19 -0400
Pradeep Jagadeesh <pradeepkiruv...@gmail.com> wrote:
> Uses throttling APIs to limit I/O bandwidth and number of operations on the
> devices which use 9p-local driver.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagade...@huawei.com>
> ---
Hi Pradeep,
So where are we with this patch ? Have you solved the issues you mentioned in
<CAJ2SuLn9Bz4=kzx61KVaG_E=wid9w_uafjdhwhb7zis4yui...@mail.gmail.com> ?
I don't have much time so I'll wait for v5 to comment.
And maybe you can push the option list to a THROTTLE_COMMON_OPTS macro
in a include/qemu/throttle-options.h header file, to be included by
both blockdev.c and fsdev/qemu-fsdev-throttle.c.
Cheers.
--
Greg
> fsdev/Makefile.objs | 1 +
> fsdev/file-op-9p.h | 3 +
> fsdev/qemu-fsdev-opts.c | 76 +++++++++++++++++++++++
> fsdev/qemu-fsdev-throttle.c | 146
> ++++++++++++++++++++++++++++++++++++++++++++
> fsdev/qemu-fsdev-throttle.h | 36 +++++++++++
> hw/9pfs/9p-local.c | 9 ++-
> hw/9pfs/9p.c | 6 ++
> hw/9pfs/cofile.c | 3 +
> 8 files changed, 278 insertions(+), 2 deletions(-)
> create mode 100644 fsdev/qemu-fsdev-throttle.c
> create mode 100644 fsdev/qemu-fsdev-throttle.h
>
> This adds the support for the 9p-local driver.
> For now this functionality can be enabled only through qemu cli options.
> QMP interface and support to other drivers need further extensions.
> To make it simple for other drivers, the throttle code has been put in
> separate files.
>
> v1 -> v2:
>
> -Fixed FsContext redeclaration issue
> -Removed couple of function declarations from 9p-throttle.h
> -Fixed some of the .help messages
>
> v2 -> v3:
>
> -Addressed follwing comments by Claudio Fontana
> -Removed redundant memset calls in fsdev_throttle_configure_iolimits function
> -Checking throttle structure validity before initializing other structures
> in fsdev_throttle_configure_iolimits
>
> -Addressed following comments by Greg Kurz
> -Moved the code from 9pfs directory to fsdev directory, because the
> throttling
> is for the fsdev devices.Renamed the files and functions to fsdev_ from
> 9pfs_
> -Renamed throttling cli options to throttling.*, as in QMP cli options
> -Removed some of the unwanted .h files from qemu-fsdev-throttle.[ch]
> -Using throttle_enabled() function to set the thottle enabled flag for fsdev.
>
> v3 -> v4:
>
> -Addressed following comments by Alberto Garcia
> -Removed the unwanted locking and other data structures in
> qemu-fsdev-throttle.[ch]
>
> -Addressed following comments by Greg Kurz
> -Removed fsdev_iolimitsenable/disable functions, instead using
> throttle_enabled function
>
>
>
> diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
> index 1b120a4..2c6da2d 100644
> --- a/fsdev/Makefile.objs
> +++ b/fsdev/Makefile.objs
> @@ -7,6 +7,7 @@ common-obj-y = qemu-fsdev-dummy.o
> endif
> common-obj-y += qemu-fsdev-opts.o
>
> +common-obj-y += qemu-fsdev-throttle.o
> # Toplevel always builds this; targets without virtio will put it in
> # common-obj-y
> common-obj-$(CONFIG_ALL) += qemu-fsdev-dummy.o
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index 6db9fea..33fe822 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -17,6 +17,7 @@
> #include <dirent.h>
> #include <utime.h>
> #include <sys/vfs.h>
> +#include "qemu-fsdev-throttle.h"
>
> #define SM_LOCAL_MODE_BITS 0600
> #define SM_LOCAL_DIR_MODE_BITS 0700
> @@ -74,6 +75,7 @@ typedef struct FsDriverEntry {
> char *path;
> int export_flags;
> FileOperations *ops;
> + FsThrottle fst;
> } FsDriverEntry;
>
> typedef struct FsContext
> @@ -83,6 +85,7 @@ typedef struct FsContext
> int export_flags;
> struct xattr_operations **xops;
> struct extended_ops exops;
> + FsThrottle *fst;
> /* fs driver specific data */
> void *private;
> } FsContext;
> diff --git a/fsdev/qemu-fsdev-opts.c b/fsdev/qemu-fsdev-opts.c
> index 1dd8c7a..395d497 100644
> --- a/fsdev/qemu-fsdev-opts.c
> +++ b/fsdev/qemu-fsdev-opts.c
> @@ -37,6 +37,82 @@ static QemuOptsList qemu_fsdev_opts = {
> }, {
> .name = "sock_fd",
> .type = QEMU_OPT_NUMBER,
> + }, {
> + .name = "throttling.iops-total",
> + .type = QEMU_OPT_NUMBER,
> + .help = "limit total I/O operations per second",
> + },{
> + .name = "throttling.iops-read",
> + .type = QEMU_OPT_NUMBER,
> + .help = "limit read operations per second",
> + },{
> + .name = "throttling.iops-write",
> + .type = QEMU_OPT_NUMBER,
> + .help = "limit write operations per second",
> + },{
> + .name = "throttling.bps-total",
> + .type = QEMU_OPT_NUMBER,
> + .help = "limit total bytes per second",
> + },{
> + .name = "throttling.bps-read",
> + .type = QEMU_OPT_NUMBER,
> + .help = "limit read bytes per second",
> + },{
> + .name = "throttling.bps-write",
> + .type = QEMU_OPT_NUMBER,
> + .help = "limit write bytes per second",
> + },{
> + .name = "throttling.iops-total-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "I/O operations burst",
> + },{
> + .name = "throttling.iops-read-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "I/O operations read burst",
> + },{
> + .name = "throttling.iops-write-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "I/O operations write burst",
> + },{
> + .name = "throttling.bps-total-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "total bytes burst",
> + },{
> + .name = "throttling.bps-read-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "total bytes read burst",
> + },{
> + .name = "throttling.bps-write-max",
> + .type = QEMU_OPT_NUMBER,
> + .help = "total bytes write burst",
> + },{
> + .name = "throttling.iops-total-max-length",
> + .type = QEMU_OPT_NUMBER,
> + .help = "length of the iops-total-max burst period, in seconds",
> + },{
> + .name = "throttling.iops-read-max-length",
> + .type = QEMU_OPT_NUMBER,
> + .help = "length of the iops-read-max burst period, in seconds",
> + },{
> + .name = "throttling.iops-write-max-length",
> + .type = QEMU_OPT_NUMBER,
> + .help = "length of the iops-write-max burst period, in seconds",
> + },{
> + .name = "throttling.bps-total-max-length",
> + .type = QEMU_OPT_NUMBER,
> + .help = "length of the bps-total-max burst period, in seconds",
> + },{
> + .name = "throttling.bps-read-max-length",
> + .type = QEMU_OPT_NUMBER,
> + .help = "length of the bps-read-max burst period, in seconds",
> + },{
> + .name = "throttling.bps-write-max-length",
> + .type = QEMU_OPT_NUMBER,
> + .help = "length of the bps-write-max burst period, in seconds",
> + },{
> + .name = "throttling.iops-size",
> + .type = QEMU_OPT_NUMBER,
> + .help = "when limiting by iops max size of an I/O in bytes",
> },
>
> { /*End of list */ }
> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
> new file mode 100644
> index 0000000..704c687
> --- /dev/null
> +++ b/fsdev/qemu-fsdev-throttle.c
> @@ -0,0 +1,146 @@
> +/*
> + * Fsdev Throttle
> + *
> + * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
> + *
> + * Author: Pradeep Jagadeesh <pradeep.jagade...@huawei.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version.
> + *
> + * See the COPYING file in the top-level directory for details.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +#include "qemu-fsdev-throttle.h"
> +
> +static bool fsdev_throttle_check_for_wait(FsThrottle *fst, bool is_write)
> +{
> + return throttle_schedule_timer(&fst->ts, &fst->tt, is_write);
> +}
> +
> +static void fsdev_throttle_schedule_next_request(FsThrottle *fst, bool
> is_write)
> +{
> + bool must_wait = fsdev_throttle_check_for_wait(fst, is_write);
> + if (!must_wait) {
> + if (qemu_in_coroutine() &&
> + qemu_co_queue_next(&fst->throttled_reqs[is_write])) {
> + ;
> + } else {
> + int64_t now = qemu_clock_get_ns(fst->tt.clock_type);
> + timer_mod(fst->tt.timers[is_write], now + 1);
> + }
> + }
> +}
> +
> +static void fsdev_throttle_timer_cb(FsThrottle *fst, bool is_write)
> +{
> + bool empty_queue;
> +
> + empty_queue = !qemu_co_enter_next(&fst->throttled_reqs[is_write]);
> + if (empty_queue) {
> + fsdev_throttle_schedule_next_request(fst, is_write);
> + }
> +}
> +
> +static void fsdev_throttle_read_timer_cb(void *opaque)
> +{
> + fsdev_throttle_timer_cb(opaque, false);
> +}
> +
> +static void fsdev_throttle_write_timer_cb(void *opaque)
> +{
> + fsdev_throttle_timer_cb(opaque, true);
> +}
> +
> +static void fsdev_parse_io_limit_opts(QemuOpts *opts, FsThrottle *fst)
> +{
> + fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
> + qemu_opt_get_number(opts, "throttling.bps-total", 0);
> + fst->cfg.buckets[THROTTLE_BPS_READ].avg =
> + qemu_opt_get_number(opts, "throttling.bps-read", 0);
> + fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
> + qemu_opt_get_number(opts, "throttling.bps-write", 0);
> + fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
> + qemu_opt_get_number(opts, "throttling.iops-total", 0);
> + fst->cfg.buckets[THROTTLE_OPS_READ].avg =
> + qemu_opt_get_number(opts, "throttling.iops-read", 0);
> + fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
> + qemu_opt_get_number(opts, "throttling.iops-write", 0);
> +
> + fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
> + qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> + fst->cfg.buckets[THROTTLE_BPS_READ].max =
> + qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> + fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
> + qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> + fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
> + qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> + fst->cfg.buckets[THROTTLE_OPS_READ].max =
> + qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> + fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
> + qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> +
> + fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
> + qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> + fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =
> + qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> + fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
> + qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> + fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
> + qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> + fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
> + qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> + fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
> + qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> + fst->cfg.op_size =
> + qemu_opt_get_number(opts, "throttling.iops-size", 0);
> +}
> +
> +int fsdev_throttle_configure_iolimits(QemuOpts *opts, FsThrottle *fst)
> +{
> + Error *err = NULL;
> +
> + /* Parse and set populate the cfg structure */
> + fsdev_parse_io_limit_opts(opts, fst);
> +
> + if (!throttle_is_valid(&fst->cfg, &err)) {
> + error_reportf_err(err, "Throttle configuration is not valid: ");
> + return -1;
> + }
> + if (throttle_enabled(&fst->cfg)) {
> + g_assert((fst->aioctx = qemu_get_aio_context()));
> + throttle_init(&fst->ts);
> + throttle_timers_init(&fst->tt,
> + fst->aioctx,
> + QEMU_CLOCK_REALTIME,
> + fsdev_throttle_read_timer_cb,
> + fsdev_throttle_write_timer_cb,
> + fst);
> + throttle_config(&fst->ts, &fst->tt, &fst->cfg);
> + qemu_co_queue_init(&fst->throttled_reqs[0]);
> + qemu_co_queue_init(&fst->throttled_reqs[1]);
> + }
> + return 0;
> +}
> +
> +void fsdev_throttle_request(FsThrottle *fst, bool is_write, ssize_t bytes)
> +{
> + bool must_wait;
> +
> + if (throttle_enabled(&fst->cfg)) {
> + must_wait = fsdev_throttle_check_for_wait(fst, is_write);
> + if (must_wait) {
> + qemu_co_queue_wait(&fst->throttled_reqs[is_write]);
> + }
> + throttle_account(&fst->ts, is_write, bytes);
> + fsdev_throttle_schedule_next_request(fst, is_write);
> + }
> +}
> +void fsdev_throttle_cleanup(FsThrottle *fst)
> +{
> + throttle_timers_destroy(&fst->tt);
> +}
> diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
> new file mode 100644
> index 0000000..5ae452c
> --- /dev/null
> +++ b/fsdev/qemu-fsdev-throttle.h
> @@ -0,0 +1,36 @@
> +/*
> + * Fsdev Throttle
> + *
> + * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
> + *
> + * Author: Pradeep Jagadeesh <pradeep.jagade...@huawei.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version.
> + *
> + * See the COPYING file in the top-level directory for details.
> + *
> + */
> +
> +#ifndef _FSDEV_THROTTLE_H
> +#define _FSDEV_THROTTLE_H
> +
> +#include "block/aio.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/coroutine.h"
> +#include "qemu/throttle.h"
> +
> +typedef struct FsThrottle {
> + ThrottleState ts;
> + ThrottleTimers tt;
> + AioContext *aioctx;
> + ThrottleConfig cfg;
> + CoQueue throttled_reqs[2];
> +} FsThrottle;
> +
> +int fsdev_throttle_configure_iolimits(QemuOpts *, FsThrottle *);
> +
> +void fsdev_throttle_request(FsThrottle *, bool , ssize_t);
> +
> +void fsdev_throttle_cleanup(FsThrottle *);
> +#endif /* _FSDEV_THROTTLE_H */
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 3f271fc..54459f2 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -436,8 +436,8 @@ static ssize_t local_pwritev(FsContext *ctx,
> V9fsFidOpenState *fs,
> const struct iovec *iov,
> int iovcnt, off_t offset)
> {
> - ssize_t ret
> -;
> + ssize_t ret;
> +
> #ifdef CONFIG_PREADV
> ret = pwritev(fs->fd, iov, iovcnt, offset);
> #else
> @@ -1240,6 +1240,11 @@ static int local_parse_opts(QemuOpts *opts, struct
> FsDriverEntry *fse)
> error_report("fsdev: No path specified");
> return -1;
> }
> +
> + if (fsdev_throttle_configure_iolimits(opts, &fse->fst) < 0) {
> + return -1;
> + }
> +
> fse->path = g_strdup(path);
>
> return 0;
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index dfe293d..ccf6d0c 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3490,6 +3490,9 @@ int v9fs_device_realize_common(V9fsState *s, Error
> **errp)
> error_setg(errp, "share path %s is not a directory", fse->path);
> goto out;
> }
> +
> + s->ctx.fst = &fse->fst;
> +
> v9fs_path_free(&path);
>
> rc = 0;
> @@ -3504,6 +3507,9 @@ out:
>
> void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
> {
> + if (throttle_enabled(&s->ctx.fst->cfg)) {
> + fsdev_throttle_cleanup(s->ctx.fst);
> + }
> g_free(s->ctx.fs_root);
> g_free(s->tag);
> }
> diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> index 10343c0..5b0ebe0 100644
> --- a/hw/9pfs/cofile.c
> +++ b/hw/9pfs/cofile.c
> @@ -16,6 +16,7 @@
> #include "qemu/thread.h"
> #include "qemu/coroutine.h"
> #include "coth.h"
> +#include "fsdev/qemu-fsdev-throttle.h"
>
> int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode,
> V9fsStatDotl *v9stat)
> @@ -245,6 +246,7 @@ int v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
> if (v9fs_request_cancelled(pdu)) {
> return -EINTR;
> }
> + fsdev_throttle_request(s->ctx.fst, true, iov->iov_len);
> v9fs_co_run_in_worker(
> {
> err = s->ops->pwritev(&s->ctx, &fidp->fs, iov, iovcnt, offset);
> @@ -264,6 +266,7 @@ int v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp,
> if (v9fs_request_cancelled(pdu)) {
> return -EINTR;
> }
> + fsdev_throttle_request(s->ctx.fst, false, iov->iov_len);
> v9fs_co_run_in_worker(
> {
> err = s->ops->preadv(&s->ctx, &fidp->fs, iov, iovcnt, offset);
