* Re: virtio ring layout changes for optimal single-stream performance
From: Cornelia Huck @ 2016-01-21 15:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-dev, kvm, dev, linux-kernel, qemu-devel, Xie, Huawei,
virtio, virtualization
In-Reply-To: <20160121145418-mutt-send-email-mst@redhat.com>
On Thu, 21 Jan 2016 15:39:26 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Hi all!
> I have been experimenting with alternative virtio ring layouts,
> in order to speed up single stream performance.
>
> I have just posted a benchmark I wrote for the purpose, and a (partial)
> alternative layout implementation. This achieves 20-40% reduction in
> virtio overhead in the (default) polling mode.
>
> http://article.gmane.org/gmane.linux.kernel.virtualization/26889
>
> The layout is trying to be as simple as possible, to reduce
> the number of cache lines bouncing between CPUs.
Some kind of diagram or textual description would really help to review
this.
>
> For benchmarking, the idea is to emulate virtio in user-space,
> artificially adding overhead for e.g. signalling to match what happens
> in case of a VM.
Hm... is this overhead comparable enough between different platform so
that you can get a halfway realistic scenario? What about things like
endianness conversions?
>
> I'd be very curious to get feedback on this, in particular, some people
> discussed using vectored operations to format virtio ring - would it
> conflict with this work?
>
> You are all welcome to post enhancements or more layout alternatives as
> patches.
Let me see if I can find time to experiment a bit.
^ permalink raw reply
* virtio ring layout changes for optimal single-stream performance
From: Michael S. Tsirkin @ 2016-01-21 13:39 UTC (permalink / raw)
To: virtio
Cc: virtio-dev, kvm, dev, linux-kernel, qemu-devel, virtualization,
Xie, Huawei
Hi all!
I have been experimenting with alternative virtio ring layouts,
in order to speed up single stream performance.
I have just posted a benchmark I wrote for the purpose, and a (partial)
alternative layout implementation. This achieves 20-40% reduction in
virtio overhead in the (default) polling mode.
http://article.gmane.org/gmane.linux.kernel.virtualization/26889
The layout is trying to be as simple as possible, to reduce
the number of cache lines bouncing between CPUs.
For benchmarking, the idea is to emulate virtio in user-space,
artificially adding overhead for e.g. signalling to match what happens
in case of a VM.
I'd be very curious to get feedback on this, in particular, some people
discussed using vectored operations to format virtio ring - would it
conflict with this work?
You are all welcome to post enhancements or more layout alternatives as
patches.
TODO:
- documentation+discussion of interaction with CPU caching
- thorough benchmarking of different configurations/hosts
- experiment with event index replacements
- better emulate vmexit/vmentry cost overhead
- virtio spec proposal
Thanks!
--
MST
^ permalink raw reply
* [PATCH] tools/virtio: add ringtest utilities
From: Michael S. Tsirkin @ 2016-01-21 12:52 UTC (permalink / raw)
To: linux-kernel; +Cc: virtualization
This adds micro-benchmarks useful for tuning virtio ring layouts.
Three layouts are currently implemented:
- virtio 0.9 compatible one
- an experimental extension bypassing the ring index, polling ring
itself instead
- an experimental extension bypassing avail and used ring completely
Typical use:
sh run-on-all.sh perf stat -r 10 --log-fd 1 -- ./ring
It doesn't depend on the kernel directly, but it's handy
to have as much virtio stuff as possible in one tree.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
I have this in my tree at the moment. As this is completely standalone,
I think it's reasonable to merge this in 4.5.
tools/virtio/ringtest/main.h | 119 ++++++++++
tools/virtio/ringtest/main.c | 366 +++++++++++++++++++++++++++++++
tools/virtio/ringtest/ring.c | 272 +++++++++++++++++++++++
tools/virtio/ringtest/virtio_ring_0_9.c | 316 ++++++++++++++++++++++++++
tools/virtio/ringtest/virtio_ring_poll.c | 2 +
tools/virtio/ringtest/Makefile | 22 ++
tools/virtio/ringtest/README | 2 +
tools/virtio/ringtest/run-on-all.sh | 24 ++
8 files changed, 1123 insertions(+)
create mode 100644 tools/virtio/ringtest/main.h
create mode 100644 tools/virtio/ringtest/main.c
create mode 100644 tools/virtio/ringtest/ring.c
create mode 100644 tools/virtio/ringtest/virtio_ring_0_9.c
create mode 100644 tools/virtio/ringtest/virtio_ring_poll.c
create mode 100644 tools/virtio/ringtest/Makefile
create mode 100644 tools/virtio/ringtest/README
create mode 100755 tools/virtio/ringtest/run-on-all.sh
diff --git a/tools/virtio/ringtest/main.h b/tools/virtio/ringtest/main.h
new file mode 100644
index 0000000..16917ac
--- /dev/null
+++ b/tools/virtio/ringtest/main.h
@@ -0,0 +1,119 @@
+/*
+ * Copyright (C) 2016 Red Hat, Inc.
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ *
+ * Common macros and functions for ring benchmarking.
+ */
+#ifndef MAIN_H
+#define MAIN_H
+
+#include <stdbool.h>
+
+extern bool do_exit;
+
+#if defined(__x86_64__) || defined(__i386__)
+#include "x86intrin.h"
+
+static inline void wait_cycles(unsigned long long cycles)
+{
+ unsigned long long t;
+
+ t = __rdtsc();
+ while (__rdtsc() - t < cycles) {}
+}
+
+#define VMEXIT_CYCLES 500
+#define VMENTRY_CYCLES 500
+
+#else
+static inline void wait_cycles(unsigned long long cycles)
+{
+ _Exit(5);
+}
+#define VMEXIT_CYCLES 0
+#define VMENTRY_CYCLES 0
+#endif
+
+static inline void vmexit(void)
+{
+ if (!do_exit)
+ return;
+
+ wait_cycles(VMEXIT_CYCLES);
+}
+static inline void vmentry(void)
+{
+ if (!do_exit)
+ return;
+
+ wait_cycles(VMENTRY_CYCLES);
+}
+
+/* implemented by ring */
+void alloc_ring(void);
+/* guest side */
+int add_inbuf(unsigned, void *, void *);
+void *get_buf(unsigned *, void **);
+void disable_call();
+bool enable_call();
+void kick_available();
+void poll_used();
+/* host side */
+void disable_kick();
+bool enable_kick();
+bool use_buf(unsigned *, void **);
+void call_used();
+void poll_avail();
+
+/* implemented by main */
+extern bool do_sleep;
+void kick(void);
+void wait_for_kick(void);
+void call(void);
+void wait_for_call(void);
+
+extern unsigned ring_size;
+
+/* Compiler barrier - similar to what Linux uses */
+#define barrier() asm volatile("" ::: "memory")
+
+/* Is there a portable way to do this? */
+#if defined(__x86_64__) || defined(__i386__)
+#define cpu_relax() asm ("rep; nop" ::: "memory")
+#else
+#define cpu_relax() assert(0)
+#endif
+
+extern bool do_relax;
+
+static inline void busy_wait(void)
+{
+ if (do_relax)
+ cpu_relax();
+ else
+ /* prevent compiler from removing busy loops */
+ barrier();
+}
+
+/*
+ * Not using __ATOMIC_SEQ_CST since gcc docs say they are only synchronized
+ * with other __ATOMIC_SEQ_CST calls.
+ */
+#define smp_mb() __sync_synchronize()
+
+/*
+ * This abuses the atomic builtins for thread fences, and
+ * adds a compiler barrier.
+ */
+#define smp_release() do { \
+ barrier(); \
+ __atomic_thread_fence(__ATOMIC_RELEASE); \
+} while (0)
+
+#define smp_acquire() do { \
+ __atomic_thread_fence(__ATOMIC_ACQUIRE); \
+ barrier(); \
+} while (0)
+
+#endif
diff --git a/tools/virtio/ringtest/main.c b/tools/virtio/ringtest/main.c
new file mode 100644
index 0000000..3a5ff43
--- /dev/null
+++ b/tools/virtio/ringtest/main.c
@@ -0,0 +1,366 @@
+/*
+ * Copyright (C) 2016 Red Hat, Inc.
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ *
+ * Command line processing and common functions for ring benchmarking.
+ */
+#define _GNU_SOURCE
+#include <getopt.h>
+#include <pthread.h>
+#include <assert.h>
+#include <sched.h>
+#include "main.h"
+#include <sys/eventfd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <limits.h>
+
+int runcycles = 10000000;
+int max_outstanding = INT_MAX;
+int batch = 1;
+
+bool do_sleep = false;
+bool do_relax = false;
+bool do_exit = true;
+
+unsigned ring_size = 256;
+
+static int kickfd = -1;
+static int callfd = -1;
+
+void notify(int fd)
+{
+ unsigned long long v = 1;
+ int r;
+
+ vmexit();
+ r = write(fd, &v, sizeof v);
+ assert(r == sizeof v);
+ vmentry();
+}
+
+void wait_for_notify(int fd)
+{
+ unsigned long long v = 1;
+ int r;
+
+ vmexit();
+ r = read(fd, &v, sizeof v);
+ assert(r == sizeof v);
+ vmentry();
+}
+
+void kick(void)
+{
+ notify(kickfd);
+}
+
+void wait_for_kick(void)
+{
+ wait_for_notify(kickfd);
+}
+
+void call(void)
+{
+ notify(callfd);
+}
+
+void wait_for_call(void)
+{
+ wait_for_notify(callfd);
+}
+
+void set_affinity(const char *arg)
+{
+ cpu_set_t cpuset;
+ int ret;
+ pthread_t self;
+ long int cpu;
+ char *endptr;
+
+ if (!arg)
+ return;
+
+ cpu = strtol(arg, &endptr, 0);
+ assert(!*endptr);
+
+ assert(cpu >= 0 || cpu < CPU_SETSIZE);
+
+ self = pthread_self();
+ CPU_ZERO(&cpuset);
+ CPU_SET(cpu, &cpuset);
+
+ ret = pthread_setaffinity_np(self, sizeof(cpu_set_t), &cpuset);
+ assert(!ret);
+}
+
+static void run_guest(void)
+{
+ int completed_before;
+ int completed = 0;
+ int started = 0;
+ int bufs = runcycles;
+ int spurious = 0;
+ int r;
+ unsigned len;
+ void *buf;
+ int tokick = batch;
+
+ for (;;) {
+ if (do_sleep)
+ disable_call();
+ completed_before = completed;
+ do {
+ if (started < bufs &&
+ started - completed < max_outstanding) {
+ r = add_inbuf(0, NULL, "Hello, world!");
+ if (__builtin_expect(r == 0, true)) {
+ ++started;
+ if (!--tokick) {
+ tokick = batch;
+ if (do_sleep)
+ kick_available();
+ }
+
+ }
+ } else
+ r = -1;
+
+ /* Flush out completed bufs if any */
+ if (get_buf(&len, &buf)) {
+ ++completed;
+ if (__builtin_expect(completed == bufs, false))
+ return;
+ r = 0;
+ }
+ } while (r == 0);
+ if (completed == completed_before)
+ ++spurious;
+ assert(completed <= bufs);
+ assert(started <= bufs);
+ if (do_sleep) {
+ if (enable_call())
+ wait_for_call();
+ } else {
+ poll_used();
+ }
+ }
+}
+
+static void run_host(void)
+{
+ int completed_before;
+ int completed = 0;
+ int spurious = 0;
+ int bufs = runcycles;
+ unsigned len;
+ void *buf;
+
+ for (;;) {
+ if (do_sleep) {
+ if (enable_kick())
+ wait_for_kick();
+ } else {
+ poll_avail();
+ }
+ if (do_sleep)
+ disable_kick();
+ completed_before = completed;
+ while (__builtin_expect(use_buf(&len, &buf), true)) {
+ if (do_sleep)
+ call_used();
+ ++completed;
+ if (__builtin_expect(completed == bufs, false))
+ return;
+ }
+ if (completed == completed_before)
+ ++spurious;
+ assert(completed <= bufs);
+ if (completed == bufs)
+ break;
+ }
+}
+
+void *start_guest(void *arg)
+{
+ set_affinity(arg);
+ run_guest();
+ pthread_exit(NULL);
+}
+
+void *start_host(void *arg)
+{
+ set_affinity(arg);
+ run_host();
+ pthread_exit(NULL);
+}
+
+static const char optstring[] = "";
+static const struct option longopts[] = {
+ {
+ .name = "help",
+ .has_arg = no_argument,
+ .val = 'h',
+ },
+ {
+ .name = "host-affinity",
+ .has_arg = required_argument,
+ .val = 'H',
+ },
+ {
+ .name = "guest-affinity",
+ .has_arg = required_argument,
+ .val = 'G',
+ },
+ {
+ .name = "ring-size",
+ .has_arg = required_argument,
+ .val = 'R',
+ },
+ {
+ .name = "run-cycles",
+ .has_arg = required_argument,
+ .val = 'C',
+ },
+ {
+ .name = "outstanding",
+ .has_arg = required_argument,
+ .val = 'o',
+ },
+ {
+ .name = "batch",
+ .has_arg = required_argument,
+ .val = 'b',
+ },
+ {
+ .name = "sleep",
+ .has_arg = no_argument,
+ .val = 's',
+ },
+ {
+ .name = "relax",
+ .has_arg = no_argument,
+ .val = 'x',
+ },
+ {
+ .name = "exit",
+ .has_arg = no_argument,
+ .val = 'e',
+ },
+ {
+ }
+};
+
+static void help(void)
+{
+ fprintf(stderr, "Usage: <test> [--help]"
+ " [--host-affinity H]"
+ " [--guest-affinity G]"
+ " [--ring-size R (default: %d)]"
+ " [--run-cycles C (default: %d)]"
+ " [--batch b]"
+ " [--outstanding o]"
+ " [--sleep]"
+ " [--relax]"
+ " [--exit]"
+ "\n",
+ ring_size,
+ runcycles);
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+ pthread_t host, guest;
+ void *tret;
+ char *host_arg = NULL;
+ char *guest_arg = NULL;
+ char *endptr;
+ long int c;
+
+ kickfd = eventfd(0, 0);
+ assert(kickfd >= 0);
+ callfd = eventfd(0, 0);
+ assert(callfd >= 0);
+
+ for (;;) {
+ int o = getopt_long(argc, argv, optstring, longopts, NULL);
+ switch (o) {
+ case -1:
+ goto done;
+ case '?':
+ help();
+ exit(2);
+ case 'H':
+ host_arg = optarg;
+ break;
+ case 'G':
+ guest_arg = optarg;
+ break;
+ case 'R':
+ ring_size = strtol(optarg, &endptr, 0);
+ assert(ring_size && !(ring_size & (ring_size - 1)));
+ assert(!*endptr);
+ break;
+ case 'C':
+ c = strtol(optarg, &endptr, 0);
+ assert(!*endptr);
+ assert(c > 0 && c < INT_MAX);
+ runcycles = c;
+ break;
+ case 'o':
+ c = strtol(optarg, &endptr, 0);
+ assert(!*endptr);
+ assert(c > 0 && c < INT_MAX);
+ max_outstanding = c;
+ break;
+ case 'b':
+ c = strtol(optarg, &endptr, 0);
+ assert(!*endptr);
+ assert(c > 0 && c < INT_MAX);
+ batch = c;
+ break;
+ case 's':
+ do_sleep = true;
+ break;
+ case 'x':
+ do_relax = true;
+ break;
+ case 'e':
+ do_exit = true;
+ break;
+ default:
+ help();
+ exit(4);
+ break;
+ }
+ }
+
+ /* does nothing here, used to make sure all smp APIs compile */
+ smp_acquire();
+ smp_release();
+ smp_mb();
+done:
+
+ if (batch > max_outstanding)
+ batch = max_outstanding;
+
+ if (optind < argc) {
+ help();
+ exit(4);
+ }
+ alloc_ring();
+
+ ret = pthread_create(&host, NULL, start_host, host_arg);
+ assert(!ret);
+ ret = pthread_create(&guest, NULL, start_guest, guest_arg);
+ assert(!ret);
+
+ ret = pthread_join(guest, &tret);
+ assert(!ret);
+ ret = pthread_join(host, &tret);
+ assert(!ret);
+ return 0;
+}
diff --git a/tools/virtio/ringtest/ring.c b/tools/virtio/ringtest/ring.c
new file mode 100644
index 0000000..c25c8d2
--- /dev/null
+++ b/tools/virtio/ringtest/ring.c
@@ -0,0 +1,272 @@
+/*
+ * Copyright (C) 2016 Red Hat, Inc.
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ *
+ * Simple descriptor-based ring. virtio 0.9 compatible event index is used for
+ * signalling, unconditionally.
+ */
+#define _GNU_SOURCE
+#include "main.h"
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+/* Next - Where next entry will be written.
+ * Prev - "Next" value when event triggered previously.
+ * Event - Peer requested event after writing this entry.
+ */
+static inline bool need_event(unsigned short event,
+ unsigned short next,
+ unsigned short prev)
+{
+ return (unsigned short)(next - event - 1) < (unsigned short)(next - prev);
+}
+
+/* Design:
+ * Guest adds descriptors with unique index values and DESC_HW in flags.
+ * Host overwrites used descriptors with correct len, index, and DESC_HW clear.
+ * Flags are always set last.
+ */
+#define DESC_HW 0x1
+
+struct desc {
+ unsigned short flags;
+ unsigned short index;
+ unsigned len;
+ unsigned long long addr;
+};
+
+/* how much padding is needed to avoid false cache sharing */
+#define HOST_GUEST_PADDING 0x80
+
+/* Mostly read */
+struct event {
+ unsigned short kick_index;
+ unsigned char reserved0[HOST_GUEST_PADDING - 2];
+ unsigned short call_index;
+ unsigned char reserved1[HOST_GUEST_PADDING - 2];
+};
+
+struct data {
+ void *buf; /* descriptor is writeable, we can't get buf from there */
+ void *data;
+} *data;
+
+struct desc *ring;
+struct event *event;
+
+struct guest {
+ unsigned avail_idx;
+ unsigned last_used_idx;
+ unsigned num_free;
+ unsigned kicked_avail_idx;
+ unsigned char reserved[HOST_GUEST_PADDING - 12];
+} guest;
+
+struct host {
+ /* we do not need to track last avail index
+ * unless we have more than one in flight.
+ */
+ unsigned used_idx;
+ unsigned called_used_idx;
+ unsigned char reserved[HOST_GUEST_PADDING - 4];
+} host;
+
+/* implemented by ring */
+void alloc_ring(void)
+{
+ int ret;
+ int i;
+
+ ret = posix_memalign((void **)&ring, 0x1000, ring_size * sizeof *ring);
+ if (ret) {
+ perror("Unable to allocate ring buffer.\n");
+ exit(3);
+ }
+ event = malloc(sizeof *event);
+ if (!event) {
+ perror("Unable to allocate event buffer.\n");
+ exit(3);
+ }
+ memset(event, 0, sizeof *event);
+ guest.avail_idx = 0;
+ guest.kicked_avail_idx = -1;
+ guest.last_used_idx = 0;
+ host.used_idx = 0;
+ host.called_used_idx = -1;
+ for (i = 0; i < ring_size; ++i) {
+ struct desc desc = {
+ .index = i,
+ };
+ ring[i] = desc;
+ }
+ guest.num_free = ring_size;
+ data = malloc(ring_size * sizeof *data);
+ if (!data) {
+ perror("Unable to allocate data buffer.\n");
+ exit(3);
+ }
+ memset(data, 0, ring_size * sizeof *data);
+}
+
+/* guest side */
+int add_inbuf(unsigned len, void *buf, void *datap)
+{
+ unsigned head, index;
+
+ if (!guest.num_free)
+ return -1;
+
+ guest.num_free--;
+ head = (ring_size - 1) & (guest.avail_idx++);
+
+ /* Start with a write. On MESI architectures this helps
+ * avoid a shared state with consumer that is polling this descriptor.
+ */
+ ring[head].addr = (unsigned long)(void*)buf;
+ ring[head].len = len;
+ /* read below might bypass write above. That is OK because it's just an
+ * optimization. If this happens, we will get the cache line in a
+ * shared state which is unfortunate, but probably not worth it to
+ * add an explicit full barrier to avoid this.
+ */
+ barrier();
+ index = ring[head].index;
+ data[index].buf = buf;
+ data[index].data = datap;
+ /* Barrier A (for pairing) */
+ smp_release();
+ ring[head].flags = DESC_HW;
+
+ return 0;
+}
+
+void *get_buf(unsigned *lenp, void **bufp)
+{
+ unsigned head = (ring_size - 1) & guest.last_used_idx;
+ unsigned index;
+ void *datap;
+
+ if (ring[head].flags & DESC_HW)
+ return NULL;
+ /* Barrier B (for pairing) */
+ smp_acquire();
+ *lenp = ring[head].len;
+ index = ring[head].index & (ring_size - 1);
+ datap = data[index].data;
+ *bufp = data[index].buf;
+ data[index].buf = NULL;
+ data[index].data = NULL;
+ guest.num_free++;
+ guest.last_used_idx++;
+ return datap;
+}
+
+void poll_used(void)
+{
+ unsigned head = (ring_size - 1) & guest.last_used_idx;
+
+ while (ring[head].flags & DESC_HW)
+ busy_wait();
+}
+
+void disable_call()
+{
+ /* Doing nothing to disable calls might cause
+ * extra interrupts, but reduces the number of cache misses.
+ */
+}
+
+bool enable_call()
+{
+ unsigned head = (ring_size - 1) & guest.last_used_idx;
+
+ event->call_index = guest.last_used_idx;
+ /* Flush call index write */
+ /* Barrier D (for pairing) */
+ smp_mb();
+ return ring[head].flags & DESC_HW;
+}
+
+void kick_available(void)
+{
+ /* Flush in previous flags write */
+ /* Barrier C (for pairing) */
+ smp_mb();
+ if (!need_event(event->kick_index,
+ guest.avail_idx,
+ guest.kicked_avail_idx))
+ return;
+
+ guest.kicked_avail_idx = guest.avail_idx;
+ kick();
+}
+
+/* host side */
+void disable_kick()
+{
+ /* Doing nothing to disable kicks might cause
+ * extra interrupts, but reduces the number of cache misses.
+ */
+}
+
+bool enable_kick()
+{
+ unsigned head = (ring_size - 1) & host.used_idx;
+
+ event->kick_index = host.used_idx;
+ /* Barrier C (for pairing) */
+ smp_mb();
+ return !(ring[head].flags & DESC_HW);
+}
+
+void poll_avail(void)
+{
+ unsigned head = (ring_size - 1) & host.used_idx;
+
+ while (!(ring[head].flags & DESC_HW))
+ busy_wait();
+}
+
+bool use_buf(unsigned *lenp, void **bufp)
+{
+ unsigned head = (ring_size - 1) & host.used_idx;
+
+ if (!(ring[head].flags & DESC_HW))
+ return false;
+
+ /* make sure length read below is not speculated */
+ /* Barrier A (for pairing) */
+ smp_acquire();
+
+ /* simple in-order completion: we don't need
+ * to touch index at all. This also means we
+ * can just modify the descriptor in-place.
+ */
+ ring[head].len--;
+ /* Make sure len is valid before flags.
+ * Note: alternative is to write len and flags in one access -
+ * possible on 64 bit architectures but wmb is free on Intel anyway
+ * so I have no way to test whether it's a gain.
+ */
+ /* Barrier B (for pairing) */
+ smp_release();
+ ring[head].flags = 0;
+ host.used_idx++;
+ return true;
+}
+
+void call_used(void)
+{
+ /* Flush in previous flags write */
+ /* Barrier D (for pairing) */
+ smp_mb();
+ if (!need_event(event->call_index,
+ host.used_idx,
+ host.called_used_idx))
+ return;
+
+ host.called_used_idx = host.used_idx;
+ call();
+}
diff --git a/tools/virtio/ringtest/virtio_ring_0_9.c b/tools/virtio/ringtest/virtio_ring_0_9.c
new file mode 100644
index 0000000..47c9a1a
--- /dev/null
+++ b/tools/virtio/ringtest/virtio_ring_0_9.c
@@ -0,0 +1,316 @@
+/*
+ * Copyright (C) 2016 Red Hat, Inc.
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ *
+ * Partial implementation of virtio 0.9. event index is used for signalling,
+ * unconditionally. Design roughly follows linux kernel implementation in order
+ * to be able to judge its performance.
+ */
+#define _GNU_SOURCE
+#include "main.h"
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <string.h>
+#include <linux/virtio_ring.h>
+
+struct data {
+ void *data;
+} *data;
+
+struct vring ring;
+
+/* enabling the below activates experimental ring polling code
+ * (which skips index reads on consumer in favor of looking at
+ * high bits of ring id ^ 0x8000).
+ */
+/* #ifdef RING_POLL */
+
+/* how much padding is needed to avoid false cache sharing */
+#define HOST_GUEST_PADDING 0x80
+
+struct guest {
+ unsigned short avail_idx;
+ unsigned short last_used_idx;
+ unsigned short num_free;
+ unsigned short kicked_avail_idx;
+ unsigned short free_head;
+ unsigned char reserved[HOST_GUEST_PADDING - 10];
+} guest;
+
+struct host {
+ /* we do not need to track last avail index
+ * unless we have more than one in flight.
+ */
+ unsigned short used_idx;
+ unsigned short called_used_idx;
+ unsigned char reserved[HOST_GUEST_PADDING - 4];
+} host;
+
+/* implemented by ring */
+void alloc_ring(void)
+{
+ int ret;
+ int i;
+ void *p;
+
+ ret = posix_memalign(&p, 0x1000, vring_size(ring_size, 0x1000));
+ if (ret) {
+ perror("Unable to allocate ring buffer.\n");
+ exit(3);
+ }
+ memset(p, 0, vring_size(ring_size, 0x1000));
+ vring_init(&ring, ring_size, p, 0x1000);
+
+ guest.avail_idx = 0;
+ guest.kicked_avail_idx = -1;
+ guest.last_used_idx = 0;
+ /* Put everything in free lists. */
+ guest.free_head = 0;
+ for (i = 0; i < ring_size - 1; i++)
+ ring.desc[i].next = i + 1;
+ host.used_idx = 0;
+ host.called_used_idx = -1;
+ guest.num_free = ring_size;
+ data = malloc(ring_size * sizeof *data);
+ if (!data) {
+ perror("Unable to allocate data buffer.\n");
+ exit(3);
+ }
+ memset(data, 0, ring_size * sizeof *data);
+}
+
+/* guest side */
+int add_inbuf(unsigned len, void *buf, void *datap)
+{
+ unsigned head, avail;
+ struct vring_desc *desc;
+
+ if (!guest.num_free)
+ return -1;
+
+ head = guest.free_head;
+ guest.num_free--;
+
+ desc = ring.desc;
+ desc[head].flags = VRING_DESC_F_NEXT;
+ desc[head].addr = (unsigned long)(void *)buf;
+ desc[head].len = len;
+ /* We do it like this to simulate the way
+ * we'd have to flip it if we had multiple
+ * descriptors.
+ */
+ desc[head].flags &= ~VRING_DESC_F_NEXT;
+ guest.free_head = desc[head].next;
+
+ data[head].data = datap;
+
+#ifdef RING_POLL
+ /* Barrier A (for pairing) */
+ smp_release();
+ avail = guest.avail_idx++;
+ ring.avail->ring[avail & (ring_size - 1)] =
+ (head | (avail & ~(ring_size - 1))) ^ 0x8000;
+#else
+ avail = (ring_size - 1) & (guest.avail_idx++);
+ ring.avail->ring[avail] = head;
+ /* Barrier A (for pairing) */
+ smp_release();
+#endif
+ ring.avail->idx = guest.avail_idx;
+ return 0;
+}
+
+void *get_buf(unsigned *lenp, void **bufp)
+{
+ unsigned head;
+ unsigned index;
+ void *datap;
+
+#ifdef RING_POLL
+ head = (ring_size - 1) & guest.last_used_idx;
+ index = ring.used->ring[head].id;
+ if ((index ^ guest.last_used_idx ^ 0x8000) & ~(ring_size - 1))
+ return NULL;
+ /* Barrier B (for pairing) */
+ smp_acquire();
+ index &= ring_size - 1;
+#else
+ if (ring.used->idx == guest.last_used_idx)
+ return NULL;
+ /* Barrier B (for pairing) */
+ smp_acquire();
+ head = (ring_size - 1) & guest.last_used_idx;
+ index = ring.used->ring[head].id;
+#endif
+ *lenp = ring.used->ring[head].len;
+ datap = data[index].data;
+ *bufp = (void*)(unsigned long)ring.desc[index].addr;
+ data[index].data = NULL;
+ ring.desc[index].next = guest.free_head;
+ guest.free_head = index;
+ guest.num_free++;
+ guest.last_used_idx++;
+ return datap;
+}
+
+void poll_used(void)
+{
+#ifdef RING_POLL
+ unsigned head = (ring_size - 1) & guest.last_used_idx;
+
+ for (;;) {
+ unsigned index = ring.used->ring[head].id;
+
+ if ((index ^ guest.last_used_idx ^ 0x8000) & ~(ring_size - 1))
+ busy_wait();
+ else
+ break;
+ }
+#else
+ unsigned head = guest.last_used_idx;
+
+ while (ring.used->idx == head)
+ busy_wait();
+#endif
+}
+
+void disable_call()
+{
+ /* Doing nothing to disable calls might cause
+ * extra interrupts, but reduces the number of cache misses.
+ */
+}
+
+bool enable_call()
+{
+ unsigned short last_used_idx;
+
+ vring_used_event(&ring) = (last_used_idx = guest.last_used_idx);
+ /* Flush call index write */
+ /* Barrier D (for pairing) */
+ smp_mb();
+#ifdef RING_POLL
+ {
+ unsigned short head = last_used_idx & (ring_size - 1);
+ unsigned index = ring.used->ring[head].id;
+
+ return (index ^ last_used_idx ^ 0x8000) & ~(ring_size - 1);
+ }
+#else
+ return ring.used->idx == last_used_idx;
+#endif
+}
+
+void kick_available(void)
+{
+ /* Flush in previous flags write */
+ /* Barrier C (for pairing) */
+ smp_mb();
+ if (!vring_need_event(vring_avail_event(&ring),
+ guest.avail_idx,
+ guest.kicked_avail_idx))
+ return;
+
+ guest.kicked_avail_idx = guest.avail_idx;
+ kick();
+}
+
+/* host side */
+void disable_kick()
+{
+ /* Doing nothing to disable kicks might cause
+ * extra interrupts, but reduces the number of cache misses.
+ */
+}
+
+bool enable_kick()
+{
+ unsigned head = host.used_idx;
+
+ vring_avail_event(&ring) = head;
+ /* Barrier C (for pairing) */
+ smp_mb();
+#ifdef RING_POLL
+ {
+ unsigned index = ring.avail->ring[head & (ring_size - 1)];
+
+ return (index ^ head ^ 0x8000) & ~(ring_size - 1);
+ }
+#else
+ return head == ring.avail->idx;
+#endif
+}
+
+void poll_avail(void)
+{
+ unsigned head = host.used_idx;
+#ifdef RING_POLL
+ for (;;) {
+ unsigned index = ring.avail->ring[head & (ring_size - 1)];
+ if ((index ^ head ^ 0x8000) & ~(ring_size - 1))
+ busy_wait();
+ else
+ break;
+ }
+#else
+ while (ring.avail->idx == head)
+ busy_wait();
+#endif
+}
+
+bool use_buf(unsigned *lenp, void **bufp)
+{
+ unsigned used_idx = host.used_idx;
+ struct vring_desc *desc;
+ unsigned head;
+
+#ifdef RING_POLL
+ head = ring.avail->ring[used_idx & (ring_size - 1)];
+ if ((used_idx ^ head ^ 0x8000) & ~(ring_size - 1))
+ return false;
+ /* Barrier A (for pairing) */
+ smp_acquire();
+
+ used_idx &= ring_size - 1;
+ desc = &ring.desc[head & (ring_size - 1)];
+#else
+ if (used_idx == ring.avail->idx)
+ return false;
+
+ /* Barrier A (for pairing) */
+ smp_acquire();
+
+ used_idx &= ring_size - 1;
+ head = ring.avail->ring[used_idx];
+ desc = &ring.desc[head];
+#endif
+
+ *lenp = desc->len;
+ *bufp = (void *)(unsigned long)desc->addr;
+
+ /* now update used ring */
+ ring.used->ring[used_idx].id = head;
+ ring.used->ring[used_idx].len = desc->len - 1;
+ /* Barrier B (for pairing) */
+ smp_release();
+ host.used_idx++;
+ ring.used->idx = host.used_idx;
+
+ return true;
+}
+
+void call_used(void)
+{
+ /* Flush in previous flags write */
+ /* Barrier D (for pairing) */
+ smp_mb();
+ if (!vring_need_event(vring_used_event(&ring),
+ host.used_idx,
+ host.called_used_idx))
+ return;
+
+ host.called_used_idx = host.used_idx;
+ call();
+}
diff --git a/tools/virtio/ringtest/virtio_ring_poll.c b/tools/virtio/ringtest/virtio_ring_poll.c
new file mode 100644
index 0000000..84fc2c5
--- /dev/null
+++ b/tools/virtio/ringtest/virtio_ring_poll.c
@@ -0,0 +1,2 @@
+#define RING_POLL 1
+#include "virtio_ring_0_9.c"
diff --git a/tools/virtio/ringtest/Makefile b/tools/virtio/ringtest/Makefile
new file mode 100644
index 0000000..feaa64a
--- /dev/null
+++ b/tools/virtio/ringtest/Makefile
@@ -0,0 +1,22 @@
+all:
+
+all: ring virtio_ring_0_9 virtio_ring_poll
+
+CFLAGS += -Wall
+CFLAGS += -pthread -O2 -ggdb
+LDFLAGS += -pthread -O2 -ggdb
+
+main.o: main.c main.h
+ring.o: ring.c main.h
+virtio_ring_0_9.o: virtio_ring_0_9.c main.h
+virtio_ring_poll.o: virtio_ring_poll.c virtio_ring_0_9.c main.h
+ring: ring.o main.o
+virtio_ring_0_9: virtio_ring_0_9.o main.o
+virtio_ring_poll: virtio_ring_poll.o main.o
+clean:
+ -rm main.o
+ -rm ring.o ring
+ -rm virtio_ring_0_9.o virtio_ring_0_9
+ -rm virtio_ring_poll.o virtio_ring_poll
+
+.PHONY: all clean
diff --git a/tools/virtio/ringtest/README b/tools/virtio/ringtest/README
new file mode 100644
index 0000000..34e94c4
--- /dev/null
+++ b/tools/virtio/ringtest/README
@@ -0,0 +1,2 @@
+Partial implementation of various ring layouts, useful to tune virtio design.
+Uses shared memory heavily.
diff --git a/tools/virtio/ringtest/run-on-all.sh b/tools/virtio/ringtest/run-on-all.sh
new file mode 100755
index 0000000..52b0f71
--- /dev/null
+++ b/tools/virtio/ringtest/run-on-all.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+#use last CPU for host. Why not the first?
+#many devices tend to use cpu0 by default so
+#it tends to be busier
+HOST_AFFINITY=$(cd /dev/cpu; ls|grep -v '[a-z]'|sort -n|tail -1)
+
+#run command on all cpus
+for cpu in $(cd /dev/cpu; ls|grep -v '[a-z]'|sort -n);
+do
+ #Don't run guest and host on same CPU
+ #It actually works ok if using signalling
+ if
+ (echo "$@" | grep -e "--sleep" > /dev/null) || \
+ test $HOST_AFFINITY '!=' $cpu
+ then
+ echo "GUEST AFFINITY $cpu"
+ "$@" --host-affinity $HOST_AFFINITY --guest-affinity $cpu
+ fi
+done
+echo "NO GUEST AFFINITY"
+"$@" --host-affinity $HOST_AFFINITY
+echo "NO AFFINITY"
+"$@"
--
MST
^ permalink raw reply related
* [PATCH] sh: fix smp_store_mb for !SMP
From: Michael S. Tsirkin @ 2016-01-21 10:29 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, Arnd Bergmann, linux-sh, Peter Zijlstra,
virtualization, Linus Torvalds, Ingo Molnar
sh variant of smp_store_mb() calls xchg() on !SMP which is stronger than
implied by both the name and the documentation.
commit 90a3ccb0be538a914e6a5c51ae919762261563ad ("sh: define __smp_xxx,
fix smp_store_mb for !SMP") was supposed to fix it but
left the bug in place.
Drop smp_store_mb, so that code in asm-generic/barrier.h
will define it correctly depending on CONFIG_SMP.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
I put this in my tree as that's where the original commit
came through, but if anyone else prefers to merge it, pls
let me know.
arch/sh/include/asm/barrier.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/sh/include/asm/barrier.h b/arch/sh/include/asm/barrier.h
index f887c64..8a84e05 100644
--- a/arch/sh/include/asm/barrier.h
+++ b/arch/sh/include/asm/barrier.h
@@ -33,7 +33,6 @@
#endif
#define __smp_store_mb(var, value) do { (void)xchg(&var, value); } while (0)
-#define smp_store_mb(var, value) __smp_store_mb(var, value)
#include <asm-generic/barrier.h>
--
MST
^ permalink raw reply related
* Re: virtio pull for 4.5 (was Re: [PULL] virtio: barrier rework+fixes)
From: Michael S. Tsirkin @ 2016-01-21 10:23 UTC (permalink / raw)
To: Linus Torvalds
Cc: tony.luck, Rafael Aquini, KVM list, Network Development,
Boqun Feng, Linux Kernel Mailing List, Alexander Duyck,
virtualization, Arnd Bergmann, Bjorn Andersson,
Benjamin Herrenschmidt, Andrew Morton
In-Reply-To: <CA+55aFyh_1Lpp4U2wsFJQu_4PmSBD=36YeAhQN94L4GUhQrb2Q@mail.gmail.com>
On Mon, Jan 18, 2016 at 05:01:26PM -0800, Linus Torvalds wrote:
> On Mon, Jan 18, 2016 at 5:21 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > Hi Linus,
> > Just making sure nothing's wrong with this pull request.
> > If there's an issue, pls let me know!
>
> It was just pulled because I wasn't 100% sure I wanted the extra
> indirection. Oh well, pulled now.
>
> One question:
>
> - the arch/sh/ part of the pacth looks dubious. Why does it do that
>
> #define smp_store_mb(var, value) __smp_store_mb(var, value)
>
> despite the commit log saying it's done by asm-generic?
>
> I haven't pushed out yet, my allmodconfig sanity-check build is still going..
>
> Linus
Oh that's because that's what the existing code did, so
the original version of the patch left the bug in place.
I wanted to include the fix and I wrote this in the
commit message, but I forgot to include it in the code :(
Thankfully this just means that the commit log is wrong,
the code is just unchanged from 4.4, and I'll
include a fix in my tree shortly.
--
MST
^ permalink raw reply
* Re: [PATCH 2/2] vhost: disentangle vring endianness stuff from the core code
From: Cornelia Huck @ 2016-01-21 9:56 UTC (permalink / raw)
To: Greg Kurz; +Cc: netdev, virtualization, linux-kernel, kvm, Michael S. Tsirkin
In-Reply-To: <20160113170947.23705.95216.stgit@bahia.huguette.org>
On Wed, 13 Jan 2016 18:09:47 +0100
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> The way vring endianness is being handled currently obfuscates
> the code in vhost_init_used().
>
> This patch tries to fix that by doing the following:
> - move the the code that adjusts endianness to a dedicated helper
> - export this helper so that backends explicitely call it
>
> No behaviour change.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> drivers/vhost/net.c | 3 +++
> drivers/vhost/scsi.c | 3 +++
> drivers/vhost/test.c | 2 ++
> drivers/vhost/vhost.c | 16 +++++++++++-----
> drivers/vhost/vhost.h | 1 +
> 5 files changed, 20 insertions(+), 5 deletions(-)
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply
* Re: [PATCH 1/2] vhost: helpers to enable/disable vring endianness
From: Cornelia Huck @ 2016-01-21 9:55 UTC (permalink / raw)
To: Greg Kurz; +Cc: netdev, virtualization, linux-kernel, kvm, Michael S. Tsirkin
In-Reply-To: <20160113170941.23705.93915.stgit@bahia.huguette.org>
On Wed, 13 Jan 2016 18:09:41 +0100
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> The default use case for vhost is when the host and the vring have the
> same endianness (default native endianness). But there are cases where
> they differ and vhost should byteswap when accessing the vring:
> - the host is big endian and the vring comes from a virtio 1.0 device
> which is always little endian
> - the architecture is bi-endian and the vring comes from a legacy virtio
> device with a different endianness than the endianness of the host (aka
> legacy cross-endian)
>
> These cases are handled by the vq->is_le and the optional vq->user_be,
> with the following logic:
> - if none of the fields is enabled, vhost access the vring without byteswap
> - if the vring is virtio 1.0 and the host is big endian, vq->is_le is
> enabled to enforce little endian access to the vring
> - if the vring is legacy cross-endian, userspace enables vq->user_be
> to inform vhost about the vring endianness. This endianness is then
> enforced for vring accesses through vq->is_le again
>
> The logic is unclear in the current code.
>
> This patch introduces helpers with explicit enable and disable semantics,
> for better clarity.
>
> No behaviour change.
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> drivers/vhost/vhost.c | 28 +++++++++++++++++++---------
> 1 file changed, 19 insertions(+), 9 deletions(-)
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply
* Re: [PATCH V2 3/3] vhost_net: basic polling support
From: Yang Zhang @ 2016-01-21 6:39 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20160121064759-mutt-send-email-mst@redhat.com>
On 2016/1/21 13:13, Michael S. Tsirkin wrote:
> On Thu, Jan 21, 2016 at 10:11:35AM +0800, Yang Zhang wrote:
>> On 2016/1/20 22:35, Michael S. Tsirkin wrote:
>>> On Tue, Dec 01, 2015 at 02:39:45PM +0800, Jason Wang wrote:
>>>> This patch tries to poll for new added tx buffer or socket receive
>>>> queue for a while at the end of tx/rx processing. The maximum time
>>>> spent on polling were specified through a new kind of vring ioctl.
>>>>
>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>>> ---
>>>> drivers/vhost/net.c | 72 ++++++++++++++++++++++++++++++++++++++++++----
>>>> drivers/vhost/vhost.c | 15 ++++++++++
>>>> drivers/vhost/vhost.h | 1 +
>>>> include/uapi/linux/vhost.h | 11 +++++++
>>>> 4 files changed, 94 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>>>> index 9eda69e..ce6da77 100644
>>>> --- a/drivers/vhost/net.c
>>>> +++ b/drivers/vhost/net.c
>>>> @@ -287,6 +287,41 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
>>>> rcu_read_unlock_bh();
>>>> }
>>>>
>>>> +static inline unsigned long busy_clock(void)
>>>> +{
>>>> + return local_clock() >> 10;
>>>> +}
>>>> +
>>>> +static bool vhost_can_busy_poll(struct vhost_dev *dev,
>>>> + unsigned long endtime)
>>>> +{
>>>> + return likely(!need_resched()) &&
>>>> + likely(!time_after(busy_clock(), endtime)) &&
>>>> + likely(!signal_pending(current)) &&
>>>> + !vhost_has_work(dev) &&
>>>> + single_task_running();
>>>> +}
>>>> +
>>>> +static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
>>>> + struct vhost_virtqueue *vq,
>>>> + struct iovec iov[], unsigned int iov_size,
>>>> + unsigned int *out_num, unsigned int *in_num)
>>>> +{
>>>> + unsigned long uninitialized_var(endtime);
>>>> +
>>>> + if (vq->busyloop_timeout) {
>>>> + preempt_disable();
>>>> + endtime = busy_clock() + vq->busyloop_timeout;
>>>> + while (vhost_can_busy_poll(vq->dev, endtime) &&
>>>> + !vhost_vq_more_avail(vq->dev, vq))
>>>> + cpu_relax();
>>>> + preempt_enable();
>>>> + }
>>>
>>> Isn't there a way to call all this after vhost_get_vq_desc?
>>> First, this will reduce the good path overhead as you
>>> won't have to play with timers and preemption.
>>>
>>> Second, this will reduce the chance of a pagefault on avail ring read.
>>>
>>>> +
>>>> + return vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
>>>> + out_num, in_num, NULL, NULL);
>>>> +}
>>>> +
>>>> /* Expects to be always run from workqueue - which acts as
>>>> * read-size critical section for our kind of RCU. */
>>>> static void handle_tx(struct vhost_net *net)
>>>> @@ -331,10 +366,9 @@ static void handle_tx(struct vhost_net *net)
>>>> % UIO_MAXIOV == nvq->done_idx))
>>>> break;
>>>>
>>>> - head = vhost_get_vq_desc(vq, vq->iov,
>>>> - ARRAY_SIZE(vq->iov),
>>>> - &out, &in,
>>>> - NULL, NULL);
>>>> + head = vhost_net_tx_get_vq_desc(net, vq, vq->iov,
>>>> + ARRAY_SIZE(vq->iov),
>>>> + &out, &in);
>>>> /* On error, stop handling until the next kick. */
>>>> if (unlikely(head < 0))
>>>> break;
>>>> @@ -435,6 +469,34 @@ static int peek_head_len(struct sock *sk)
>>>> return len;
>>>> }
>>>>
>>>> +static int vhost_net_peek_head_len(struct vhost_net *net, struct sock *sk)
>>>
>>> Need a hint that it's rx related in the name.
>>>
>>>> +{
>>>> + struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
>>>> + struct vhost_virtqueue *vq = &nvq->vq;
>>>> + unsigned long uninitialized_var(endtime);
>>>> +
>>>> + if (vq->busyloop_timeout) {
>>>> + mutex_lock(&vq->mutex);
>>>
>>> This appears to be called under vq mutex in handle_rx.
>>> So how does this work then?
>>>
>>>
>>>> + vhost_disable_notify(&net->dev, vq);
>>>
>>> This appears to be called after disable notify
>>> in handle_rx - so why disable here again?
>>>
>>>> +
>>>> + preempt_disable();
>>>> + endtime = busy_clock() + vq->busyloop_timeout;
>>>> +
>>>> + while (vhost_can_busy_poll(&net->dev, endtime) &&
>>>> + skb_queue_empty(&sk->sk_receive_queue) &&
>>>> + !vhost_vq_more_avail(&net->dev, vq))
>>>> + cpu_relax();
>>>
>>> This seems to mix in several items.
>>> RX queue is normally not empty. I don't think
>>> we need to poll for that.
>>
>> I have seen the RX queue is easy to be empty under some extreme conditions
>> like lots of small packet. So maybe the check is useful here.
>
> It's not useful *here*.
> If you have an rx packet but no space in the ring,
> this will exit immediately.
Indeed!
>
> It might be useful elsewhere but I doubt it -
> if rx ring is out of buffers, you are better off
> backing out and giving guest some breathing space.
>
>> --
>> best regards
>> yang
--
best regards
yang
^ permalink raw reply
* Re: [PATCH V2 3/3] vhost_net: basic polling support
From: Michael S. Tsirkin @ 2016-01-21 5:13 UTC (permalink / raw)
To: Yang Zhang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <56A03E57.2020400@gmail.com>
On Thu, Jan 21, 2016 at 10:11:35AM +0800, Yang Zhang wrote:
> On 2016/1/20 22:35, Michael S. Tsirkin wrote:
> >On Tue, Dec 01, 2015 at 02:39:45PM +0800, Jason Wang wrote:
> >>This patch tries to poll for new added tx buffer or socket receive
> >>queue for a while at the end of tx/rx processing. The maximum time
> >>spent on polling were specified through a new kind of vring ioctl.
> >>
> >>Signed-off-by: Jason Wang <jasowang@redhat.com>
> >>---
> >> drivers/vhost/net.c | 72 ++++++++++++++++++++++++++++++++++++++++++----
> >> drivers/vhost/vhost.c | 15 ++++++++++
> >> drivers/vhost/vhost.h | 1 +
> >> include/uapi/linux/vhost.h | 11 +++++++
> >> 4 files changed, 94 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> >>index 9eda69e..ce6da77 100644
> >>--- a/drivers/vhost/net.c
> >>+++ b/drivers/vhost/net.c
> >>@@ -287,6 +287,41 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> >> rcu_read_unlock_bh();
> >> }
> >>
> >>+static inline unsigned long busy_clock(void)
> >>+{
> >>+ return local_clock() >> 10;
> >>+}
> >>+
> >>+static bool vhost_can_busy_poll(struct vhost_dev *dev,
> >>+ unsigned long endtime)
> >>+{
> >>+ return likely(!need_resched()) &&
> >>+ likely(!time_after(busy_clock(), endtime)) &&
> >>+ likely(!signal_pending(current)) &&
> >>+ !vhost_has_work(dev) &&
> >>+ single_task_running();
> >>+}
> >>+
> >>+static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> >>+ struct vhost_virtqueue *vq,
> >>+ struct iovec iov[], unsigned int iov_size,
> >>+ unsigned int *out_num, unsigned int *in_num)
> >>+{
> >>+ unsigned long uninitialized_var(endtime);
> >>+
> >>+ if (vq->busyloop_timeout) {
> >>+ preempt_disable();
> >>+ endtime = busy_clock() + vq->busyloop_timeout;
> >>+ while (vhost_can_busy_poll(vq->dev, endtime) &&
> >>+ !vhost_vq_more_avail(vq->dev, vq))
> >>+ cpu_relax();
> >>+ preempt_enable();
> >>+ }
> >
> >Isn't there a way to call all this after vhost_get_vq_desc?
> >First, this will reduce the good path overhead as you
> >won't have to play with timers and preemption.
> >
> >Second, this will reduce the chance of a pagefault on avail ring read.
> >
> >>+
> >>+ return vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> >>+ out_num, in_num, NULL, NULL);
> >>+}
> >>+
> >> /* Expects to be always run from workqueue - which acts as
> >> * read-size critical section for our kind of RCU. */
> >> static void handle_tx(struct vhost_net *net)
> >>@@ -331,10 +366,9 @@ static void handle_tx(struct vhost_net *net)
> >> % UIO_MAXIOV == nvq->done_idx))
> >> break;
> >>
> >>- head = vhost_get_vq_desc(vq, vq->iov,
> >>- ARRAY_SIZE(vq->iov),
> >>- &out, &in,
> >>- NULL, NULL);
> >>+ head = vhost_net_tx_get_vq_desc(net, vq, vq->iov,
> >>+ ARRAY_SIZE(vq->iov),
> >>+ &out, &in);
> >> /* On error, stop handling until the next kick. */
> >> if (unlikely(head < 0))
> >> break;
> >>@@ -435,6 +469,34 @@ static int peek_head_len(struct sock *sk)
> >> return len;
> >> }
> >>
> >>+static int vhost_net_peek_head_len(struct vhost_net *net, struct sock *sk)
> >
> >Need a hint that it's rx related in the name.
> >
> >>+{
> >>+ struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> >>+ struct vhost_virtqueue *vq = &nvq->vq;
> >>+ unsigned long uninitialized_var(endtime);
> >>+
> >>+ if (vq->busyloop_timeout) {
> >>+ mutex_lock(&vq->mutex);
> >
> >This appears to be called under vq mutex in handle_rx.
> >So how does this work then?
> >
> >
> >>+ vhost_disable_notify(&net->dev, vq);
> >
> >This appears to be called after disable notify
> >in handle_rx - so why disable here again?
> >
> >>+
> >>+ preempt_disable();
> >>+ endtime = busy_clock() + vq->busyloop_timeout;
> >>+
> >>+ while (vhost_can_busy_poll(&net->dev, endtime) &&
> >>+ skb_queue_empty(&sk->sk_receive_queue) &&
> >>+ !vhost_vq_more_avail(&net->dev, vq))
> >>+ cpu_relax();
> >
> >This seems to mix in several items.
> >RX queue is normally not empty. I don't think
> >we need to poll for that.
>
> I have seen the RX queue is easy to be empty under some extreme conditions
> like lots of small packet. So maybe the check is useful here.
It's not useful *here*.
If you have an rx packet but no space in the ring,
this will exit immediately.
It might be useful elsewhere but I doubt it -
if rx ring is out of buffers, you are better off
backing out and giving guest some breathing space.
> --
> best regards
> yang
^ permalink raw reply
* Re: [PATCH V2 3/3] vhost_net: basic polling support
From: Yang Zhang @ 2016-01-21 2:11 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20160120143524.GA27168@redhat.com>
On 2016/1/20 22:35, Michael S. Tsirkin wrote:
> On Tue, Dec 01, 2015 at 02:39:45PM +0800, Jason Wang wrote:
>> This patch tries to poll for new added tx buffer or socket receive
>> queue for a while at the end of tx/rx processing. The maximum time
>> spent on polling were specified through a new kind of vring ioctl.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/vhost/net.c | 72 ++++++++++++++++++++++++++++++++++++++++++----
>> drivers/vhost/vhost.c | 15 ++++++++++
>> drivers/vhost/vhost.h | 1 +
>> include/uapi/linux/vhost.h | 11 +++++++
>> 4 files changed, 94 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index 9eda69e..ce6da77 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -287,6 +287,41 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
>> rcu_read_unlock_bh();
>> }
>>
>> +static inline unsigned long busy_clock(void)
>> +{
>> + return local_clock() >> 10;
>> +}
>> +
>> +static bool vhost_can_busy_poll(struct vhost_dev *dev,
>> + unsigned long endtime)
>> +{
>> + return likely(!need_resched()) &&
>> + likely(!time_after(busy_clock(), endtime)) &&
>> + likely(!signal_pending(current)) &&
>> + !vhost_has_work(dev) &&
>> + single_task_running();
>> +}
>> +
>> +static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
>> + struct vhost_virtqueue *vq,
>> + struct iovec iov[], unsigned int iov_size,
>> + unsigned int *out_num, unsigned int *in_num)
>> +{
>> + unsigned long uninitialized_var(endtime);
>> +
>> + if (vq->busyloop_timeout) {
>> + preempt_disable();
>> + endtime = busy_clock() + vq->busyloop_timeout;
>> + while (vhost_can_busy_poll(vq->dev, endtime) &&
>> + !vhost_vq_more_avail(vq->dev, vq))
>> + cpu_relax();
>> + preempt_enable();
>> + }
>
> Isn't there a way to call all this after vhost_get_vq_desc?
> First, this will reduce the good path overhead as you
> won't have to play with timers and preemption.
>
> Second, this will reduce the chance of a pagefault on avail ring read.
>
>> +
>> + return vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
>> + out_num, in_num, NULL, NULL);
>> +}
>> +
>> /* Expects to be always run from workqueue - which acts as
>> * read-size critical section for our kind of RCU. */
>> static void handle_tx(struct vhost_net *net)
>> @@ -331,10 +366,9 @@ static void handle_tx(struct vhost_net *net)
>> % UIO_MAXIOV == nvq->done_idx))
>> break;
>>
>> - head = vhost_get_vq_desc(vq, vq->iov,
>> - ARRAY_SIZE(vq->iov),
>> - &out, &in,
>> - NULL, NULL);
>> + head = vhost_net_tx_get_vq_desc(net, vq, vq->iov,
>> + ARRAY_SIZE(vq->iov),
>> + &out, &in);
>> /* On error, stop handling until the next kick. */
>> if (unlikely(head < 0))
>> break;
>> @@ -435,6 +469,34 @@ static int peek_head_len(struct sock *sk)
>> return len;
>> }
>>
>> +static int vhost_net_peek_head_len(struct vhost_net *net, struct sock *sk)
>
> Need a hint that it's rx related in the name.
>
>> +{
>> + struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
>> + struct vhost_virtqueue *vq = &nvq->vq;
>> + unsigned long uninitialized_var(endtime);
>> +
>> + if (vq->busyloop_timeout) {
>> + mutex_lock(&vq->mutex);
>
> This appears to be called under vq mutex in handle_rx.
> So how does this work then?
>
>
>> + vhost_disable_notify(&net->dev, vq);
>
> This appears to be called after disable notify
> in handle_rx - so why disable here again?
>
>> +
>> + preempt_disable();
>> + endtime = busy_clock() + vq->busyloop_timeout;
>> +
>> + while (vhost_can_busy_poll(&net->dev, endtime) &&
>> + skb_queue_empty(&sk->sk_receive_queue) &&
>> + !vhost_vq_more_avail(&net->dev, vq))
>> + cpu_relax();
>
> This seems to mix in several items.
> RX queue is normally not empty. I don't think
> we need to poll for that.
I have seen the RX queue is easy to be empty under some extreme
conditions like lots of small packet. So maybe the check is useful here.
--
best regards
yang
^ permalink raw reply
* [PATCH] tools/virtio: use virt_xxx barriers
From: Michael S. Tsirkin @ 2016-01-20 19:14 UTC (permalink / raw)
To: linux-kernel; +Cc: Kamal Mostafa, virtualization
Fix build after API changes.
Reported-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tools/virtio/asm/barrier.h | 22 +++++++++++++---------
tools/virtio/linux/compiler.h | 9 +++++++++
tools/virtio/linux/kernel.h | 1 +
3 files changed, 23 insertions(+), 9 deletions(-)
create mode 100644 tools/virtio/linux/compiler.h
diff --git a/tools/virtio/asm/barrier.h b/tools/virtio/asm/barrier.h
index 26b7926..ba34f9e 100644
--- a/tools/virtio/asm/barrier.h
+++ b/tools/virtio/asm/barrier.h
@@ -1,15 +1,19 @@
#if defined(__i386__) || defined(__x86_64__)
#define barrier() asm volatile("" ::: "memory")
-#define mb() __sync_synchronize()
-
-#define smp_mb() mb()
-# define dma_rmb() barrier()
-# define dma_wmb() barrier()
-# define smp_rmb() barrier()
-# define smp_wmb() barrier()
+#define virt_mb() __sync_synchronize()
+#define virt_rmb() barrier()
+#define virt_wmb() barrier()
+/* Atomic store should be enough, but gcc generates worse code in that case. */
+#define virt_store_mb(var, value) do { \
+ typeof(var) virt_store_mb_value = (value); \
+ __atomic_exchange(&(var), &virt_store_mb_value, &virt_store_mb_value, \
+ __ATOMIC_SEQ_CST); \
+ barrier(); \
+} while (0);
/* Weak barriers should be used. If not - it's a bug */
-# define rmb() abort()
-# define wmb() abort()
+# define mb() abort()
+# define rmb() abort()
+# define wmb() abort()
#else
#error Please fill in barrier macros
#endif
diff --git a/tools/virtio/linux/compiler.h b/tools/virtio/linux/compiler.h
new file mode 100644
index 0000000..845960e
--- /dev/null
+++ b/tools/virtio/linux/compiler.h
@@ -0,0 +1,9 @@
+#ifndef LINUX_COMPILER_H
+#define LINUX_COMPILER_H
+
+#define WRITE_ONCE(var, val) \
+ (*((volatile typeof(val) *)(&(var))) = (val))
+
+#define READ_ONCE(var) (*((volatile typeof(val) *)(&(var))))
+
+#endif
diff --git a/tools/virtio/linux/kernel.h b/tools/virtio/linux/kernel.h
index 4db7d56..0338499 100644
--- a/tools/virtio/linux/kernel.h
+++ b/tools/virtio/linux/kernel.h
@@ -8,6 +8,7 @@
#include <assert.h>
#include <stdarg.h>
+#include <linux/compiler.h>
#include <linux/types.h>
#include <linux/printk.h>
#include <linux/bug.h>
--
MST
^ permalink raw reply related
* CISTI'2016 - 11th Iberian Conference on Information Systems and Technologies
From: Maria Lemos @ 2016-01-20 16:05 UTC (permalink / raw)
To: virtualization
[-- Attachment #1: Type: text/plain, Size: 4911 bytes --]
-
---
CISTI'2016 - 11th Iberian Conference on Information Systems and Technologies
June 15 18, 2016, Gran Canaria, Canary Islands, Spain
http://www.aisti.eu/cisti2016/index.php/en
------------------------------------------------------------------------------------
We are pleased to invite the academic and business community to submit their papers to CISTI'2016 - 11th Iberian Conference on Information Systems and Technologies, to be held in Gran Canaria, Canary Islands, Spain, between the 15th and 18th of June 2016. Authors are encouraged to submit original scientific contributions such as state-of-art reviews and new research perspectives, groundbreaking ideas and/or architectures, solutions and/or applications for real problems, empirical and/or evaluation works, case studies, etc., in conformity with the themes of this Conference.
Four types of papers can be submitted:
Full paper: Finished or consolidated R&D works, to be included in one of the Conference themes. These papers are assigned a 6-page limit.
Short paper: Ongoing works with relevant preliminary results, opened to discussion. These papers are assigned a 4-page limit.
Poster paper: Initial work with relevant ideas, opened to discussion. These papers are assigned a 2-page limit.
Company paper: Companies' papers that show practical experience, R & D, tools, etc., focused in some topics of the conference. These articles are abstracts with a maximum of 2 pages.
Papers submitted for the Scientific Committee’s evaluation must not include any information leading to the authors’ identification. Therefore, the authors’ names, affiliations and bibliographic references should not be included in the early version. This information should only be included in the final version.
Submitted papers must not have been published and must not be under review for any other conference and national or international publication.
Papers must comply with the format standard and be written in Portuguese, Spanish or English.
All papers will be subjected to a “blind review” by at least two members of the Scientific Committee.
Full papers can be accepted as short papers or poster papers only. Similarly, short papers can be accepted as poster papers only. In these two cases, the authors will be allowed to maintain the original number of pages in the proceedings publication.
The authors of accepted poster papers must also build and print a poster to be exhibited during the Conference. This poster must follow an A1 or A2 vertical format. The Conference includes Work Sessions where these posters are presented and orally discussed, with a 5-minute limit per poster.
The authors of accepted full papers will dispose of a 15-minute presentation in the Conference Work Session, and approximately 5 minutes of discussion will follow each presentation. The authors of accepted short papers and company papers will dispose of an 11-minute presentation in the Conference Work Session, and approximately 4 minutes of discussion will follow each presentation.
THEMES
Submitted papers must follow the main themes proposed for the Conference (the topics proposed in each theme constitute a mere framework reference; they are not intended as restrictive):
A) OMIS - Organizational Models and Information Systems
B) KMDSS - Knowledge Management and Decision Support Systems
C) SSAAT - Software Systems, Architectures, Applications and Tools
D) CNMPS - Computer Networks, Mobility and Pervasive Systems
E) HCC - Human Centered Computing
F) HIS - Health Informatics
G) ITE - Information Technologies in Education
H) AEC – Architecture and Engineering of Construction
PUBLICATION & INDEXING
To ensure that the contribution (full paper, short paper, poster paper or company paper) is published in the Proceedings, at least one of the authors must be fully registered by the 11th of April, and the paper must comply with the suggested layout and page-limit. Additionally, all recommended modifications must be addressed by the authors before they submit the final version.
No more than one paper per registration will be published in the Conference Proceedings. An extra fee must be paid for publication of additional papers, with a maximum of one additional paper per registration.
Published full and short papers will be sent to EI, IEEE XPlore, INSPEC, ISI, SCOPUS and Google Scholar. Poster papers and company papers will be sent to EBSCO and EI.
IMPORTANT DATES
Paper submission: February 14, 2016
Notification of acceptance: March 27, 2016
Submission of accepted papers: April 10, 2016
Payment of registration, to ensure the inclusion of an accepted paper in the conference proceedings: April 8, 2016
Best regards,
CISTI'2016 Team
http://www.aisti.eu/cisti2016/index.php/en
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH V2 3/3] vhost_net: basic polling support
From: Michael S. Tsirkin @ 2016-01-20 14:35 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1448951985-12385-4-git-send-email-jasowang@redhat.com>
On Tue, Dec 01, 2015 at 02:39:45PM +0800, Jason Wang wrote:
> This patch tries to poll for new added tx buffer or socket receive
> queue for a while at the end of tx/rx processing. The maximum time
> spent on polling were specified through a new kind of vring ioctl.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vhost/net.c | 72 ++++++++++++++++++++++++++++++++++++++++++----
> drivers/vhost/vhost.c | 15 ++++++++++
> drivers/vhost/vhost.h | 1 +
> include/uapi/linux/vhost.h | 11 +++++++
> 4 files changed, 94 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9eda69e..ce6da77 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -287,6 +287,41 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> rcu_read_unlock_bh();
> }
>
> +static inline unsigned long busy_clock(void)
> +{
> + return local_clock() >> 10;
> +}
> +
> +static bool vhost_can_busy_poll(struct vhost_dev *dev,
> + unsigned long endtime)
> +{
> + return likely(!need_resched()) &&
> + likely(!time_after(busy_clock(), endtime)) &&
> + likely(!signal_pending(current)) &&
> + !vhost_has_work(dev) &&
> + single_task_running();
> +}
> +
> +static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> + struct vhost_virtqueue *vq,
> + struct iovec iov[], unsigned int iov_size,
> + unsigned int *out_num, unsigned int *in_num)
> +{
> + unsigned long uninitialized_var(endtime);
> +
> + if (vq->busyloop_timeout) {
> + preempt_disable();
> + endtime = busy_clock() + vq->busyloop_timeout;
> + while (vhost_can_busy_poll(vq->dev, endtime) &&
> + !vhost_vq_more_avail(vq->dev, vq))
> + cpu_relax();
> + preempt_enable();
> + }
Isn't there a way to call all this after vhost_get_vq_desc?
First, this will reduce the good path overhead as you
won't have to play with timers and preemption.
Second, this will reduce the chance of a pagefault on avail ring read.
> +
> + return vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> + out_num, in_num, NULL, NULL);
> +}
> +
> /* Expects to be always run from workqueue - which acts as
> * read-size critical section for our kind of RCU. */
> static void handle_tx(struct vhost_net *net)
> @@ -331,10 +366,9 @@ static void handle_tx(struct vhost_net *net)
> % UIO_MAXIOV == nvq->done_idx))
> break;
>
> - head = vhost_get_vq_desc(vq, vq->iov,
> - ARRAY_SIZE(vq->iov),
> - &out, &in,
> - NULL, NULL);
> + head = vhost_net_tx_get_vq_desc(net, vq, vq->iov,
> + ARRAY_SIZE(vq->iov),
> + &out, &in);
> /* On error, stop handling until the next kick. */
> if (unlikely(head < 0))
> break;
> @@ -435,6 +469,34 @@ static int peek_head_len(struct sock *sk)
> return len;
> }
>
> +static int vhost_net_peek_head_len(struct vhost_net *net, struct sock *sk)
Need a hint that it's rx related in the name.
> +{
> + struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> + struct vhost_virtqueue *vq = &nvq->vq;
> + unsigned long uninitialized_var(endtime);
> +
> + if (vq->busyloop_timeout) {
> + mutex_lock(&vq->mutex);
This appears to be called under vq mutex in handle_rx.
So how does this work then?
> + vhost_disable_notify(&net->dev, vq);
This appears to be called after disable notify
in handle_rx - so why disable here again?
> +
> + preempt_disable();
> + endtime = busy_clock() + vq->busyloop_timeout;
> +
> + while (vhost_can_busy_poll(&net->dev, endtime) &&
> + skb_queue_empty(&sk->sk_receive_queue) &&
> + !vhost_vq_more_avail(&net->dev, vq))
> + cpu_relax();
This seems to mix in several items.
RX queue is normally not empty. I don't think
we need to poll for that.
So IMHO we only need to poll for sk_receive_queue really.
> +
> + preempt_enable();
> +
> + if (vhost_enable_notify(&net->dev, vq))
> + vhost_poll_queue(&vq->poll);
But vhost_enable_notify returns true on queue not empty.
So in fact this will requeue on good path -
does not make sense to me.
> + mutex_unlock(&vq->mutex);
> + }
> +
Same comment as for get vq desc here: don't slow
down the good path.
> + return peek_head_len(sk);
> +}
> +
> /* This is a multi-buffer version of vhost_get_desc, that works if
> * vq has read descriptors only.
> * @vq - the relevant virtqueue
> @@ -553,7 +615,7 @@ static void handle_rx(struct vhost_net *net)
> vq->log : NULL;
> mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);
>
> - while ((sock_len = peek_head_len(sock->sk))) {
> + while ((sock_len = vhost_net_peek_head_len(net, sock->sk))) {
> sock_len += sock_hlen;
> vhost_len = sock_len + vhost_hlen;
> headcount = get_rx_bufs(vq, vq->heads, vhost_len,
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 4f45a03..b8ca873 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -285,6 +285,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> vq->memory = NULL;
> vq->is_le = virtio_legacy_is_little_endian();
> vhost_vq_reset_user_be(vq);
> + vq->busyloop_timeout = 0;
> }
>
> static int vhost_worker(void *data)
> @@ -747,6 +748,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> struct vhost_vring_state s;
> struct vhost_vring_file f;
> struct vhost_vring_addr a;
> + struct vhost_vring_busyloop_timeout t;
> u32 idx;
> long r;
>
> @@ -919,6 +921,19 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> case VHOST_GET_VRING_ENDIAN:
> r = vhost_get_vring_endian(vq, idx, argp);
> break;
> + case VHOST_SET_VRING_BUSYLOOP_TIMEOUT:
> + if (copy_from_user(&t, argp, sizeof(t))) {
> + r = -EFAULT;
> + break;
> + }
> + vq->busyloop_timeout = t.timeout;
> + break;
> + case VHOST_GET_VRING_BUSYLOOP_TIMEOUT:
> + t.index = idx;
> + t.timeout = vq->busyloop_timeout;
> + if (copy_to_user(argp, &t, sizeof(t)))
> + r = -EFAULT;
> + break;
> default:
> r = -ENOIOCTLCMD;
> }
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 2f3c57c..4b7d4fa 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -115,6 +115,7 @@ struct vhost_virtqueue {
> /* Ring endianness requested by userspace for cross-endian support. */
> bool user_be;
> #endif
> + u32 busyloop_timeout;
> };
>
> struct vhost_dev {
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index ab373191..eaf6c33 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -27,6 +27,11 @@ struct vhost_vring_file {
>
> };
>
> +struct vhost_vring_busyloop_timeout {
> + unsigned int index;
> + unsigned int timeout;
> +};
> +
So why not reuse vhost_vring_state then?
> struct vhost_vring_addr {
> unsigned int index;
> /* Option flags. */
> @@ -126,6 +131,12 @@ struct vhost_memory {
> #define VHOST_SET_VRING_CALL _IOW(VHOST_VIRTIO, 0x21, struct vhost_vring_file)
> /* Set eventfd to signal an error */
> #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
> +/* Set busy loop timeout */
Units?
> +#define VHOST_SET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x23, \
> + struct vhost_vring_busyloop_timeout)
> +/* Get busy loop timeout */
> +#define VHOST_GET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x24, \
> + struct vhost_vring_busyloop_timeout)
>
> /* VHOST_NET specific defines */
>
> --
> 2.5.0
^ permalink raw reply
* Re: [PATCH V2 2/3] vhost: introduce vhost_vq_more_avail()
From: Michael S. Tsirkin @ 2016-01-20 14:09 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1448951985-12385-3-git-send-email-jasowang@redhat.com>
On Tue, Dec 01, 2015 at 02:39:44PM +0800, Jason Wang wrote:
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Wow new API with no comments anywhere, and no
commit log to say what it's good for.
Want to know what it does and whether
it's correct? You have to read the next patch.
So what is the point of splitting it out?
It's confusing, and in fact it made you
miss a bug.
> ---
> drivers/vhost/vhost.c | 13 +++++++++++++
> drivers/vhost/vhost.h | 1 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 163b365..4f45a03 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1633,6 +1633,19 @@ void vhost_add_used_and_signal_n(struct vhost_dev *dev,
> }
> EXPORT_SYMBOL_GPL(vhost_add_used_and_signal_n);
>
> +bool vhost_vq_more_avail(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> +{
> + __virtio16 avail_idx;
> + int r;
> +
> + r = __get_user(avail_idx, &vq->avail->idx);
> + if (r)
> + return false;
So the result is that if the page is not present,
you return false (empty ring) and the
caller will busy wait with preempt disabled.
Nasty.
So it should return something that breaks
the loop, and this means it should have
a different name for the return code
to make sense.
Maybe reverse the polarity: vhost_vq_avail_empty?
And add a comment saying we can't be sure ring
is empty so return false.
> +
> + return vhost16_to_cpu(vq, avail_idx) != vq->avail_idx;
> +}
> +EXPORT_SYMBOL_GPL(vhost_vq_more_avail);
> +
> /* OK, now we need to know about added descriptors. */
> bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> {
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 43284ad..2f3c57c 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -159,6 +159,7 @@ void vhost_add_used_and_signal_n(struct vhost_dev *, struct vhost_virtqueue *,
> struct vring_used_elem *heads, unsigned count);
> void vhost_signal(struct vhost_dev *, struct vhost_virtqueue *);
> void vhost_disable_notify(struct vhost_dev *, struct vhost_virtqueue *);
> +bool vhost_vq_more_avail(struct vhost_dev *, struct vhost_virtqueue *);
> bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>
> int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> --
> 2.5.0
^ permalink raw reply
* Re: [PATCH RFC] vhost: convert pre sorted vhost memory array to interval tree
From: Jason Wang @ 2016-01-20 2:50 UTC (permalink / raw)
To: Igor Mammedov; +Cc: netdev, virtualization, linux-kernel, kvm, mst
In-Reply-To: <20160119162406.7f0c2008@nial.brq.redhat.com>
On 01/19/2016 11:24 PM, Igor Mammedov wrote:
> On Mon, 18 Jan 2016 10:42:29 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> > Current pre-sorted memory region array has some limitations for future
>> > device IOTLB conversion:
>> >
>> > 1) need extra work for adding and removing a single region, and it's
>> > expected to be slow because of sorting or memory re-allocation.
>> > 2) need extra work of removing a large range which may intersect
>> > several regions with different size.
>> > 3) need trick for a replacement policy like LRU
>> >
>> > To overcome the above shortcomings, this patch convert it to interval
>> > tree which can easily address the above issue with almost no extra
>> > work.
>> >
>> > The patch could be used for:
>> >
>> > - Extend the current API and only let the userspace to send diffs of
>> > memory table.
>> > - Simplify Device IOTLB implementation.
> I'm curios how performance changes on translate_desc() hot-path in
> default case and in the case of 256 memory regions?
>
Haven't measured this. But consider both methods are O(logN), should be
no noticeable difference. Will measure it in the future.
The main user is for the device IOTLB API[1], which:
- have thousands of userspace memory region sections
- may dynamically adding or removing one or some of the regions
- need a replacement algorithm like LRU
[1] https://lkml.org/lkml/2015/12/31/16
^ permalink raw reply
* [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
From: Sinclair Yeh @ 2016-01-19 21:46 UTC (permalink / raw)
To: x86
Cc: Sinclair Yeh, pv-drivers, gregkh, linux-kernel, virtualization,
Xavier Deguillard
In-Reply-To: <1453239965-1466-1-git-send-email-syeh@vmware.com>
Updated VMWARE_BALLOON_CMD to use the common VMW_PORT macro.
Doing this rather than replacing all instances of VMWARE_BALLOON_CMD
to minimize code change.
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Acked-by: Xavier Deguillard <xdeguillard@vmware.com>
Cc: pv-drivers@vmware.com
Cc: Xavier Deguillard <xdeguillard@vmware.com>
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v1
Swapped parameters 1 and 2 to VMW_PORT because the macro has been
updated
v2
Updated VMW_PORT() usage because the macro and vmw_balloon.c's
usage have changed.
---
drivers/misc/vmw_balloon.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index 1e688bf..6476b74 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -46,6 +46,7 @@
#include <linux/vmw_vmci_defs.h>
#include <linux/vmw_vmci_api.h>
#include <asm/hypervisor.h>
+#include <asm/vmware.h>
MODULE_AUTHOR("VMware, Inc.");
MODULE_DESCRIPTION("VMware Memory Control (Balloon) Driver");
@@ -197,25 +198,17 @@ static void vmballoon_batch_set_pa(struct vmballoon_batch_page *batch, int idx,
}
-#define VMWARE_BALLOON_CMD(cmd, arg1, arg2, result) \
-({ \
- unsigned long __status, __dummy1, __dummy2, __dummy3; \
- __asm__ __volatile__ ("inl %%dx" : \
- "=a"(__status), \
- "=c"(__dummy1), \
- "=d"(__dummy2), \
- "=b"(result), \
- "=S" (__dummy3) : \
- "0"(VMW_BALLOON_HV_MAGIC), \
- "1"(VMW_BALLOON_CMD_##cmd), \
- "2"(VMW_BALLOON_HV_PORT), \
- "3"(arg1), \
- "4" (arg2) : \
- "memory"); \
- if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START) \
- result = __dummy1; \
- result &= -1UL; \
- __status & -1UL; \
+#define VMWARE_BALLOON_CMD(cmd, arg1, arg2, result) \
+({ \
+ unsigned long __status, __dummy1, __dummy2; \
+ unsigned long __si, __di; \
+ VMW_PORT(VMW_BALLOON_CMD_##cmd, arg1, arg2, 0, \
+ VMW_BALLOON_HV_PORT, VMW_BALLOON_HV_MAGIC, \
+ __status, result, __dummy1, __dummy2, __si, __di); \
+ if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START) \
+ result = __dummy1; \
+ result &= -1UL; \
+ __status & -1UL; \
})
#ifdef CONFIG_DEBUG_FS
--
1.9.1
^ permalink raw reply related
* [PATCH 4/6] Input: Remove vmmouse port reservation
From: Sinclair Yeh @ 2016-01-19 21:46 UTC (permalink / raw)
To: x86
Cc: Sinclair Yeh, pv-drivers, gregkh, linux-kernel, virtualization,
linux-graphics-maintainer
In-Reply-To: <1453239965-1466-1-git-send-email-syeh@vmware.com>
This port is used by quite a few guest-to-host communication
capabilities, e.g. getting configuration, logging, etc. Currently
multiple kernel modules, and one or more priviledged guest user
mode app, e.g. open-vm-tools, use this port without reservation.
It was determined that no reservation is required when accessing
the port in this manner.
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: pv-drivers@vmware.com
Cc: linux-graphics-maintainer@vmware.com
Cc: virtualization@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/input/mouse/vmmouse.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index d06daf6..85fb3d47 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -347,16 +347,10 @@ int vmmouse_detect(struct psmouse *psmouse, bool set_properties)
return -ENXIO;
}
- if (!request_region(VMMOUSE_PROTO_PORT, 4, "vmmouse")) {
- psmouse_dbg(psmouse, "VMMouse port in use.\n");
- return -EBUSY;
- }
-
/* Check if the device is present */
response = ~VMMOUSE_PROTO_MAGIC;
VMMOUSE_CMD(GETVERSION, 0, version, response, dummy1, dummy2);
if (response != VMMOUSE_PROTO_MAGIC || version == 0xffffffffU) {
- release_region(VMMOUSE_PROTO_PORT, 4);
return -ENXIO;
}
@@ -366,8 +360,6 @@ int vmmouse_detect(struct psmouse *psmouse, bool set_properties)
psmouse->model = version;
}
- release_region(VMMOUSE_PROTO_PORT, 4);
-
return 0;
}
@@ -386,7 +378,6 @@ static void vmmouse_disconnect(struct psmouse *psmouse)
psmouse_reset(psmouse);
input_unregister_device(priv->abs_dev);
kfree(priv);
- release_region(VMMOUSE_PROTO_PORT, 4);
}
/**
@@ -430,15 +421,10 @@ int vmmouse_init(struct psmouse *psmouse)
struct input_dev *rel_dev = psmouse->dev, *abs_dev;
int error;
- if (!request_region(VMMOUSE_PROTO_PORT, 4, "vmmouse")) {
- psmouse_dbg(psmouse, "VMMouse port in use.\n");
- return -EBUSY;
- }
-
psmouse_reset(psmouse);
error = vmmouse_enable(psmouse);
if (error)
- goto release_region;
+ return error;
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
abs_dev = input_allocate_device();
@@ -493,8 +479,5 @@ init_fail:
kfree(priv);
psmouse->private = NULL;
-release_region:
- release_region(VMMOUSE_PROTO_PORT, 4);
-
return error;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
From: Sinclair Yeh @ 2016-01-19 21:46 UTC (permalink / raw)
To: x86
Cc: Arnd Bergmann, Sinclair Yeh, pv-drivers, gregkh, Dmitry Torokhov,
linux-kernel, virtualization, linux-graphics-maintainer,
linux-input
In-Reply-To: <1453239965-1466-1-git-send-email-syeh@vmware.com>
Updated the VMMOUSE macro to use the new VMW_PORT macro. Doing
this instead of replacing all existing instances of VMWMOUSE
to minimize code change.
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: pv-drivers@vmware.com
Cc: linux-graphics-maintainer@vmware.com
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: linux-input@vger.kernel.org
---
v2:
Instead of replacing existing VMMOUSE defines, only modify enough
to use the new VMW_PORT define.
v3:
Use updated VMWARE_PORT() which requires hypervisor magic as an added
parameter
v4:
Swapped parameters 1 and 2 when calling VMW_PORT because the macro
has been updated
v5:
Updated VMW_PORT() usage since the macro has been updated
---
drivers/input/mouse/vmmouse.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index e272f06..d06daf6 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -19,6 +19,7 @@
#include <linux/slab.h>
#include <linux/module.h>
#include <asm/hypervisor.h>
+#include <asm/vmware.h>
#include "psmouse.h"
#include "vmmouse.h"
@@ -84,21 +85,12 @@ struct vmmouse_data {
* implementing the vmmouse protocol. Should never execute on
* bare metal hardware.
*/
-#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
-({ \
- unsigned long __dummy1, __dummy2; \
- __asm__ __volatile__ ("inl %%dx" : \
- "=a"(out1), \
- "=b"(out2), \
- "=c"(out3), \
- "=d"(out4), \
- "=S"(__dummy1), \
- "=D"(__dummy2) : \
- "a"(VMMOUSE_PROTO_MAGIC), \
- "b"(in1), \
- "c"(VMMOUSE_PROTO_CMD_##cmd), \
- "d"(VMMOUSE_PROTO_PORT) : \
- "memory"); \
+#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
+({ \
+ unsigned long __dummy1, __dummy2; \
+ VMW_PORT(VMMOUSE_PROTO_CMD_##cmd, in1, 0, 0, \
+ VMMOUSE_PROTO_PORT, VMMOUSE_PROTO_MAGIC, \
+ out1, out2, out3, out4, __dummy1, __dummy2); \
})
/**
--
1.9.1
^ permalink raw reply related
* [PATCH 2/6] x86: Update vmware.c to use the common VMW_PORT macros
From: Sinclair Yeh @ 2016-01-19 21:46 UTC (permalink / raw)
To: x86
Cc: Sinclair Yeh, pv-drivers, gregkh, linux-kernel, virtualization,
Ingo Molnar, H. Peter Anvin, Thomas Gleixner
In-Reply-To: <1453239965-1466-1-git-send-email-syeh@vmware.com>
Updated the VMWARE_PORT macro to use the new VMW_PORT
macro. Doing this instead of replacing all existing
instances of VMWARE_PORT to minimize code change.
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: pv-drivers@vmware.com
Cc: virtualization@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2:
Instead of replacing all existing instances of VMWARE_PORT
with VMW_PORT, update VMWARE_PORT to use the new VMW_PORT.
v3:
Using updated VMWARE_PORT() macro, which needs hypervisor magic in the
parameter
v4:
Swapped the first and second parameters because VMW_PORT has
changed.
v5:
si and di are now separate input/output arguments in VMW_PORT()
---
arch/x86/kernel/cpu/vmware.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 628a059..8e900d3 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -26,6 +26,7 @@
#include <asm/div64.h>
#include <asm/x86_init.h>
#include <asm/hypervisor.h>
+#include <asm/vmware.h>
#define CPUID_VMWARE_INFO_LEAF 0x40000000
#define VMWARE_HYPERVISOR_MAGIC 0x564D5868
@@ -38,12 +39,13 @@
#define VMWARE_PORT_CMD_VCPU_RESERVED 31
#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
- __asm__("inl (%%dx)" : \
- "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
- "0"(VMWARE_HYPERVISOR_MAGIC), \
- "1"(VMWARE_PORT_CMD_##cmd), \
- "2"(VMWARE_HYPERVISOR_PORT), "3"(UINT_MAX) : \
- "memory");
+({ \
+ unsigned long __si, __di; /* Not used */ \
+ VMW_PORT(VMWARE_PORT_CMD_##cmd, UINT_MAX, 0, 0, \
+ VMWARE_HYPERVISOR_PORT, VMWARE_HYPERVISOR_MAGIC, \
+ eax, ebx, ecx, edx, __si, __di); \
+})
+
static inline int __vmware_platform(void)
{
--
1.9.1
^ permalink raw reply related
* [PATCH 1/6] x86: Add VMWare Host Communication Macros
From: Sinclair Yeh @ 2016-01-19 21:46 UTC (permalink / raw)
To: x86
Cc: Sinclair Yeh, Xavier Deguillard, gregkh, linux-kernel,
virtualization, Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
Alok Kataria
In-Reply-To: <1453239965-1466-1-git-send-email-syeh@vmware.com>
These macros will be used by multiple VMWare modules for handling
host communication.
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Alok Kataria <akataria@vmware.com>
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: Xavier Deguillard <xdeguillard@vmware.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2:
* Keeping only the minimal common platform defines
* added vmware_platform() check function
v3:
* Added new field to handle different hypervisor magic values
v4:
* Make it so that "cmd" is always the first parameter for both
High-Bandwidh and non-High-Bandwidth version of the macro
* Addressed comments from H. Peter Anvin on the assembly code
v5:
* Separate SI/DI as independent input/output arguments, given
the latest update to vmw_balloon.c
---
arch/x86/include/asm/vmware.h | 138 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 138 insertions(+)
create mode 100644 arch/x86/include/asm/vmware.h
diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
new file mode 100644
index 0000000..7d02a2b
--- /dev/null
+++ b/arch/x86/include/asm/vmware.h
@@ -0,0 +1,138 @@
+/*
+ * Copyright (C) 2015, VMware, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT. See the GNU General Public License for more
+ * details.
+ *
+ * Based on code from vmware.c and vmmouse.c.
+ * Author:
+ * Sinclair Yeh <syeh@vmware.com>
+ */
+#ifndef _ASM_X86_VMWARE_H
+#define _ASM_X86_VMWARE_H
+
+
+/**
+ * Hypervisor-specific bi-directional communication channel. Should never
+ * execute on bare metal hardware. The caller must make sure to check for
+ * supported hypervisor before using these macros.
+ *
+ * The last two parameters are both input and output and must be initialized.
+ *
+ * @cmd: [IN] Message Cmd
+ * @in_ebx: [IN] Message Len, through EBX
+ * @in_si: [IN] Input argument through SI, set to 0 if not used
+ * @in_di: [IN] Input argument through DI, set ot 0 if not used
+ * @port_num: [IN] port number + [channel id]
+ * @magic: [IN] hypervisor magic value
+ * @eax: [OUT] value of EAX register
+ * @ebx: [OUT] e.g. status from an HB message status command
+ * @ecx: [OUT] e.g. status from a non-HB message status command
+ * @edx: [OUT] e.g. channel id
+ * @si: [OUT]
+ * @di: [OUT]
+ */
+#define VMW_PORT(cmd, in_ebx, in_si, in_di, \
+ port_num, magic, \
+ eax, ebx, ecx, edx, si, di) \
+({ \
+ asm volatile ("inl %%dx, %%eax;" : \
+ "=a"(eax), \
+ "=b"(ebx), \
+ "=c"(ecx), \
+ "=d"(edx), \
+ "=S"(si), \
+ "=D"(di) : \
+ "a"(magic), \
+ "b"(in_ebx), \
+ "c"(cmd), \
+ "d"(port_num), \
+ "S"(in_si), \
+ "D"(in_di) : \
+ "memory"); \
+})
+
+
+/**
+ * Hypervisor-specific bi-directional communication channel. Should never
+ * execute on bare metal hardware. The caller must make sure to check for
+ * supported hypervisor before using these macros.
+ *
+ * The last 3 parameters are both input and output and must be initialized.
+ *
+ * @cmd: [IN] Message Cmd
+ * @in_ecx: [IN] Message Len, through ECX
+ * @in_si: [IN] Input argument through SI, set to 0 if not used
+ * @in_di: [IN] Input argument through DI, set to 0 if not used
+ * @port_num: [IN] port number + [channel id]
+ * @magic: [IN] hypervisor magic value
+ * @eax: [OUT] value of EAX register
+ * @ebx: [OUT] e.g. status from an HB message status command
+ * @ecx: [OUT] e.g. status from a non-HB message status command
+ * @edx: [OUT] e.g. channel id
+ * @si: [OUT]
+ * @di: [OUT]
+ * @bp: [INOUT] set to 0 if not used
+ */
+#define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di, \
+ port_num, magic, \
+ eax, ebx, ecx, edx, si, di, bp) \
+({ \
+ asm volatile ("push %%rbp;" \
+ "xchgq %6, %%rbp;" \
+ "rep outsb;" \
+ "xchgq %%rbp, %6;" \
+ "pop %%rbp;" : \
+ "=a"(eax), \
+ "=b"(ebx), \
+ "=c"(ecx), \
+ "=d"(edx), \
+ "=S"(si), \
+ "=D"(di), \
+ "+r"(bp) : \
+ "a"(magic), \
+ "b"(cmd), \
+ "c"(in_ecx), \
+ "d"(port_num), \
+ "S"(in_si), \
+ "D"(in_di) : \
+ "memory", "cc"); \
+})
+
+
+#define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di, \
+ port_num, magic, \
+ eax, ebx, ecx, edx, si, di, bp) \
+({ \
+ asm volatile ("push %%rbp;" \
+ "xchgq %6, %%rbp;" \
+ "rep insb;" \
+ "xchgq %%rbp, %6;" \
+ "pop %%rbp" : \
+ "=a"(eax), \
+ "=b"(ebx), \
+ "=c"(ecx), \
+ "=d"(edx), \
+ "=S"(si), \
+ "=D"(di), \
+ "+r"(bp) : \
+ "a"(magic), \
+ "b"(cmd), \
+ "c"(in_ecx), \
+ "d"(port_num), \
+ "S"(in_si), \
+ "D"(in_di) : \
+ "memory", "cc"); \
+})
+
+
+#endif
+
--
1.9.1
^ permalink raw reply related
* Re: [PATCH RFC] vhost: convert pre sorted vhost memory array to interval tree
From: Igor Mammedov @ 2016-01-19 15:24 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, virtualization, linux-kernel, kvm, mst
In-Reply-To: <1453084949-4896-1-git-send-email-jasowang@redhat.com>
On Mon, 18 Jan 2016 10:42:29 +0800
Jason Wang <jasowang@redhat.com> wrote:
> Current pre-sorted memory region array has some limitations for future
> device IOTLB conversion:
>
> 1) need extra work for adding and removing a single region, and it's
> expected to be slow because of sorting or memory re-allocation.
> 2) need extra work of removing a large range which may intersect
> several regions with different size.
> 3) need trick for a replacement policy like LRU
>
> To overcome the above shortcomings, this patch convert it to interval
> tree which can easily address the above issue with almost no extra
> work.
>
> The patch could be used for:
>
> - Extend the current API and only let the userspace to send diffs of
> memory table.
> - Simplify Device IOTLB implementation.
I'm curios how performance changes on translate_desc() hot-path in
default case and in the case of 256 memory regions?
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vhost/net.c | 8 +--
> drivers/vhost/vhost.c | 179 +++++++++++++++++++++++++++-----------------------
> drivers/vhost/vhost.h | 27 ++++++--
> 3 files changed, 125 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9eda69e..481db96 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -968,20 +968,20 @@ static long vhost_net_reset_owner(struct vhost_net *n)
> struct socket *tx_sock = NULL;
> struct socket *rx_sock = NULL;
> long err;
> - struct vhost_memory *memory;
> + struct vhost_umem *umem;
>
> mutex_lock(&n->dev.mutex);
> err = vhost_dev_check_owner(&n->dev);
> if (err)
> goto done;
> - memory = vhost_dev_reset_owner_prepare();
> - if (!memory) {
> + umem = vhost_dev_reset_owner_prepare();
> + if (!umem) {
> err = -ENOMEM;
> goto done;
> }
> vhost_net_stop(n, &tx_sock, &rx_sock);
> vhost_net_flush(n);
> - vhost_dev_reset_owner(&n->dev, memory);
> + vhost_dev_reset_owner(&n->dev, umem);
> vhost_net_vq_reset(n);
> done:
> mutex_unlock(&n->dev.mutex);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index ad2146a..851dce8 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -27,6 +27,7 @@
> #include <linux/cgroup.h>
> #include <linux/module.h>
> #include <linux/sort.h>
> +#include <linux/interval_tree_generic.h>
>
> #include "vhost.h"
>
> @@ -42,6 +43,10 @@ enum {
> #define vhost_used_event(vq) ((__virtio16 __user *)&vq->avail->ring[vq->num])
> #define vhost_avail_event(vq) ((__virtio16 __user *)&vq->used->ring[vq->num])
>
> +INTERVAL_TREE_DEFINE(struct vhost_umem_node,
> + rb, __u64, __subtree_last,
> + START, LAST, , vhost_umem_interval_tree);
> +
> #ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY
> static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq)
> {
> @@ -275,7 +280,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> vq->call_ctx = NULL;
> vq->call = NULL;
> vq->log_ctx = NULL;
> - vq->memory = NULL;
> + vq->umem = NULL;
> vq->is_le = virtio_legacy_is_little_endian();
> vhost_vq_reset_user_be(vq);
> }
> @@ -381,7 +386,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> mutex_init(&dev->mutex);
> dev->log_ctx = NULL;
> dev->log_file = NULL;
> - dev->memory = NULL;
> + dev->umem = NULL;
> dev->mm = NULL;
> spin_lock_init(&dev->work_lock);
> INIT_LIST_HEAD(&dev->work_list);
> @@ -486,27 +491,36 @@ err_mm:
> }
> EXPORT_SYMBOL_GPL(vhost_dev_set_owner);
>
> -struct vhost_memory *vhost_dev_reset_owner_prepare(void)
> +static void *vhost_kvzalloc(unsigned long size)
> +{
> + void *n = kzalloc(size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
> +
> + if (!n)
> + n = vzalloc(size);
> + return n;
> +}
> +
> +struct vhost_umem *vhost_dev_reset_owner_prepare(void)
> {
> - return kmalloc(offsetof(struct vhost_memory, regions), GFP_KERNEL);
> + return vhost_kvzalloc(sizeof(struct vhost_umem));
> }
> EXPORT_SYMBOL_GPL(vhost_dev_reset_owner_prepare);
>
> /* Caller should have device mutex */
> -void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_memory *memory)
> +void vhost_dev_reset_owner(struct vhost_dev *dev, struct vhost_umem *umem)
> {
> int i;
>
> vhost_dev_cleanup(dev, true);
>
> /* Restore memory to default empty mapping. */
> - memory->nregions = 0;
> - dev->memory = memory;
> + INIT_LIST_HEAD(&umem->umem_list);
> + dev->umem = umem;
> /* We don't need VQ locks below since vhost_dev_cleanup makes sure
> * VQs aren't running.
> */
> for (i = 0; i < dev->nvqs; ++i)
> - dev->vqs[i]->memory = memory;
> + dev->vqs[i]->umem = umem;
> }
> EXPORT_SYMBOL_GPL(vhost_dev_reset_owner);
>
> @@ -523,6 +537,19 @@ void vhost_dev_stop(struct vhost_dev *dev)
> }
> EXPORT_SYMBOL_GPL(vhost_dev_stop);
>
> +static void vhost_umem_clean(struct vhost_dev *dev)
> +{
> + struct vhost_umem_node *node, *tmp;
> + struct vhost_umem *umem = dev->umem;
> +
> + list_for_each_entry_safe(node, tmp, &umem->umem_list, link) {
> + vhost_umem_interval_tree_remove(node, &umem->umem_tree);
> + list_del(&node->link);
> + kvfree(node);
> + }
> + kvfree(umem);
> +}
> +
> /* Caller should have device mutex if and only if locked is set */
> void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
> {
> @@ -549,8 +576,8 @@ void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
> fput(dev->log_file);
> dev->log_file = NULL;
> /* No one will access memory at this point */
> - kvfree(dev->memory);
> - dev->memory = NULL;
> + vhost_umem_clean(dev);
> + dev->umem = NULL;
> WARN_ON(!list_empty(&dev->work_list));
> if (dev->worker) {
> kthread_stop(dev->worker);
> @@ -576,25 +603,25 @@ static int log_access_ok(void __user *log_base, u64 addr, unsigned long sz)
> }
>
> /* Caller should have vq mutex and device mutex. */
> -static int vq_memory_access_ok(void __user *log_base, struct vhost_memory *mem,
> +static int vq_memory_access_ok(void __user *log_base, struct vhost_umem *umem,
> int log_all)
> {
> - int i;
> + struct vhost_umem_node *node;
>
> - if (!mem)
> + if (!umem)
> return 0;
>
> - for (i = 0; i < mem->nregions; ++i) {
> - struct vhost_memory_region *m = mem->regions + i;
> - unsigned long a = m->userspace_addr;
> - if (m->memory_size > ULONG_MAX)
> + list_for_each_entry(node, &umem->umem_list, link) {
> + unsigned long a = node->userspace_addr;
> +
> + if (node->size > ULONG_MAX)
> return 0;
> else if (!access_ok(VERIFY_WRITE, (void __user *)a,
> - m->memory_size))
> + node->size))
> return 0;
> else if (log_all && !log_access_ok(log_base,
> - m->guest_phys_addr,
> - m->memory_size))
> + node->start,
> + node->size))
> return 0;
> }
> return 1;
> @@ -602,7 +629,7 @@ static int vq_memory_access_ok(void __user *log_base, struct vhost_memory *mem,
>
> /* Can we switch to this memory table? */
> /* Caller should have device mutex but not vq mutex */
> -static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem,
> +static int memory_access_ok(struct vhost_dev *d, struct vhost_umem *umem,
> int log_all)
> {
> int i;
> @@ -615,7 +642,8 @@ static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem,
> log = log_all || vhost_has_feature(d->vqs[i], VHOST_F_LOG_ALL);
> /* If ring is inactive, will check when it's enabled. */
> if (d->vqs[i]->private_data)
> - ok = vq_memory_access_ok(d->vqs[i]->log_base, mem, log);
> + ok = vq_memory_access_ok(d->vqs[i]->log_base,
> + umem, log);
> else
> ok = 1;
> mutex_unlock(&d->vqs[i]->mutex);
> @@ -642,7 +670,7 @@ static int vq_access_ok(struct vhost_virtqueue *vq, unsigned int num,
> /* Caller should have device mutex but not vq mutex */
> int vhost_log_access_ok(struct vhost_dev *dev)
> {
> - return memory_access_ok(dev, dev->memory, 1);
> + return memory_access_ok(dev, dev->umem, 1);
> }
> EXPORT_SYMBOL_GPL(vhost_log_access_ok);
>
> @@ -653,7 +681,7 @@ static int vq_log_access_ok(struct vhost_virtqueue *vq,
> {
> size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
>
> - return vq_memory_access_ok(log_base, vq->memory,
> + return vq_memory_access_ok(log_base, vq->umem,
> vhost_has_feature(vq, VHOST_F_LOG_ALL)) &&
> (!vq->log_used || log_access_ok(log_base, vq->log_addr,
> sizeof *vq->used +
> @@ -669,28 +697,12 @@ int vhost_vq_access_ok(struct vhost_virtqueue *vq)
> }
> EXPORT_SYMBOL_GPL(vhost_vq_access_ok);
>
> -static int vhost_memory_reg_sort_cmp(const void *p1, const void *p2)
> -{
> - const struct vhost_memory_region *r1 = p1, *r2 = p2;
> - if (r1->guest_phys_addr < r2->guest_phys_addr)
> - return 1;
> - if (r1->guest_phys_addr > r2->guest_phys_addr)
> - return -1;
> - return 0;
> -}
> -
> -static void *vhost_kvzalloc(unsigned long size)
> -{
> - void *n = kzalloc(size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
> -
> - if (!n)
> - n = vzalloc(size);
> - return n;
> -}
> -
> static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
> {
> - struct vhost_memory mem, *newmem, *oldmem;
> + struct vhost_memory mem, *newmem;
> + struct vhost_memory_region *region;
> + struct vhost_umem_node *node;
> + struct vhost_umem *newumem, *oldumem;
> unsigned long size = offsetof(struct vhost_memory, regions);
> int i;
>
> @@ -710,24 +722,51 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
> kvfree(newmem);
> return -EFAULT;
> }
> - sort(newmem->regions, newmem->nregions, sizeof(*newmem->regions),
> - vhost_memory_reg_sort_cmp, NULL);
>
> - if (!memory_access_ok(d, newmem, 0)) {
> + newumem = vhost_kvzalloc(sizeof(*newumem));
> + if (!newumem) {
> kvfree(newmem);
> - return -EFAULT;
> + return -ENOMEM;
> + }
> +
> + newumem->umem_tree = RB_ROOT;
> + INIT_LIST_HEAD(&newumem->umem_list);
> +
> + for (region = newmem->regions;
> + region < newmem->regions + mem.nregions;
> + region++) {
> + node = vhost_kvzalloc(sizeof(*node));
> + if (!node)
> + goto err;
> + node->start = region->guest_phys_addr;
> + node->size = region->memory_size;
> + node->last = node->start + node->size - 1;
> + node->userspace_addr = region->userspace_addr;
> + INIT_LIST_HEAD(&node->link);
> + list_add_tail(&node->link, &newumem->umem_list);
> + vhost_umem_interval_tree_insert(node, &newumem->umem_tree);
> }
> - oldmem = d->memory;
> - d->memory = newmem;
> +
> + if (!memory_access_ok(d, newumem, 0))
> + goto err;
> +
> + oldumem = d->umem;
> + d->umem = newumem;
>
> /* All memory accesses are done under some VQ mutex. */
> for (i = 0; i < d->nvqs; ++i) {
> mutex_lock(&d->vqs[i]->mutex);
> - d->vqs[i]->memory = newmem;
> + d->vqs[i]->umem = newumem;
> mutex_unlock(&d->vqs[i]->mutex);
> }
> - kvfree(oldmem);
> +
> + kvfree(newmem);
> return 0;
> +
> +err:
> + vhost_umem_clean(d);
> + kvfree(newmem);
> + return -EFAULT;
> }
>
> long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> @@ -1017,28 +1056,6 @@ done:
> }
> EXPORT_SYMBOL_GPL(vhost_dev_ioctl);
>
> -static const struct vhost_memory_region *find_region(struct vhost_memory *mem,
> - __u64 addr, __u32 len)
> -{
> - const struct vhost_memory_region *reg;
> - int start = 0, end = mem->nregions;
> -
> - while (start < end) {
> - int slot = start + (end - start) / 2;
> - reg = mem->regions + slot;
> - if (addr >= reg->guest_phys_addr)
> - end = slot;
> - else
> - start = slot + 1;
> - }
> -
> - reg = mem->regions + start;
> - if (addr >= reg->guest_phys_addr &&
> - reg->guest_phys_addr + reg->memory_size > addr)
> - return reg;
> - return NULL;
> -}
> -
> /* TODO: This is really inefficient. We need something like get_user()
> * (instruction directly accesses the data, with an exception table entry
> * returning -EFAULT). See Documentation/x86/exception-tables.txt.
> @@ -1180,29 +1197,29 @@ EXPORT_SYMBOL_GPL(vhost_init_used);
> static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
> struct iovec iov[], int iov_size)
> {
> - const struct vhost_memory_region *reg;
> - struct vhost_memory *mem;
> + const struct vhost_umem_node *node;
> + struct vhost_umem *umem = vq->umem;
> struct iovec *_iov;
> u64 s = 0;
> int ret = 0;
>
> - mem = vq->memory;
> while ((u64)len > s) {
> u64 size;
> if (unlikely(ret >= iov_size)) {
> ret = -ENOBUFS;
> break;
> }
> - reg = find_region(mem, addr, len);
> - if (unlikely(!reg)) {
> + node = vhost_umem_interval_tree_iter_first(&umem->umem_tree,
> + addr, addr + len - 1);
> + if (node == NULL || node->start > addr) {
> ret = -EFAULT;
> break;
> }
> _iov = iov + ret;
> - size = reg->memory_size - addr + reg->guest_phys_addr;
> + size = node->size - addr + node->start;
> _iov->iov_len = min((u64)len - s, size);
> _iov->iov_base = (void __user *)(unsigned long)
> - (reg->userspace_addr + addr - reg->guest_phys_addr);
> + (node->userspace_addr + addr - node->start);
> s += size;
> addr += size;
> ++ret;
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index d3f7674..5d64393 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -52,6 +52,25 @@ struct vhost_log {
> u64 len;
> };
>
> +#define START(node) ((node)->start)
> +#define LAST(node) ((node)->last)
> +
> +struct vhost_umem_node {
> + struct rb_node rb;
> + struct list_head link;
> + __u64 start;
> + __u64 last;
> + __u64 size;
> + __u64 userspace_addr;
> + __u64 flags_padding;
> + __u64 __subtree_last;
> +};
> +
> +struct vhost_umem {
> + struct rb_root umem_tree;
> + struct list_head umem_list;
> +};
> +
> /* The virtqueue structure describes a queue attached to a device. */
> struct vhost_virtqueue {
> struct vhost_dev *dev;
> @@ -100,7 +119,7 @@ struct vhost_virtqueue {
> struct iovec *indirect;
> struct vring_used_elem *heads;
> /* Protected by virtqueue mutex. */
> - struct vhost_memory *memory;
> + struct vhost_umem *umem;
> void *private_data;
> u64 acked_features;
> /* Log write descriptors */
> @@ -117,7 +136,6 @@ struct vhost_virtqueue {
> };
>
> struct vhost_dev {
> - struct vhost_memory *memory;
> struct mm_struct *mm;
> struct mutex mutex;
> struct vhost_virtqueue **vqs;
> @@ -127,14 +145,15 @@ struct vhost_dev {
> spinlock_t work_lock;
> struct list_head work_list;
> struct task_struct *worker;
> + struct vhost_umem *umem;
> };
>
> void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs, int nvqs);
> long vhost_dev_set_owner(struct vhost_dev *dev);
> bool vhost_dev_has_owner(struct vhost_dev *dev);
> long vhost_dev_check_owner(struct vhost_dev *);
> -struct vhost_memory *vhost_dev_reset_owner_prepare(void);
> -void vhost_dev_reset_owner(struct vhost_dev *, struct vhost_memory *);
> +struct vhost_umem *vhost_dev_reset_owner_prepare(void);
> +void vhost_dev_reset_owner(struct vhost_dev *, struct vhost_umem *);
> void vhost_dev_cleanup(struct vhost_dev *, bool locked);
> void vhost_dev_stop(struct vhost_dev *);
> long vhost_dev_ioctl(struct vhost_dev *, unsigned int ioctl, void __user *argp);
^ permalink raw reply
* Re: virtio pull for 4.5 (was Re: [PULL] virtio: barrier rework+fixes)
From: Linus Torvalds @ 2016-01-19 1:01 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Cc: dave@stgolabs.net, davem@davemloft.net, david.vrabel@citrix.com, dbueso@suse.de, heiko.carstens@de.ibm.com, james.hogan@imgtec.com, joe@perches.com, julian.calaby@gmail.com, linux-arch@vger.kernel.org, linux@arm.linux.org.uk, minchan@kernel.org, mingo@kernel.org, paulmck@linux.vnet.ibm.com, peterz@infradead.org, richard@nod.at, rmk+kernel@arm.linux.org.uk, schwidefsky@de.ibm.com, stable@vger.kernel.org, stefanha@redhat.com, tglx@linutronix.de,,
Rafael Aquini, KVM list, Network Development, Boqun Feng
In-Reply-To: <20160118152028-mutt-send-email-mst@redhat.com>
On Mon, Jan 18, 2016 at 5:21 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> Hi Linus,
> Just making sure nothing's wrong with this pull request.
> If there's an issue, pls let me know!
It was just pulled because I wasn't 100% sure I wanted the extra
indirection. Oh well, pulled now.
One question:
- the arch/sh/ part of the pacth looks dubious. Why does it do that
#define smp_store_mb(var, value) __smp_store_mb(var, value)
despite the commit log saying it's done by asm-generic?
I haven't pushed out yet, my allmodconfig sanity-check build is still going..
Linus
^ permalink raw reply
* Re: virtio pull for 4.5 (was Re: [PULL] virtio: barrier rework+fixes)
From: Davidlohr Bueso @ 2016-01-18 19:07 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aquini, kvm, peterz, benh, heiko.carstens, alexander.duyck,
virtualization, mingo, linux-arch, linux, dbueso, richard,
bjorn.andersson, rmk+kernel, paulmck, james.hogan, arnd,
julian.calaby, boqun.feng, schwidefsky, stefanha, tglx, tony.luck,
netdev, linux-kernel, stable, minchan, david.vrabel, joe, akpm,
Linus Torvalds, davem
In-Reply-To: <20160118152028-mutt-send-email-mst@redhat.com>
On Mon, 18 Jan 2016, Michael S. Tsirkin wrote:
>Hi Linus,
>Just making sure nothing's wrong with this pull request.
>If there's an issue, pls let me know!
>Thanks!
>
>On Wed, Jan 13, 2016 at 06:28:55PM +0200, Michael S. Tsirkin wrote:
>> The following changes since commit afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc:
>>
>> Linux 4.4 (2016-01-10 15:01:32 -0800)
>>
>> are available in the git repository at:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
>>
>> for you to fetch changes up to 43e361f23c49dbddf74f56ddf6cdd85c5dbff6da:
>>
>> checkpatch: add virt barriers (2016-01-12 20:47:08 +0200)
>>
>> ----------------------------------------------------------------
>> virtio: barrier rework+fixes
>>
>> This adds a new kind of barrier, and reworks virtio and xen
>> to use it.
>> Plus some fixes here and there.
>>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>
>> ----------------------------------------------------------------
>>
>> Yes I know that the patch by Davidlohr Bueso has a typo in the subject :(
>>
>> Davidlohr Bueso (1):
>> lcoking/barriers, arch: Use smp barriers in smp_store_release()
fyi this arrived in Linus' tree through Ingo's pull request for -tip:
https://lkml.org/lkml/2016/1/11/319
^ permalink raw reply
* Re: [v3,11/41] mips: reuse asm-generic/barrier.h
From: Paul E. McKenney @ 2016-01-18 15:46 UTC (permalink / raw)
To: Herbert Xu
Cc: linux-mips, linux-ia64, mst, peterz, will.deacon, virtualization,
hpa, sparclinux, mingo, linux-arch, linux-s390, linux, arnd,
linux-sh, mpe, x86, Linus Torvalds, xen-devel, mingo,
linux-xtensa, james.hogan, user-mode-linux-devel,
stefano.stabellini, adi-buildroot-devel, Leonid.Yegoshin,
ddaney.cavm, tglx, linux-metag, linux-arm-kernel, andrew.cooper3,
linux-kernel, ralf, joe
In-Reply-To: <20160118081929.GA30420@gondor.apana.org.au>
On Mon, Jan 18, 2016 at 04:19:29PM +0800, Herbert Xu wrote:
> Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> >
> > You could use SYNC_ACQUIRE() to implement read_barrier_depends() and
> > smp_read_barrier_depends(), but SYNC_RMB probably does not suffice.
> > The reason for this is that smp_read_barrier_depends() must order the
> > pointer load against any subsequent read or write through a dereference
> > of that pointer. For example:
> >
> > p = READ_ONCE(gp);
> > smp_rmb();
> > r1 = p->a; /* ordered by smp_rmb(). */
> > p->b = 42; /* NOT ordered by smp_rmb(), BUG!!! */
> > r2 = x; /* ordered by smp_rmb(), but doesn't need to be. */
> >
> > In contrast:
> >
> > p = READ_ONCE(gp);
> > smp_read_barrier_depends();
> > r1 = p->a; /* ordered by smp_read_barrier_depends(). */
> > p->b = 42; /* ordered by smp_read_barrier_depends(). */
> > r2 = x; /* not ordered by smp_read_barrier_depends(), which is OK. */
> >
> > Again, if your hardware maintains local ordering for address
> > and data dependencies, you can have read_barrier_depends() and
> > smp_read_barrier_depends() be no-ops like they are for most
> > architectures.
> >
> > Does that help?
>
> This is crazy! smp_rmb started out being strictly stronger than
> smp_read_barrier_depends, when did this stop being the case?
Hello, Herbert!
It is true that most Linux kernel code relies only on the read-read
properties of dependencies, but the read-write properties are useful.
Admittedly relatively rarely, but useful.
The better comparison for smp_read_barrier_depends(), especially in
its rcu_dereference*() form, is smp_load_acquire().
Thanx, Paul
^ permalink raw reply
* virtio pull for 4.5 (was Re: [PULL] virtio: barrier rework+fixes)
From: Michael S. Tsirkin @ 2016-01-18 13:21 UTC (permalink / raw)
To: Linus Torvalds
Cc: aquini, kvm, peterz, benh, heiko.carstens, alexander.duyck,
virtualization, mingo, linux-arch, dave, linux, dbueso, richard,
bjorn.andersson, rmk+kernel, paulmck, james.hogan, arnd,
julian.calaby, boqun.feng, schwidefsky, stefanha, tglx, tony.luck,
netdev, linux-kernel, stable, minchan, david.vrabel, joe, akpm,
davem
In-Reply-To: <20160113182855-mutt-send-email-mst@redhat.com>
Hi Linus,
Just making sure nothing's wrong with this pull request.
If there's an issue, pls let me know!
Thanks!
On Wed, Jan 13, 2016 at 06:28:55PM +0200, Michael S. Tsirkin wrote:
> The following changes since commit afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc:
>
> Linux 4.4 (2016-01-10 15:01:32 -0800)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
>
> for you to fetch changes up to 43e361f23c49dbddf74f56ddf6cdd85c5dbff6da:
>
> checkpatch: add virt barriers (2016-01-12 20:47:08 +0200)
>
> ----------------------------------------------------------------
> virtio: barrier rework+fixes
>
> This adds a new kind of barrier, and reworks virtio and xen
> to use it.
> Plus some fixes here and there.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ----------------------------------------------------------------
>
> Yes I know that the patch by Davidlohr Bueso has a typo in the subject :(
>
> Davidlohr Bueso (1):
> lcoking/barriers, arch: Use smp barriers in smp_store_release()
>
> Michael S. Tsirkin (40):
> asm-generic: guard smp_store_release/load_acquire
> ia64: rename nop->iosapic_nop
> ia64: reuse asm-generic/barrier.h
> powerpc: reuse asm-generic/barrier.h
> s390: reuse asm-generic/barrier.h
> sparc: reuse asm-generic/barrier.h
> arm: reuse asm-generic/barrier.h
> arm64: reuse asm-generic/barrier.h
> metag: reuse asm-generic/barrier.h
> mips: reuse asm-generic/barrier.h
> x86/um: reuse asm-generic/barrier.h
> x86: reuse asm-generic/barrier.h
> asm-generic: add __smp_xxx wrappers
> powerpc: define __smp_xxx
> arm64: define __smp_xxx
> arm: define __smp_xxx
> blackfin: define __smp_xxx
> ia64: define __smp_xxx
> metag: define __smp_xxx
> mips: define __smp_xxx
> s390: define __smp_xxx
> sh: define __smp_xxx, fix smp_store_mb for !SMP
> sparc: define __smp_xxx
> tile: define __smp_xxx
> xtensa: define __smp_xxx
> x86: define __smp_xxx
> asm-generic: implement virt_xxx memory barriers
> Revert "virtio_ring: Update weak barriers to use dma_wmb/rmb"
> virtio_ring: update weak barriers to use virt_xxx
> sh: support 1 and 2 byte xchg
> sh: move xchg_cmpxchg to a header by itself
> virtio_ring: use virt_store_mb
> xenbus: use virt_xxx barriers
> xen/io: use virt_xxx barriers
> xen/events: use virt_xxx barriers
> s390: use generic memory barriers
> s390: more efficient smp barriers
> checkpatch.pl: add missing memory barriers
> checkpatch: check for __smp outside barrier.h
> checkpatch: add virt barriers
>
> Minchan Kim (2):
> virtio_balloon: fix race by fill and leak
> virtio_balloon: fix race between migration and ballooning
>
> Stefan Hajnoczi (1):
> virtio: make find_vqs() checkpatch.pl-friendly
>
> arch/arm/include/asm/barrier.h | 35 ++---------
> arch/arm64/include/asm/barrier.h | 19 ++----
> arch/blackfin/include/asm/barrier.h | 4 +-
> arch/ia64/include/asm/barrier.h | 24 +++-----
> arch/metag/include/asm/barrier.h | 55 +++++------------
> arch/mips/include/asm/barrier.h | 51 +++++-----------
> arch/powerpc/include/asm/barrier.h | 33 ++++------
> arch/s390/include/asm/barrier.h | 23 ++++---
> arch/sh/include/asm/barrier.h | 3 +-
> arch/sh/include/asm/cmpxchg-grb.h | 22 +++++++
> arch/sh/include/asm/cmpxchg-irq.h | 11 ++++
> arch/sh/include/asm/cmpxchg-llsc.h | 25 +-------
> arch/sh/include/asm/cmpxchg-xchg.h | 51 ++++++++++++++++
> arch/sh/include/asm/cmpxchg.h | 3 +
> arch/sparc/include/asm/barrier_32.h | 1 -
> arch/sparc/include/asm/barrier_64.h | 29 ++-------
> arch/sparc/include/asm/processor.h | 3 -
> arch/tile/include/asm/barrier.h | 9 +--
> arch/x86/include/asm/barrier.h | 36 +++++------
> arch/x86/um/asm/barrier.h | 9 +--
> arch/xtensa/include/asm/barrier.h | 4 +-
> drivers/virtio/virtio_pci_common.h | 2 +-
> include/asm-generic/barrier.h | 106 ++++++++++++++++++++++++++++++---
> include/linux/virtio_config.h | 2 +-
> include/linux/virtio_ring.h | 21 +++++--
> include/xen/interface/io/ring.h | 16 ++---
> arch/ia64/kernel/iosapic.c | 6 +-
> drivers/gpu/drm/virtio/virtgpu_kms.c | 2 +-
> drivers/misc/mic/card/mic_virtio.c | 2 +-
> drivers/remoteproc/remoteproc_virtio.c | 2 +-
> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +-
> drivers/s390/virtio/kvm_virtio.c | 2 +-
> drivers/s390/virtio/virtio_ccw.c | 2 +-
> drivers/virtio/virtio_balloon.c | 4 +-
> drivers/virtio/virtio_input.c | 2 +-
> drivers/virtio/virtio_mmio.c | 2 +-
> drivers/virtio/virtio_pci_common.c | 4 +-
> drivers/virtio/virtio_pci_modern.c | 2 +-
> drivers/virtio/virtio_ring.c | 15 +++--
> drivers/xen/events/events_fifo.c | 3 +-
> drivers/xen/xenbus/xenbus_comms.c | 8 +--
> mm/balloon_compaction.c | 4 +-
> Documentation/memory-barriers.txt | 28 +++++++--
> scripts/checkpatch.pl | 33 +++++++++-
> 44 files changed, 401 insertions(+), 319 deletions(-)
> create mode 100644 arch/sh/include/asm/cmpxchg-xchg.h
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox