(no subject)
diff mbox

Message ID 1477841443-19428-1-git-send-email-pradeep.jagadeesh@huawei.com
State New
Headers show

Commit Message

Pradeep Jagadeesh Oct. 30, 2016, 3:30 p.m. UTC
Date: Sun, 30 Oct 2016 10:53:16 -0400
Subject: [PATCH V8] fsdev: add IO throttle support to fsdev devices

Uses throttling APIs to limit I/O bandwidth and number of operations on the 
devices which use 9p-local driver.

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.

Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
 fsdev/Makefile.objs         |   2 +-
 fsdev/file-op-9p.h          |   3 +
 fsdev/qemu-fsdev-opts.c     |  76 ++++++++++++++++++++++++
 fsdev/qemu-fsdev-throttle.c | 139 ++++++++++++++++++++++++++++++++++++++++++++
 fsdev/qemu-fsdev-throttle.h |  39 +++++++++++++
 hw/9pfs/9p-local.c          |   2 +
 hw/9pfs/9p.c                |  10 ++++
 hw/9pfs/cofile.c            |   2 +
 8 files changed, 272 insertions(+), 1 deletion(-)
 create mode 100644 fsdev/qemu-fsdev-throttle.c
 create mode 100644 fsdev/qemu-fsdev-throttle.h

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

v4 -> V5:
 -Fixed the issue with the larger block size accounting.
 (i.e, when the 9pfs mounted using msize=xxx option)

V5 -> V6:
-Addressed the comments by Alberto Garcia
 -Removed the fsdev_throttle_timer_cb()
 -Simplified the  fsdev_throttle_schedule_next_request() as suggested

V6 -> V7:
-Addressed the comments by Alberto Garcia
 -changed the  fsdev_throttle_schedule_next_request() as suggested

v7 -> v8:
-Addressed comments by Alberto Garcia
 -Fixed some indentation issues and split the configure_io_limit function
 -Inlined throttle_timer_check code

Comments

Greg Kurz Oct. 31, 2016, 12:09 a.m. UTC | #1
Hi Pradeep,

There are still a couple of issues to address with this v8, even if we're not
that far from the final version.

On Sun, 30 Oct 2016 11:30:43 -0400
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:

> Date: Sun, 30 Oct 2016 10:53:16 -0400
> Subject: [PATCH V8] fsdev: add IO throttle support to fsdev devices
> 

It is weird to have the Subject: in the mail body... what happened ?

> Uses throttling APIs to limit I/O bandwidth and number of operations on the 
> devices which use 9p-local driver.
> 
> 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.
> 
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> ---
>  fsdev/Makefile.objs         |   2 +-
>  fsdev/file-op-9p.h          |   3 +
>  fsdev/qemu-fsdev-opts.c     |  76 ++++++++++++++++++++++++
>  fsdev/qemu-fsdev-throttle.c | 139 ++++++++++++++++++++++++++++++++++++++++++++
>  fsdev/qemu-fsdev-throttle.h |  39 +++++++++++++
>  hw/9pfs/9p-local.c          |   2 +
>  hw/9pfs/9p.c                |  10 ++++
>  hw/9pfs/cofile.c            |   2 +
>  8 files changed, 272 insertions(+), 1 deletion(-)
>  create mode 100644 fsdev/qemu-fsdev-throttle.c
>  create mode 100644 fsdev/qemu-fsdev-throttle.h
> 
> 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
> 
> v4 -> V5:
>  -Fixed the issue with the larger block size accounting.
>  (i.e, when the 9pfs mounted using msize=xxx option)
> 
> V5 -> V6:
> -Addressed the comments by Alberto Garcia
>  -Removed the fsdev_throttle_timer_cb()
>  -Simplified the  fsdev_throttle_schedule_next_request() as suggested
> 
> V6 -> V7:
> -Addressed the comments by Alberto Garcia
>  -changed the  fsdev_throttle_schedule_next_request() as suggested
> 
> v7 -> v8:
> -Addressed comments by Alberto Garcia
>  -Fixed some indentation issues and split the configure_io_limit function
>  -Inlined throttle_timer_check code
> 
> 
> diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
> index 1b120a4..659df6e 100644
> --- a/fsdev/Makefile.objs
> +++ b/fsdev/Makefile.objs
> @@ -5,7 +5,7 @@ common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
>  else
>  common-obj-y = qemu-fsdev-dummy.o
>  endif
> -common-obj-y += qemu-fsdev-opts.o
> +common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
>  
>  # Toplevel always builds this; targets without virtio will put it in
>  # common-obj-y
> 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..768d27b
> --- /dev/null
> +++ b/fsdev/qemu-fsdev-throttle.c
> @@ -0,0 +1,139 @@
> +/*
> + * Fsdev Throttle
> + *
> + * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
> + *
> + * Author: Pradeep Jagadeesh <pradeep.jagadeesh@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 void fsdev_throttle_schedule_next_request(FsThrottle *fst, bool is_write)
> +{
> +    if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
> +        if (throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
> +            return;
> +        }
> +       qemu_co_queue_next(&fst->throttled_reqs[is_write]);
> +   }
> +}
> +
> +static void fsdev_throttle_read_timer_cb(void *opaque)
> +{
> +    FsThrottle *fst = opaque;
> +    qemu_co_enter_next(&fst->throttled_reqs[false]);
> +}
> +
> +static void fsdev_throttle_write_timer_cb(void *opaque)
> +{
> +    FsThrottle *fst = opaque;
> +    qemu_co_enter_next(&fst->throttled_reqs[true]);
> +}
> +
> +void fsdev_throttle_parse_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_init(FsThrottle *fst)
> +{
> +    Error *err = NULL;
> +
> +    if (!throttle_is_valid(&fst->cfg, &err)) {
> +        error_reportf_err(err, "Throttle configuration is not valid: ");
> +        return -1;
> +    }

This check belongs to fsdev_throttle_parse_opts() which should be passed an
Error *errp argument. See extract_common_blockdev_options() and how it is
called from blockdev_init() in blockdev.c.

> +    if (throttle_enabled(&fst->cfg)) {
> +        g_assert((fst-> = qemu_get_aio_context()));

qemu_get_aio_context() cannot return NULL since fsdev_throttle_init() gets called
long after qemu_init_main_loop() which initializes the QEMU aio context.

> +        throttle_init(&fst->ts);
> +        throttle_timers_init(&fst->tt,
> +                             fst->aioctx,

fst->aioctx isn't actually needed. You can pass qemu_get_aio_context() here.

> +                             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;
> +}
> +
> +static uint64_t get_num_bytes(struct iovec *iov, int iovcnt)
> +{
> +    int i;
> +    uint64_t bytes = 0;
> +
> +    for (i = 0; i < iovcnt; i++) {
> +        bytes += iov[i].iov_len;
> +    }
> +    return bytes;
> +}
> +

This is iov_size() from include/qemu/iov.h

> +void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
> +                                            struct iovec *iov, int iovcnt)
> +{
> +    if (throttle_enabled(&fst->cfg)) {
> +        uint64_t bytes = get_num_bytes(iov, iovcnt);

The variable isn't needed: this could be passed directly to throttle_account().

> +        bool must_wait = throttle_schedule_timer(&fst->ts, &fst->tt, is_write);

Same here: throttle_schedule_timer() could be called directly in the check below.

> +        if (must_wait || !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
> +            qemu_co_queue_wait(&fst->throttled_reqs[is_write]);
> +        }
> +        throttle_account(&fst->ts, is_write, bytes);
> +
> +        fsdev_throttle_schedule_next_request(fst, is_write);
> +    }
> +}
> +

If you also inline fsdev_throttle_schedule_next_request() then this function
would become:

void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
                                            struct iovec *iov, int iovcnt)
{
    if (throttle_enabled(&fst->cfg)) {
        if (throttle_schedule_timer(&fst->ts, &fst->tt, is_write) ||
            !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
            qemu_co_queue_wait(&fst->throttled_reqs[is_write]);
        }

        throttle_account(&fst->ts, is_write, iov_size(iov, iovcnt));

        if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) &&
            !throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
            qemu_co_queue_next(&fst->throttled_reqs[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..bafb340
> --- /dev/null
> +++ b/fsdev/qemu-fsdev-throttle.h
> @@ -0,0 +1,39 @@
> +/*
> + * Fsdev Throttle
> + *
> + * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
> + *
> + * Author: Pradeep Jagadeesh <pradeep.jagadeesh@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;
> +
> +void fsdev_throttle_parse_opts(QemuOpts *, FsThrottle *);
> +
> +int  fsdev_throttle_init(FsThrottle *);
> +
> +void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool ,
> +                                            struct iovec *, int);
> +
> +void fsdev_throttle_cleanup(FsThrottle *);
> +#endif /* _FSDEV_THROTTLE_H */
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 845675e..842b82e 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -1237,6 +1237,8 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
>          error_report("fsdev: No path specified");
>          return -1;
>      }
> +
> +    fsdev_throttle_parse_opts(opts, &fse->fst);

As Berto suggested in another mail, the error message should be printed here:

fsdev_throttle_parse_opts(opts, &fse->fst, &err);
if (err) {
    error_reportf_err(err, "Throttle configuration is not valid: ");
}

>      fse->path = g_strdup(path);
>  
>      return 0;
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index e88cf25..450ed12 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3506,6 +3506,13 @@ 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;
> +    if (fsdev_throttle_init(s->ctx.fst) < 0) {
> +        error_report("fsdev: Throttle configuration specified is not valid");
> +        return -1;
> +    }
> +

And fsdev_throttle_init() can no longer fail.

>      v9fs_path_free(&path);
>  
>      rc = 0;
> @@ -3520,6 +3527,9 @@ out:
>  
>  void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
>  {
> +    if (throttle_enabled(&s->ctx.fst->cfg)) {
> +        fsdev_throttle_cleanup(s->ctx.fst);
> +    }

For consistency with v9fs_device_realize_common(), please move the
throttle_enabled() check to fsdev_throttle_cleanup().

>      g_free(s->ctx.fs_root);
>      g_free(s->tag);
>  }
> diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> index 120e267..88791bc 100644
> --- a/hw/9pfs/cofile.c
> +++ b/hw/9pfs/cofile.c
> @@ -247,6 +247,7 @@ int coroutine_fn v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
>      if (v9fs_request_cancelled(pdu)) {
>          return -EINTR;
>      }
> +    fsdev_co_throttle_request(s->ctx.fst, true, iov, iovcnt);
>      v9fs_co_run_in_worker(
>          {
>              err = s->ops->pwritev(&s->ctx, &fidp->fs, iov, iovcnt, offset);
> @@ -266,6 +267,7 @@ int coroutine_fn v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp,
>      if (v9fs_request_cancelled(pdu)) {
>          return -EINTR;
>      }
> +    fsdev_co_throttle_request(s->ctx.fst, false, iov, iovcnt);
>      v9fs_co_run_in_worker(
>          {
>              err = s->ops->preadv(&s->ctx, &fidp->fs, iov, iovcnt, offset);
Alberto Garcia Oct. 31, 2016, 1:36 p.m. UTC | #2
On Mon 31 Oct 2016 01:09:42 AM CET, Greg Kurz wrote:

> There are still a couple of issues to address with this v8, even if
> we're not that far from the final version.

I agree with Greg's comments. I'll wait for the next version.

Berto
Pradeep Jagadeesh Nov. 7, 2016, 1:51 p.m. UTC | #3
Hi Greg,

Sorry for the late reply, was out of office.
I agree with all comments and fix them all.
Also will send the patch soon.

-Pradeep

> Hi Pradeep,
>
> There are still a couple of issues to address with this v8, even if we're not
> that far from the final version.
>
> On Sun, 30 Oct 2016 11:30:43 -0400
> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
>
>> Date: Sun, 30 Oct 2016 10:53:16 -0400
>> Subject: [PATCH V8] fsdev: add IO throttle support to fsdev devices
>>
>
> It is weird to have the Subject: in the mail body... what happened ?
No idea what happened.
>
>> Uses throttling APIs to limit I/O bandwidth and number of operations on the
>> devices which use 9p-local driver.
>>
>> 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.
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>> ---
>>  fsdev/Makefile.objs         |   2 +-
>>  fsdev/file-op-9p.h          |   3 +
>>  fsdev/qemu-fsdev-opts.c     |  76 ++++++++++++++++++++++++
>>  fsdev/qemu-fsdev-throttle.c | 139 ++++++++++++++++++++++++++++++++++++++++++++
>>  fsdev/qemu-fsdev-throttle.h |  39 +++++++++++++
>>  hw/9pfs/9p-local.c          |   2 +
>>  hw/9pfs/9p.c                |  10 ++++
>>  hw/9pfs/cofile.c            |   2 +
>>  8 files changed, 272 insertions(+), 1 deletion(-)
>>  create mode 100644 fsdev/qemu-fsdev-throttle.c
>>  create mode 100644 fsdev/qemu-fsdev-throttle.h
>>
>> 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
>>
>> v4 -> V5:
>>  -Fixed the issue with the larger block size accounting.
>>  (i.e, when the 9pfs mounted using msize=xxx option)
>>
>> V5 -> V6:
>> -Addressed the comments by Alberto Garcia
>>  -Removed the fsdev_throttle_timer_cb()
>>  -Simplified the  fsdev_throttle_schedule_next_request() as suggested
>>
>> V6 -> V7:
>> -Addressed the comments by Alberto Garcia
>>  -changed the  fsdev_throttle_schedule_next_request() as suggested
>>
>> v7 -> v8:
>> -Addressed comments by Alberto Garcia
>>  -Fixed some indentation issues and split the configure_io_limit function
>>  -Inlined throttle_timer_check code
>>
>>
>> diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
>> index 1b120a4..659df6e 100644
>> --- a/fsdev/Makefile.objs
>> +++ b/fsdev/Makefile.objs
>> @@ -5,7 +5,7 @@ common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
>>  else
>>  common-obj-y = qemu-fsdev-dummy.o
>>  endif
>> -common-obj-y += qemu-fsdev-opts.o
>> +common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
>>
>>  # Toplevel always builds this; targets without virtio will put it in
>>  # common-obj-y
>> 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..768d27b
>> --- /dev/null
>> +++ b/fsdev/qemu-fsdev-throttle.c
>> @@ -0,0 +1,139 @@
>> +/*
>> + * Fsdev Throttle
>> + *
>> + * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
>> + *
>> + * Author: Pradeep Jagadeesh <pradeep.jagadeesh@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 void fsdev_throttle_schedule_next_request(FsThrottle *fst, bool is_write)
>> +{
>> +    if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
>> +        if (throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
>> +            return;
>> +        }
>> +       qemu_co_queue_next(&fst->throttled_reqs[is_write]);
>> +   }
>> +}
>> +
>> +static void fsdev_throttle_read_timer_cb(void *opaque)
>> +{
>> +    FsThrottle *fst = opaque;
>> +    qemu_co_enter_next(&fst->throttled_reqs[false]);
>> +}
>> +
>> +static void fsdev_throttle_write_timer_cb(void *opaque)
>> +{
>> +    FsThrottle *fst = opaque;
>> +    qemu_co_enter_next(&fst->throttled_reqs[true]);
>> +}
>> +
>> +void fsdev_throttle_parse_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_init(FsThrottle *fst)
>> +{
>> +    Error *err = NULL;
>> +
>> +    if (!throttle_is_valid(&fst->cfg, &err)) {
>> +        error_reportf_err(err, "Throttle configuration is not valid: ");
>> +        return -1;
>> +    }
>
> This check belongs to fsdev_throttle_parse_opts() which should be passed an
> Error *errp argument. See extract_common_blockdev_options() and how it is
> called from blockdev_init() in blockdev.c.
Done
>
>> +    if (throttle_enabled(&fst->cfg)) {
>> +        g_assert((fst-> = qemu_get_aio_context()));
>
> qemu_get_aio_context() cannot return NULL since fsdev_throttle_init() gets called
> long after qemu_init_main_loop() which initializes the QEMU aio context.
OK
>
>> +        throttle_init(&fst->ts);
>> +        throttle_timers_init(&fst->tt,
>> +                             fst->aioctx,
>
> fst->aioctx isn't actually needed. You can pass qemu_get_aio_context() here.
OK
>
>> +                             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;
>> +}
>> +
>> +static uint64_t get_num_bytes(struct iovec *iov, int iovcnt)
>> +{
>> +    int i;
>> +    uint64_t bytes = 0;
>> +
>> +    for (i = 0; i < iovcnt; i++) {
>> +        bytes += iov[i].iov_len;
>> +    }
>> +    return bytes;
>> +}
>> +
>
> This is iov_size() from include/qemu/iov.h
Nice!.Done.

>
>> +void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
>> +                                            struct iovec *iov, int iovcnt)
>> +{
>> +    if (throttle_enabled(&fst->cfg)) {
>> +        uint64_t bytes = get_num_bytes(iov, iovcnt);
>
> The variable isn't needed: this could be passed directly to throttle_account().
OK
>
>> +        bool must_wait = throttle_schedule_timer(&fst->ts, &fst->tt, is_write);
>
> Same here: throttle_schedule_timer() could be called directly in the check below.
OK
>
>> +        if (must_wait || !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
>> +            qemu_co_queue_wait(&fst->throttled_reqs[is_write]);
>> +        }
>> +        throttle_account(&fst->ts, is_write, bytes);
>> +
>> +        fsdev_throttle_schedule_next_request(fst, is_write);
>> +    }
>> +}
>> +
>
> If you also inline fsdev_throttle_schedule_next_request() then this function
> would become:
>
> void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
>                                             struct iovec *iov, int iovcnt)
> {
>     if (throttle_enabled(&fst->cfg)) {
>         if (throttle_schedule_timer(&fst->ts, &fst->tt, is_write) ||
>             !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
>             qemu_co_queue_wait(&fst->throttled_reqs[is_write]);
>         }
>
>         throttle_account(&fst->ts, is_write, iov_size(iov, iovcnt));
>
>         if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) &&
>             !throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
>             qemu_co_queue_next(&fst->throttled_reqs[is_write]);
>         }
>     }
> }
Done
>
>> +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..bafb340
>> --- /dev/null
>> +++ b/fsdev/qemu-fsdev-throttle.h
>> @@ -0,0 +1,39 @@
>> +/*
>> + * Fsdev Throttle
>> + *
>> + * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
>> + *
>> + * Author: Pradeep Jagadeesh <pradeep.jagadeesh@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;
>> +
>> +void fsdev_throttle_parse_opts(QemuOpts *, FsThrottle *);
>> +
>> +int  fsdev_throttle_init(FsThrottle *);
>> +
>> +void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool ,
>> +                                            struct iovec *, int);
>> +
>> +void fsdev_throttle_cleanup(FsThrottle *);
>> +#endif /* _FSDEV_THROTTLE_H */
>> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
>> index 845675e..842b82e 100644
>> --- a/hw/9pfs/9p-local.c
>> +++ b/hw/9pfs/9p-local.c
>> @@ -1237,6 +1237,8 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
>>          error_report("fsdev: No path specified");
>>          return -1;
>>      }
>> +
>> +    fsdev_throttle_parse_opts(opts, &fse->fst);
>
> As Berto suggested in another mail, the error message should be printed here:
>
> fsdev_throttle_parse_opts(opts, &fse->fst, &err);
> if (err) {
>     error_reportf_err(err, "Throttle configuration is not valid: ");
> }
OK
>
>>      fse->path = g_strdup(path);
>>
>>      return 0;
>> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
>> index e88cf25..450ed12 100644
>> --- a/hw/9pfs/9p.c
>> +++ b/hw/9pfs/9p.c
>> @@ -3506,6 +3506,13 @@ 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;
>> +    if (fsdev_throttle_init(s->ctx.fst) < 0) {
>> +        error_report("fsdev: Throttle configuration specified is not valid");
>> +        return -1;
>> +    }
>> +
>
> And fsdev_throttle_init() can no longer fail.
ok
>
>>      v9fs_path_free(&path);
>>
>>      rc = 0;
>> @@ -3520,6 +3527,9 @@ out:
>>
>>  void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
>>  {
>> +    if (throttle_enabled(&s->ctx.fst->cfg)) {
>> +        fsdev_throttle_cleanup(s->ctx.fst);
>> +    }
>
> For consistency with v9fs_device_realize_common(), please move the
> throttle_enabled() check to fsdev_throttle_cleanup().
OK
>
>>      g_free(s->ctx.fs_root);
>>      g_free(s->tag);
>>  }
>> diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
>> index 120e267..88791bc 100644
>> --- a/hw/9pfs/cofile.c
>> +++ b/hw/9pfs/cofile.c
>> @@ -247,6 +247,7 @@ int coroutine_fn v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
>>      if (v9fs_request_cancelled(pdu)) {
>>          return -EINTR;
>>      }
>> +    fsdev_co_throttle_request(s->ctx.fst, true, iov, iovcnt);
>>      v9fs_co_run_in_worker(
>>          {
>>              err = s->ops->pwritev(&s->ctx, &fidp->fs, iov, iovcnt, offset);
>> @@ -266,6 +267,7 @@ int coroutine_fn v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp,
>>      if (v9fs_request_cancelled(pdu)) {
>>          return -EINTR;
>>      }
>> +    fsdev_co_throttle_request(s->ctx.fst, false, iov, iovcnt);
>>      v9fs_co_run_in_worker(
>>          {
>>              err = s->ops->preadv(&s->ctx, &fidp->fs, iov, iovcnt, offset);
>

Patch
diff mbox

diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
index 1b120a4..659df6e 100644
--- a/fsdev/Makefile.objs
+++ b/fsdev/Makefile.objs
@@ -5,7 +5,7 @@  common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
 else
 common-obj-y = qemu-fsdev-dummy.o
 endif
-common-obj-y += qemu-fsdev-opts.o
+common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
 
 # Toplevel always builds this; targets without virtio will put it in
 # common-obj-y
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..768d27b
--- /dev/null
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -0,0 +1,139 @@ 
+/*
+ * Fsdev Throttle
+ *
+ * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Pradeep Jagadeesh <pradeep.jagadeesh@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 void fsdev_throttle_schedule_next_request(FsThrottle *fst, bool is_write)
+{
+    if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
+        if (throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
+            return;
+        }
+       qemu_co_queue_next(&fst->throttled_reqs[is_write]);
+   }
+}
+
+static void fsdev_throttle_read_timer_cb(void *opaque)
+{
+    FsThrottle *fst = opaque;
+    qemu_co_enter_next(&fst->throttled_reqs[false]);
+}
+
+static void fsdev_throttle_write_timer_cb(void *opaque)
+{
+    FsThrottle *fst = opaque;
+    qemu_co_enter_next(&fst->throttled_reqs[true]);
+}
+
+void fsdev_throttle_parse_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_init(FsThrottle *fst)
+{
+    Error *err = NULL;
+
+    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;
+}
+
+static uint64_t get_num_bytes(struct iovec *iov, int iovcnt)
+{
+    int i;
+    uint64_t bytes = 0;
+
+    for (i = 0; i < iovcnt; i++) {
+        bytes += iov[i].iov_len;
+    }
+    return bytes;
+}
+
+void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
+                                            struct iovec *iov, int iovcnt)
+{
+    if (throttle_enabled(&fst->cfg)) {
+        uint64_t bytes = get_num_bytes(iov, iovcnt);
+        bool must_wait = throttle_schedule_timer(&fst->ts, &fst->tt, is_write);
+        if (must_wait || !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
+            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..bafb340
--- /dev/null
+++ b/fsdev/qemu-fsdev-throttle.h
@@ -0,0 +1,39 @@ 
+/*
+ * Fsdev Throttle
+ *
+ * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Pradeep Jagadeesh <pradeep.jagadeesh@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;
+
+void fsdev_throttle_parse_opts(QemuOpts *, FsThrottle *);
+
+int  fsdev_throttle_init(FsThrottle *);
+
+void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool ,
+                                            struct iovec *, int);
+
+void fsdev_throttle_cleanup(FsThrottle *);
+#endif /* _FSDEV_THROTTLE_H */
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 845675e..842b82e 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1237,6 +1237,8 @@  static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
         error_report("fsdev: No path specified");
         return -1;
     }
+
+    fsdev_throttle_parse_opts(opts, &fse->fst);
     fse->path = g_strdup(path);
 
     return 0;
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index e88cf25..450ed12 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3506,6 +3506,13 @@  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;
+    if (fsdev_throttle_init(s->ctx.fst) < 0) {
+        error_report("fsdev: Throttle configuration specified is not valid");
+        return -1;
+    }
+
     v9fs_path_free(&path);
 
     rc = 0;
@@ -3520,6 +3527,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 120e267..88791bc 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -247,6 +247,7 @@  int coroutine_fn v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
     if (v9fs_request_cancelled(pdu)) {
         return -EINTR;
     }
+    fsdev_co_throttle_request(s->ctx.fst, true, iov, iovcnt);
     v9fs_co_run_in_worker(
         {
             err = s->ops->pwritev(&s->ctx, &fidp->fs, iov, iovcnt, offset);
@@ -266,6 +267,7 @@  int coroutine_fn v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp,
     if (v9fs_request_cancelled(pdu)) {
         return -EINTR;
     }
+    fsdev_co_throttle_request(s->ctx.fst, false, iov, iovcnt);
     v9fs_co_run_in_worker(
         {
             err = s->ops->preadv(&s->ctx, &fidp->fs, iov, iovcnt, offset);