All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4] fsdev: add IO throttle support to fsdev devices
@ 2016-09-22 11:59 Pradeep Jagadeesh
  2016-10-07  7:48 ` Greg Kurz
  0 siblings, 1 reply; 5+ messages in thread
From: Pradeep Jagadeesh @ 2016-09-22 11:59 UTC (permalink / raw)
  To: Greg Kurz, Alberto Garcia
  Cc: Pradeep Jagadeesh, Aneesh Kumar K.V, qemu-devel, Claudio Fontana

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.jagadeesh@huawei.com>
---
 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.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 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.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;
+
+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);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v4] fsdev: add IO throttle support to fsdev devices
  2016-09-22 11:59 [Qemu-devel] [PATCH v4] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
@ 2016-10-07  7:48 ` Greg Kurz
  2016-10-10  9:19   ` Pradeep Jagadeesh
  2016-10-20 14:18   ` Pradeep Jagadeesh
  0 siblings, 2 replies; 5+ messages in thread
From: Greg Kurz @ 2016-10-07  7:48 UTC (permalink / raw)
  To: Pradeep Jagadeesh
  Cc: Alberto Garcia, qemu-devel, Claudio Fontana, Pradeep Jagadeesh,
	Aneesh Kumar K.V

On Thu, 22 Sep 2016 07:59:19 -0400
Pradeep Jagadeesh <pradeepkiruvale@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.jagadeesh@huawei.com>
> ---

Hi Pradeep,

So where are we with this patch ? Have you solved the issues you mentioned in
<CAJ2SuLn9Bz4=kzx61KVaG_E=wiD9w_uAfJDhWhb7zis4yUiedQ@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.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 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.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;
> +
> +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);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v4] fsdev: add IO throttle support to fsdev devices
  2016-10-07  7:48 ` Greg Kurz
@ 2016-10-10  9:19   ` Pradeep Jagadeesh
  2016-10-20 14:18   ` Pradeep Jagadeesh
  1 sibling, 0 replies; 5+ messages in thread
From: Pradeep Jagadeesh @ 2016-10-10  9:19 UTC (permalink / raw)
  To: Greg Kurz, Pradeep Jagadeesh
  Cc: Alberto Garcia, qemu-devel, Claudio Fontana, Aneesh Kumar K.V

Hi Greg,

On 10/7/2016 9:48 AM, Greg Kurz wrote:
> On Thu, 22 Sep 2016 07:59:19 -0400
> Pradeep Jagadeesh <pradeepkiruvale@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.jagadeesh@huawei.com>
>> ---
>

> Hi Pradeep,
>
> So where are we with this patch ? Have you solved the issues you mentioned in
> <CAJ2SuLn9Bz4=kzx61KVaG_E=wiD9w_uAfJDhWhb7zis4yUiedQ@mail.gmail.com> ?
>
I was busy with the Linuxcon in Berlin last week. The patch is going 
good.I am trying to find a fix for that issue I found before.I will try 
this week.
> I don't have much time so I'll wait for v5 to comment.
>
Sure
> 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.
>
Ok, I will have a look.

Regards,
Pradeep
> 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.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 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.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;
>> +
>> +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);
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v4] fsdev: add IO throttle support to fsdev devices
  2016-10-07  7:48 ` Greg Kurz
  2016-10-10  9:19   ` Pradeep Jagadeesh
@ 2016-10-20 14:18   ` Pradeep Jagadeesh
  2016-10-20 15:24     ` Greg Kurz
  1 sibling, 1 reply; 5+ messages in thread
From: Pradeep Jagadeesh @ 2016-10-20 14:18 UTC (permalink / raw)
  To: Greg Kurz, Pradeep Jagadeesh
  Cc: Alberto Garcia, qemu-devel, Claudio Fontana, Aneesh Kumar K.V

On 10/7/2016 9:48 AM, Greg Kurz wrote:
> On Thu, 22 Sep 2016 07:59:19 -0400
> Pradeep Jagadeesh <pradeepkiruvale@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.jagadeesh@huawei.com>
>> ---
>
> Hi Pradeep,
>
> So where are we with this patch ? Have you solved the issues you mentioned in
> <CAJ2SuLn9Bz4=kzx61KVaG_E=wiD9w_uAfJDhWhb7zis4yUiedQ@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.
>
As I understand correctly, this is about removing the duplication right?
I did not take care of this comment. I would like to push it as part of 
may be next patch. Is that OK with you?

Regards,
Pradeep

> 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.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 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.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;
>> +
>> +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);
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v4] fsdev: add IO throttle support to fsdev devices
  2016-10-20 14:18   ` Pradeep Jagadeesh
@ 2016-10-20 15:24     ` Greg Kurz
  0 siblings, 0 replies; 5+ messages in thread
From: Greg Kurz @ 2016-10-20 15:24 UTC (permalink / raw)
  To: Pradeep Jagadeesh
  Cc: Pradeep Jagadeesh, Alberto Garcia, qemu-devel, Claudio Fontana,
	Aneesh Kumar K.V

On Thu, 20 Oct 2016 16:18:16 +0200
Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> wrote:

> On 10/7/2016 9:48 AM, Greg Kurz wrote:
> > On Thu, 22 Sep 2016 07:59:19 -0400
> > Pradeep Jagadeesh <pradeepkiruvale@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.jagadeesh@huawei.com>
> >> ---  
> >
> > Hi Pradeep,
> >
> > So where are we with this patch ? Have you solved the issues you mentioned in
> > <CAJ2SuLn9Bz4=kzx61KVaG_E=wiD9w_uAfJDhWhb7zis4yUiedQ@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.
> >  
> As I understand correctly, this is about removing the duplication right?

Correct.

> I did not take care of this comment. I would like to push it as part of 
> may be next patch. Is that OK with you?
> 

Yes.

> Regards,
> Pradeep
> 
> > 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.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 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.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;
> >> +
> >> +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);  
> >  
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-10-20 15:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-22 11:59 [Qemu-devel] [PATCH v4] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2016-10-07  7:48 ` Greg Kurz
2016-10-10  9:19   ` Pradeep Jagadeesh
2016-10-20 14:18   ` Pradeep Jagadeesh
2016-10-20 15:24     ` Greg Kurz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.