diff mbox

9pfs: add support for IO limits to 9p-local driver

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

Commit Message

Pradeep Jagadeesh Sept. 7, 2016, 12:39 p.m. UTC 
Uses throttling APIs to limit I/O bandwidth and number of operations on the 
devices which use 9p-local driver.

Signed-off-by: Pradeep <pradeep.jagadeesh@huawei.com>
 fsdev/file-op-9p.h      |   3 +
 fsdev/qemu-fsdev-opts.c |  52 +++++++++++++
 hw/9pfs/9p-local.c      |  18 ++++-
 hw/9pfs/9p-throttle.c   | 199 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/9p-throttle.h   |  55 +++++++++++++
 hw/9pfs/9p.c            |   7 ++
 hw/9pfs/Makefile.objs   |   1 +
 7 files changed, 333 insertions(+), 2 deletions(-)
 create mode 100644 hw/9pfs/9p-throttle.c
 create mode 100644 hw/9pfs/9p-throttle.h

Hi All,

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.

Cheers,
Pradeep

Comments

no-reply@patchew.org Sept. 7, 2016, 12:44 p.m. UTC | #1 
Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Subject: [Qemu-devel] [PATCH] 9pfs: add support for IO limits to 9p-local driver
Type: series
Message-id: 1473251980-39805-1-git-send-email-pradeep.jagadeesh@huawei.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1473251980-39805-1-git-send-email-pradeep.jagadeesh@huawei.com -> patchew/1473251980-39805-1-git-send-email-pradeep.jagadeesh@huawei.com
Switched to a new branch 'test'
87e77af 9pfs: add support for IO limits to 9p-local driver

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY RUNNER
  RUN test-quick in centos6
No C++ compiler available; disabling C++ specific optional code
Install prefix    /tmp/qemu-test/src/tests/docker/install
BIOS directory    /tmp/qemu-test/src/tests/docker/install/share/qemu
binary directory  /tmp/qemu-test/src/tests/docker/install/bin
library directory /tmp/qemu-test/src/tests/docker/install/lib
module directory  /tmp/qemu-test/src/tests/docker/install/lib/qemu
libexec directory /tmp/qemu-test/src/tests/docker/install/libexec
include directory /tmp/qemu-test/src/tests/docker/install/include
config directory  /tmp/qemu-test/src/tests/docker/install/etc
local state directory   /tmp/qemu-test/src/tests/docker/install/var
Manual directory  /tmp/qemu-test/src/tests/docker/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path       /tmp/qemu-test/src
C compiler        cc
Host C compiler   cc
C++ compiler      
Objective-C compiler cc
ARFLAGS           rv
CFLAGS            -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -g 
QEMU_CFLAGS       -I/usr/include/pixman-1    -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common  -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all
LDFLAGS           -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make              make
install           install
python            python -B
smbd              /usr/sbin/smbd
module support    no
host CPU          x86_64
host big endian   no
target list       x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled     no
sparse enabled    no
strip binaries    yes
profiler          no
static build      no
pixman            system
SDL support       yes (1.2.14)
GTK support       no 
GTK GL support    no
VTE support       no 
TLS priority      NORMAL
GNUTLS support    no
GNUTLS rnd        no
libgcrypt         no
libgcrypt kdf     no
nettle            no 
nettle kdf        no
libtasn1          no
curses support    no
virgl support     no
curl support      no
mingw32 support   no
Audio drivers     oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS support    no
VNC support       yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support       no
brlapi support    no
bluez  support    no
Documentation     no
PIE               yes
vde support       no
netmap support    no
Linux AIO support no
ATTR/XATTR support yes
Install blobs     yes
KVM support       yes
RDMA support      no
TCG interpreter   no
fdt support       yes
preadv support    yes
fdatasync         yes
madvise           yes
posix_madvise     yes
uuid support      no
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
Trace backends    log
spice support     no 
rbd support       no
xfsctl support    no
smartcard support no
libusb            no
usb net redir     no
OpenGL support    no
OpenGL dmabufs    no
libiscsi support  no
libnfs support    no
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine pool    yes
GlusterFS support no
Archipelago support no
gcov              gcov
gcov enabled      no
TPM support       yes
libssh2 support   no
TPM passthrough   yes
QOM debugging     yes
vhdx              no
lzo support       no
snappy support    no
bzip2 support     no
NUMA host support no
tcmalloc support  no
jemalloc support  no
avx2 optimization no
  GEN   x86_64-softmmu/config-devices.mak.tmp
  GEN   aarch64-softmmu/config-devices.mak.tmp
  GEN   config-host.h
  GEN   qemu-options.def
  GEN   qapi-types.h
  GEN   qmp-commands.h
  GEN   qapi-visit.h
  GEN   qapi-event.h
  GEN   x86_64-softmmu/config-devices.mak
  GEN   aarch64-softmmu/config-devices.mak
  GEN   qmp-introspect.h
  GEN   tests/test-qapi-types.h
  GEN   tests/test-qapi-visit.h
  GEN   tests/test-qmp-commands.h
  GEN   tests/test-qapi-event.h
  GEN   tests/test-qmp-introspect.h
  GEN   config-all-devices.mak
  GEN   trace/generated-events.h
  GEN   trace/generated-tracers.h
  GEN   trace/generated-tcg-tracers.h
  GEN   trace/generated-helpers-wrappers.h
  GEN   trace/generated-helpers.h
  CC    tests/qemu-iotests/socket_scm_helper.o
  GEN   qga/qapi-generated/qga-qapi-types.h
  GEN   qga/qapi-generated/qga-qapi-visit.h
  GEN   qga/qapi-generated/qga-qmp-commands.h
  GEN   qga/qapi-generated/qga-qapi-types.c
  GEN   qga/qapi-generated/qga-qapi-visit.c
  GEN   qga/qapi-generated/qga-qmp-marshal.c
  GEN   qmp-introspect.c
  GEN   qapi-types.c
  GEN   qapi-event.c
  GEN   qapi-visit.c
  CC    qapi/qapi-visit-core.o
  CC    qapi/qapi-dealloc-visitor.o
  CC    qapi/qmp-input-visitor.o
  CC    qapi/qmp-output-visitor.o
  CC    qapi/qmp-registry.o
  CC    qapi/qmp-dispatch.o
  CC    qapi/string-input-visitor.o
  CC    qapi/string-output-visitor.o
  CC    qapi/opts-visitor.o
  CC    qapi/qapi-clone-visitor.o
  CC    qapi/qmp-event.o
  CC    qapi/qapi-util.o
  CC    qobject/qnull.o
  CC    qobject/qint.o
  CC    qobject/qstring.o
  CC    qobject/qdict.o
  CC    qobject/qlist.o
  CC    qobject/qfloat.o
  CC    qobject/qbool.o
  CC    qobject/qobject.o
  CC    qobject/json-lexer.o
  CC    qobject/qjson.o
  CC    qobject/json-streamer.o
  CC    qobject/json-parser.o
  GEN   trace/generated-events.c
  CC    trace/control.o
  CC    trace/qmp.o
  CC    util/osdep.o
  CC    util/cutils.o
  CC    util/unicode.o
  CC    util/qemu-timer-common.o
  CC    util/compatfd.o
  CC    util/event_notifier-posix.o
  CC    util/mmap-alloc.o
  CC    util/oslib-posix.o
  CC    util/qemu-openpty.o
  CC    util/qemu-thread-posix.o
  CC    util/memfd.o
  CC    util/envlist.o
  CC    util/path.o
  CC    util/module.o
  CC    util/bitmap.o
  CC    util/bitops.o
  CC    util/hbitmap.o
  CC    util/fifo8.o
  CC    util/acl.o
  CC    util/error.o
  CC    util/qemu-error.o
  CC    util/id.o
  CC    util/iov.o
  CC    util/qemu-config.o
  CC    util/qemu-sockets.o
  CC    util/uri.o
  CC    util/notify.o
  CC    util/qemu-option.o
  CC    util/qemu-progress.o
  CC    util/hexdump.o
  CC    util/crc32c.o
  CC    util/throttle.o
  CC    util/getauxval.o
  CC    util/readline.o
  CC    util/rfifolock.o
  CC    util/rcu.o
  CC    util/qemu-coroutine.o
  CC    util/qemu-coroutine-lock.o
  CC    util/qemu-coroutine-io.o
  CC    util/qemu-coroutine-sleep.o
  CC    util/coroutine-ucontext.o
  CC    util/buffer.o
  CC    util/timed-average.o
  CC    util/base64.o
  CC    util/log.o
  CC    util/qdist.o
  CC    util/qht.o
  CC    util/range.o
  CC    crypto/pbkdf-stub.o
  CC    stubs/arch-query-cpu-def.o
/tmp/qemu-test/src/util/qht.c: In function ‘qht_reset_size’:
/tmp/qemu-test/src/util/qht.c:413: warning: ‘new’ may be used uninitialized in this function
  CC    stubs/arch-query-cpu-model-expansion.o
  CC    stubs/arch-query-cpu-model-comparison.o
  CC    stubs/arch-query-cpu-model-baseline.o
  CC    stubs/bdrv-next-monitor-owned.o
  CC    stubs/blk-commit-all.o
  CC    stubs/blockdev-close-all-bdrv-states.o
  CC    stubs/clock-warp.o
  CC    stubs/cpu-get-clock.o
  CC    stubs/cpu-get-icount.o
  CC    stubs/dump.o
  CC    stubs/fdset-add-fd.o
  CC    stubs/fdset-find-fd.o
  CC    stubs/fdset-get-fd.o
  CC    stubs/fdset-remove-fd.o
  CC    stubs/gdbstub.o
  CC    stubs/get-fd.o
  CC    stubs/get-next-serial.o
  CC    stubs/get-vm-name.o
  CC    stubs/iothread-lock.o
  CC    stubs/is-daemonized.o
  CC    stubs/machine-init-done.o
  CC    stubs/migr-blocker.o
  CC    stubs/mon-is-qmp.o
  CC    stubs/mon-printf.o
  CC    stubs/monitor-init.o
  CC    stubs/notify-event.o
  CC    stubs/replay.o
  CC    stubs/qtest.o
  CC    stubs/replay-user.o
  CC    stubs/reset.o
  CC    stubs/runstate-check.o
  CC    stubs/set-fd-handler.o
  CC    stubs/slirp.o
  CC    stubs/sysbus.o
  CC    stubs/trace-control.o
  CC    stubs/uuid.o
  CC    stubs/vm-stop.o
  CC    stubs/vmstate.o
  CC    stubs/cpus.o
  CC    stubs/kvm.o
  CC    stubs/qmp_pc_dimm_device_list.o
  CC    stubs/target-monitor-defs.o
  CC    stubs/target-get-monitor-def.o
  CC    stubs/vhost.o
  CC    stubs/iohandler.o
  CC    stubs/smbios_type_38.o
  CC    stubs/ipmi.o
  CC    contrib/ivshmem-client/ivshmem-client.o
  CC    stubs/pc_madt_cpu_entry.o
  CC    contrib/ivshmem-client/main.o
  CC    contrib/ivshmem-server/ivshmem-server.o
  CC    contrib/ivshmem-server/main.o
  CC    async.o
  CC    qemu-nbd.o
  CC    thread-pool.o
  CC    block.o
  CC    blockjob.o
  CC    main-loop.o
  CC    iohandler.o
  CC    qemu-timer.o
  CC    aio-posix.o
  CC    qemu-io-cmds.o
  CC    block/raw_bsd.o
  CC    block/qcow.o
  CC    block/vdi.o
  CC    block/vmdk.o
  CC    block/cloop.o
  CC    block/bochs.o
  CC    block/vpc.o
  CC    block/vvfat.o
  CC    block/qcow2.o
  CC    block/qcow2-refcount.o
  CC    block/qcow2-cluster.o
  CC    block/qcow2-snapshot.o
  CC    block/qcow2-cache.o
  CC    block/qed.o
  CC    block/qed-gencb.o
  CC    block/qed-l2-cache.o
  CC    block/qed-table.o
  CC    block/qed-cluster.o
  CC    block/qed-check.o
  CC    block/quorum.o
  CC    block/parallels.o
  CC    block/blkdebug.o
  CC    block/blkverify.o
  CC    block/blkreplay.o
  CC    block/block-backend.o
  CC    block/snapshot.o
  CC    block/qapi.o
  CC    block/raw-posix.o
  CC    block/null.o
  CC    block/mirror.o
  CC    block/commit.o
  CC    block/io.o
  CC    block/throttle-groups.o
  CC    block/nbd.o
  CC    block/nbd-client.o
  CC    block/sheepdog.o
  CC    block/accounting.o
  CC    block/dirty-bitmap.o
  CC    block/write-threshold.o
  CC    block/crypto.o
  CC    nbd/server.o
  CC    nbd/client.o
  CC    nbd/common.o
  CC    block/dmg.o
  CC    crypto/init.o
  CC    crypto/hash.o
  CC    crypto/hash-glib.o
  CC    crypto/aes.o
  CC    crypto/desrfb.o
  CC    crypto/cipher.o
  CC    crypto/tlscreds.o
  CC    crypto/tlscredsanon.o
  CC    crypto/tlscredsx509.o
  CC    crypto/tlssession.o
  CC    crypto/secret.o
  CC    crypto/random-platform.o
  CC    crypto/pbkdf.o
  CC    crypto/ivgen.o
  CC    crypto/ivgen-essiv.o
  CC    crypto/ivgen-plain.o
  CC    crypto/ivgen-plain64.o
  CC    crypto/afsplit.o
  CC    crypto/xts.o
  CC    crypto/block.o
  CC    crypto/block-qcow.o
  CC    crypto/block-luks.o
  CC    io/channel.o
  CC    io/channel-buffer.o
  CC    io/channel-command.o
  CC    io/channel-file.o
  CC    io/channel-socket.o
  CC    io/channel-tls.o
  CC    io/channel-watch.o
  CC    io/channel-websock.o
  CC    io/channel-util.o
  CC    io/task.o
  CC    qom/object.o
  CC    qom/container.o
  CC    qom/qom-qobject.o
  CC    qom/object_interfaces.o
  GEN   qemu-img-cmds.h
  CC    qemu-io.o
  CC    qemu-bridge-helper.o
  CC    blockdev.o
  CC    blockdev-nbd.o
  CC    iothread.o
  CC    qdev-monitor.o
  CC    device-hotplug.o
  CC    os-posix.o
  CC    qemu-char.o
  CC    page_cache.o
  CC    accel.o
  CC    bt-host.o
  CC    bt-vhci.o
  CC    dma-helpers.o
  CC    vl.o
  CC    tpm.o
  CC    device_tree.o
  GEN   qmp-marshal.c
  CC    qmp.o
  CC    hmp.o
  CC    tcg-runtime.o
  CC    audio/audio.o
  CC    audio/noaudio.o
  CC    audio/wavaudio.o
  CC    audio/mixeng.o
  CC    audio/sdlaudio.o
  CC    audio/ossaudio.o
  CC    audio/wavcapture.o
  CC    backends/rng.o
  CC    backends/rng-egd.o
  CC    backends/rng-random.o
  CC    backends/msmouse.o
  CC    backends/testdev.o
  CC    backends/tpm.o
  CC    backends/hostmem.o
  CC    backends/hostmem-ram.o
  CC    backends/hostmem-file.o
  CC    block/stream.o
  CC    block/backup.o
  CC    disas/arm.o
  CC    disas/i386.o
  CC    fsdev/qemu-fsdev-dummy.o
  CC    fsdev/qemu-fsdev-opts.o
  CC    hw/acpi/core.o
  CC    hw/acpi/piix4.o
  CC    hw/acpi/pcihp.o
  CC    hw/acpi/ich9.o
  CC    hw/acpi/tco.o
  CC    hw/acpi/cpu_hotplug.o
  CC    hw/acpi/memory_hotplug.o
  CC    hw/acpi/memory_hotplug_acpi_table.o
  CC    hw/acpi/cpu.o
  CC    hw/acpi/acpi_interface.o
  CC    hw/acpi/bios-linker-loader.o
  CC    hw/acpi/aml-build.o
  CC    hw/acpi/ipmi.o
  CC    hw/audio/sb16.o
  CC    hw/audio/es1370.o
  CC    hw/audio/ac97.o
  CC    hw/audio/fmopl.o
  CC    hw/audio/adlib.o
  CC    hw/audio/gus.o
  CC    hw/audio/gusemu_hal.o
  CC    hw/audio/gusemu_mixer.o
  CC    hw/audio/cs4231a.o
  CC    hw/audio/intel-hda.o
  CC    hw/audio/hda-codec.o
  CC    hw/audio/pcspk.o
  CC    hw/audio/wm8750.o
  CC    hw/audio/pl041.o
  CC    hw/audio/lm4549.o
  CC    hw/audio/marvell_88w8618.o
  CC    hw/block/block.o
  CC    hw/block/cdrom.o
  CC    hw/block/hd-geometry.o
  CC    hw/block/fdc.o
  CC    hw/block/m25p80.o
  CC    hw/block/nand.o
  CC    hw/block/pflash_cfi01.o
  CC    hw/block/pflash_cfi02.o
  CC    hw/block/ecc.o
  CC    hw/block/onenand.o
  CC    hw/block/nvme.o
  CC    hw/bt/core.o
In file included from /tmp/qemu-test/src/fsdev/qemu-fsdev.h:17,
                 from /tmp/qemu-test/src/fsdev/qemu-fsdev-dummy.c:15:
/tmp/qemu-test/src/fsdev/file-op-9p.h:91: error: redefinition of typedef ‘FsContext’
/tmp/qemu-test/src/hw/9pfs/9p-throttle.h:38: note: previous declaration of ‘FsContext’ was here
  CC    hw/bt/l2cap.o
  CC    hw/bt/sdp.o
make: *** [fsdev/qemu-fsdev-dummy.o] Error 1
make: *** Waiting for unfinished jobs....
tests/docker/Makefile.include:104: recipe for target 'docker-run-test-quick@centos6' failed
make: *** [docker-run-test-quick@centos6] Error 1
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Eric Blake Sept. 7, 2016, 9:36 p.m. UTC | #2 
On 09/07/2016 07:39 AM, Pradeep 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 <pradeep.jagadeesh@huawei.com>
>  fsdev/file-op-9p.h      |   3 +
>  fsdev/qemu-fsdev-opts.c |  52 +++++++++++++
>  hw/9pfs/9p-local.c      |  18 ++++-
>  hw/9pfs/9p-throttle.c   | 199 ++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/9pfs/9p-throttle.h   |  55 +++++++++++++
>  hw/9pfs/9p.c            |   7 ++
>  hw/9pfs/Makefile.objs   |   1 +
>  7 files changed, 333 insertions(+), 2 deletions(-)
>  create mode 100644 hw/9pfs/9p-throttle.c
>  create mode 100644 hw/9pfs/9p-throttle.h
> 
> Hi All,
> 
> 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.

CLI-only extensions mean that we can't use it during hotplug.  Are you
sure that blockdev-add won't automatically enable these options, given
that it tries to convert the dictionary passed through QMP back into the
QemuOpts form that matches command line parsing?  Also, aren't these
limits very similar to what is already available in the throttle driver?
Why are we repeating them at the 9p driver, instead of putting a
throttle group BDS in the mix?

> +++ b/fsdev/qemu-fsdev-opts.c
> @@ -37,6 +37,58 @@ static QemuOptsList qemu_fsdev_opts = {
>          }, {
>              .name = "sock_fd",
>              .type = QEMU_OPT_NUMBER,
> +        }, {
> +            .name = "bps",
> +            .type = QEMU_OPT_NUMBER,
> +            .help = "total bytes burst",
> +        }, {
> +            .name = "bps_rd",
> +            .type = QEMU_OPT_NUMBER,
> +            .help = "total bytes read burst",
> +        }, {


At the very least, if we must have multiple throttle implementations,
can they at least share code so that we don't forget to update one when
adding another option to the other?
Greg Kurz Sept. 8, 2016, 7:34 a.m. UTC | #3 
On Wed, 7 Sep 2016 16:36:12 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 09/07/2016 07:39 AM, Pradeep 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 <pradeep.jagadeesh@huawei.com>
> >  fsdev/file-op-9p.h      |   3 +
> >  fsdev/qemu-fsdev-opts.c |  52 +++++++++++++
> >  hw/9pfs/9p-local.c      |  18 ++++-
> >  hw/9pfs/9p-throttle.c   | 199 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/9pfs/9p-throttle.h   |  55 +++++++++++++
> >  hw/9pfs/9p.c            |   7 ++
> >  hw/9pfs/Makefile.objs   |   1 +
> >  7 files changed, 333 insertions(+), 2 deletions(-)
> >  create mode 100644 hw/9pfs/9p-throttle.c
> >  create mode 100644 hw/9pfs/9p-throttle.h
> > 
> > Hi All,
> > 
> > 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.  
> 
> CLI-only extensions mean that we can't use it during hotplug.  Are you
> sure that blockdev-add won't automatically enable these options, given
> that it tries to convert the dictionary passed through QMP back into the
> QemuOpts form that matches command line parsing?  Also, aren't these

9p-local is fsdev, not blockdev: it has its own QemuOpts and cannot be
passed to blockdev-add (and BTW, there isn't any fsdev-add either).

> limits very similar to what is already available in the throttle driver?

Yes, IIUC this patch tries to implement similar throttling concepts to the
read/write operations in fsdev.

More details on the motivation here:

https://lists.gnu.org/archive/html/qemu-discuss/2016-04/msg00064.html

> Why are we repeating them at the 9p driver, instead of putting a
> throttle group BDS in the mix?
> 

Not sure if it is possible to mix fsdev and blockdev... to be honest, I
really don't know how one would achieve that :)

> > +++ b/fsdev/qemu-fsdev-opts.c
> > @@ -37,6 +37,58 @@ static QemuOptsList qemu_fsdev_opts = {
> >          }, {
> >              .name = "sock_fd",
> >              .type = QEMU_OPT_NUMBER,
> > +        }, {
> > +            .name = "bps",
> > +            .type = QEMU_OPT_NUMBER,
> > +            .help = "total bytes burst",
> > +        }, {
> > +            .name = "bps_rd",
> > +            .type = QEMU_OPT_NUMBER,
> > +            .help = "total bytes read burst",
> > +        }, {  
> 
> 
> At the very least, if we must have multiple throttle implementations,
> can they at least share code so that we don't forget to update one when
> adding another option to the other?
> 

This is experimental work. If it turns out that fsdev and blockdev
throttling are close enough, I agree that they should probably share
code. But at this point, I guess they should remain distinct.

Cheers.

--
Greg
diff mbox

Patch

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 6db9fea..e86b91a 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 "hw/9pfs/9p-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..f5e5e67 100644
--- a/fsdev/qemu-fsdev-opts.c
+++ b/fsdev/qemu-fsdev-opts.c
@@ -37,6 +37,58 @@  static QemuOptsList qemu_fsdev_opts = {
         }, {
             .name = "sock_fd",
             .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "bps",
+            .type = QEMU_OPT_NUMBER,
+            .help = "total bytes burst",
+        }, {
+            .name = "bps_rd",
+            .type = QEMU_OPT_NUMBER,
+            .help = "total bytes read burst",
+        }, {
+            .name = "bps_wr",
+            .type = QEMU_OPT_NUMBER,
+            .help = "total bytes write burst",
+        }, {
+            .name = "iops",
+            .type = QEMU_OPT_NUMBER,
+            .help = "total io operations burst",
+        }, {
+            .name = "iops_rd",
+            .type = QEMU_OPT_NUMBER,
+            .help = "total io operations read burst",
+        }, {
+            .name = "iops_wr",
+            .type = QEMU_OPT_NUMBER,
+            .help = "total io operations write burst",
+        }, {
+            .name = "bps_max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "maximum bytes burst",
+        }, {
+            .name = "bps_rd_max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "maximum bytes read burst",
+        }, {
+            .name = "bps_wr_max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "maximum bytes write burst",
+        }, {
+            .name = "iops_max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "maximum io operations burst",
+        }, {
+            .name = "iops_rd_max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "maximum io operations read burst",
+        }, {
+            .name = "iops_wr_max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "maximum io operations write burst",
+        }, {
+            .name = "iops_size",
+            .type = QEMU_OPT_NUMBER,
+            .help = "io iops-size",
         },
 
         { /*End of list */ }
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 3f271fc..7097f46 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -420,6 +420,8 @@  static ssize_t local_preadv(FsContext *ctx, V9fsFidOpenState *fs,
                             const struct iovec *iov,
                             int iovcnt, off_t offset)
 {
+    throttle_request(ctx, false, iov->iov_len);
+
 #ifdef CONFIG_PREADV
     return preadv(fs->fd, iov, iovcnt, offset);
 #else
@@ -436,8 +438,10 @@  static ssize_t local_pwritev(FsContext *ctx, V9fsFidOpenState *fs,
                              const struct iovec *iov,
                              int iovcnt, off_t offset)
 {
-    ssize_t ret
-;
+    ssize_t ret;
+
+    throttle_request(ctx, true, iov->iov_len);
+
 #ifdef CONFIG_PREADV
     ret = pwritev(fs->fd, iov, iovcnt, offset);
 #else
@@ -1213,6 +1217,9 @@  static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
     const char *sec_model = qemu_opt_get(opts, "security_model");
     const char *path = qemu_opt_get(opts, "path");
 
+    /* get the throttle structure */
+    FsThrottle *fst = &fse->fst;
+
     if (!sec_model) {
         error_report("Security model not specified, local fs needs security model");
         error_printf("valid options are:"
@@ -1240,6 +1247,13 @@  static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
         error_report("fsdev: No path specified");
         return -1;
     }
+
+    check_io_limits(opts, fst);
+
+    if (get_io_limits_state(fst)) {
+        throttle_configure_9p_local(opts, fst);
+    }
+
     fse->path = g_strdup(path);
 
     return 0;
diff --git a/hw/9pfs/9p-throttle.c b/hw/9pfs/9p-throttle.c
new file mode 100644
index 0000000..df8e961
--- /dev/null
+++ b/hw/9pfs/9p-throttle.c
@@ -0,0 +1,199 @@ 
+/*
+ * 9P 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 "fsdev/qemu-fsdev.h"   /* local_ops */
+#include "qemu/cutils.h"
+#include "qemu/error-report.h"
+#include <libgen.h>
+#include <linux/fs.h>
+#include <sys/ioctl.h>
+#include <string.h>
+#include "fsdev/file-op-9p.h"
+#include "9p-throttle.h"
+
+void check_io_limits(QemuOpts *opts, FsThrottle *fst)
+{
+    const float rdbps = qemu_opt_get_number(opts, "bps_rd", 0);
+    const float wrbps = qemu_opt_get_number(opts, "bps_wr", 0);
+    const float rdops = qemu_opt_get_number(opts, "iops_rd", 0);
+    const float wrops = qemu_opt_get_number(opts, "iops_wr", 0);
+
+    if (rdbps > 0 || wrbps > 0 || rdops > 0 || wrops > 0) {
+        fst->io_limits_enabled = true;
+    } else {
+        fst->io_limits_enabled = false;
+    }
+}
+
+bool get_io_limits_state(FsThrottle *fst)
+{
+
+    return fst->io_limits_enabled;
+}
+
+static void read_timer_cb(void *opaque)
+{
+    timer_cb(opaque, false);
+}
+
+static void write_timer_cb(void *opaque)
+{
+    timer_cb(opaque, true);
+}
+
+void throttle_configure_9p_local(QemuOpts *opts, FsThrottle *fst)
+{
+    memset(&fst->ts, 1, sizeof(fst->ts));
+    memset(&fst->tt, 1, sizeof(fst->tt));
+    memset(&fst->cfg, 0, sizeof(fst->cfg));
+    fst->aioctx = qemu_get_aio_context();
+
+    if (!fst->aioctx) {
+        error_report("Failed to create AIO Context");
+        exit(1);
+    }
+    throttle_init(&fst->ts);
+    throttle_timers_init(&fst->tt,
+                         fst->aioctx,
+                         QEMU_CLOCK_REALTIME,
+                         read_timer_cb,
+                         write_timer_cb,
+                         fst);
+    throttle_config_init(&fst->cfg);
+    g_assert(throttle_is_valid(&fst->cfg, NULL));
+
+    qemu_co_queue_init(&fst->throttled_reqs[0]);
+    qemu_co_queue_init(&fst->throttled_reqs[1]);
+    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
+          qemu_opt_get_number(opts, "bps", 0);
+    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
+          qemu_opt_get_number(opts, "bps_rd", 0);
+    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
+          qemu_opt_get_number(opts, "bps_wr", 0);
+    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
+          qemu_opt_get_number(opts, "iops", 0);
+    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
+          qemu_opt_get_number(opts, "iops_rd", 0);
+    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
+          qemu_opt_get_number(opts, "iops_wr", 0);
+
+    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
+          qemu_opt_get_number(opts, "bps_max", 0);
+    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
+          qemu_opt_get_number(opts, "bps_rd_max", 0);
+    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
+          qemu_opt_get_number(opts, "bps_wr_max", 0);
+    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
+          qemu_opt_get_number(opts, "iops_max", 0);
+    fst->cfg.buckets[THROTTLE_OPS_READ].max =
+          qemu_opt_get_number(opts, "iops_rd_max", 0);
+    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
+          qemu_opt_get_number(opts, "iops_wr_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, "iops_size", 0);
+
+    throttle_config(&fst->ts, &fst->tt, &fst->cfg);
+    if (!throttle_is_valid(&fst->cfg, NULL)) {
+        return;
+    }
+
+    g_assert(fst->tt.timers[0]);
+    g_assert(fst->tt.timers[1]);
+    fst->pending_reqs[0] = 0;
+    fst->pending_reqs[1] = 0;
+    fst->any_timer_armed[0] = false;
+    fst->any_timer_armed[1] = false;
+    qemu_mutex_init(&fst->lock);
+}
+
+bool check_for_wait(FsThrottle *fst, bool is_write)
+{
+    if (fst->any_timer_armed[is_write]) {
+        return true;
+    } else {
+        return throttle_schedule_timer(&fst->ts, &fst->tt, is_write);
+    }
+}
+
+void schedule_next_request(FsThrottle *fst, bool is_write)
+{
+    bool must_wait = check_for_wait(fst, is_write);
+    if (!fst->pending_reqs[is_write]) {
+        return;
+    }
+    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);
+           fst->any_timer_armed[is_write] = true;
+       }
+   }
+}
+
+void timer_cb(FsThrottle *fst, bool is_write)
+{
+    bool empty_queue;
+    qemu_mutex_lock(&fst->lock);
+    fst->any_timer_armed[is_write] = false;
+    qemu_mutex_unlock(&fst->lock);
+    empty_queue = !qemu_co_enter_next(&fst->throttled_reqs[is_write]);
+    if (empty_queue) {
+        qemu_mutex_lock(&fst->lock);
+        schedule_next_request(fst, is_write);
+        qemu_mutex_unlock(&fst->lock);
+    }
+}
+
+void throttle_request(FsContext *ctx, bool is_write, ssize_t bytes)
+{
+    FsThrottle *fst = ctx->fst;
+    if (fst->io_limits_enabled) {
+        qemu_mutex_lock(&fst->lock);
+        bool must_wait = check_for_wait(fst, is_write);
+        if (must_wait || fst->pending_reqs[is_write]) {
+            fst->pending_reqs[is_write]++;
+            qemu_mutex_unlock(&fst->lock);
+            qemu_co_queue_wait(&fst->throttled_reqs[is_write]);
+            qemu_mutex_lock(&fst->lock);
+            fst->pending_reqs[is_write]--;
+       }
+       throttle_account(&fst->ts, is_write, bytes);
+       schedule_next_request(fst, is_write);
+       qemu_mutex_unlock(&fst->lock);
+    }
+}
+
+
+void throttle_cleanup(FsContext *ctx)
+{
+    throttle_timers_destroy(&ctx->fst->tt);
+    qemu_mutex_destroy(&ctx->fst->lock);
+}
diff --git a/hw/9pfs/9p-throttle.h b/hw/9pfs/9p-throttle.h
new file mode 100644
index 0000000..9044acf
--- /dev/null
+++ b/hw/9pfs/9p-throttle.h
@@ -0,0 +1,55 @@ 
+/*
+ * 9P 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 _9P_THROTTLE_H
+#define _9P_THROTTLE_H
+
+#include <stdbool.h>
+#include <stdint.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;
+    bool io_limits_enabled;
+    CoQueue      throttled_reqs[2];
+    unsigned     pending_reqs[2];
+    bool any_timer_armed[2];
+    QemuMutex lock;
+} FsThrottle;
+
+
+typedef struct FsContext FsContext;
+
+void check_io_limits(QemuOpts *, FsThrottle *);
+
+bool get_io_limits_state(FsThrottle *);
+
+void schedule_next_request(FsThrottle *, bool);
+
+bool check_for_wait(FsThrottle *, bool);
+
+void throttle_configure_9p_local(QemuOpts *, FsThrottle *);
+
+void timer_cb(FsThrottle *, bool);
+
+void throttle_request(FsContext *, bool , ssize_t);
+
+void throttle_cleanup(FsContext *);
+#endif /* _9P_THROTTLE_H */
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index dfe293d..35e2c10 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3490,6 +3490,10 @@  int v9fs_device_realize_common(V9fsState *s, Error **errp)
         error_setg(errp, "share path %s is not a directory", fse->path);
         goto out;
     }
+
+    /* Throttle structure initialization */
+    s->ctx.fst = &fse->fst;
+
     v9fs_path_free(&path);
 
     rc = 0;
@@ -3504,6 +3508,9 @@  out:
 
 void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
 {
+    if (get_io_limits_state(s->ctx.fst)) {
+        throttle_cleanup(&s->ctx);
+    }
     g_free(s->ctx.fs_root);
     g_free(s->tag);
 }
diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
index da0ae0c..07523f1 100644
--- a/hw/9pfs/Makefile.objs
+++ b/hw/9pfs/Makefile.objs
@@ -5,5 +5,6 @@  common-obj-y += coth.o cofs.o codir.o cofile.o
 common-obj-y += coxattr.o 9p-synth.o
 common-obj-$(CONFIG_OPEN_BY_HANDLE) +=  9p-handle.o
 common-obj-y += 9p-proxy.o
+common-obj-y += 9p-throttle.o
 
 obj-y += virtio-9p-device.o