2020-04-18 10:25:17

by Eugenio Perez Martin

[permalink] [raw]
Subject: [PATCH v3 0/8] tools/vhost: Reset virtqueue on tests

This series add tests used to validate the "vhost: Reset batched
descriptors on SET_VRING_BASE call" series, with a few minor updates of
them.

They are based on the tests sent back them, the ones that were not
included (reasons in that thread). This series changes:

* Delete need to export the ugly function in virtio_ring, now all the
code is added in tools/virtio (except the one line fix).
* Add forgotten uses of vhost_vq_set_backend. Fix bad usage order in
vhost_test_set_backend.
* Drop random reset, not really needed.
* Minor changes.

The first patch of this patchset ("vhost: Not cleaning batched descs in
VHOST_SET_VRING_BASE ioctl") should be squashed with ("vhost: batching
fetches") (currenlty, commit e7539c20a4a60b3a1bda3e7218c0d2a20669f357
in mst repository vhost branch).

Thanks!

Changes from v2:
* Squashed commits with fixes.
* Back to plain vring_*

Changes from v1:
* Different base, since branch was force-pushed.
* Using new vring_legacy_*, as base uses them now.

This serie is meant to be applied on top of
801f9bae9cf35b3192c3959d81a717e7985c64ed in
git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git.

Eugenio Pérez (8):
vhost: Not cleaning batched descs in VHOST_SET_VRING_BASE ioctl
tools/virtio: Add --batch option
tools/virtio: Add --batch=random option
tools/virtio: Add --reset
tools/virtio: Use __vring_new_virtqueue in virtio_test.c
tools/virtio: Extract virtqueue initialization in vq_reset
tools/virtio: Reset index in virtio_test --reset.
tools/virtio: Use tools/include/list.h instead of stubs

drivers/vhost/test.c | 57 +++++++++++++++
drivers/vhost/test.h | 1 +
drivers/vhost/vhost.c | 1 -
tools/virtio/linux/kernel.h | 7 +-
tools/virtio/linux/virtio.h | 5 +-
tools/virtio/virtio_test.c | 139 ++++++++++++++++++++++++++++++------
tools/virtio/vringh_test.c | 2 +
7 files changed, 182 insertions(+), 30 deletions(-)

--
2.18.1


2020-04-18 10:25:55

by Eugenio Perez Martin

[permalink] [raw]
Subject: [PATCH v3 6/8] tools/virtio: Extract virtqueue initialization in vq_reset

So we can reset after that in the main loop.

Signed-off-by: Eugenio Pérez <[email protected]>
---
tools/virtio/virtio_test.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 0a247ba3b899..bc16c818bda3 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -94,6 +94,19 @@ void vhost_vq_setup(struct vdev_info *dev, struct vq_info *info)
assert(r >= 0);
}

+static void vq_reset(struct vq_info *info, int num, struct virtio_device *vdev)
+{
+ if (info->vq)
+ vring_del_virtqueue(info->vq);
+
+ memset(info->ring, 0, vring_size(num, 4096));
+ vring_init(&info->vring, num, info->ring, 4096);
+ info->vq = __vring_new_virtqueue(info->idx, info->vring, vdev, true,
+ false, vq_notify, vq_callback, "test");
+ assert(info->vq);
+ info->vq->priv = info;
+}
+
static void vq_info_add(struct vdev_info *dev, int num)
{
struct vq_info *info = &dev->vqs[dev->nvqs];
@@ -103,13 +116,7 @@ static void vq_info_add(struct vdev_info *dev, int num)
info->call = eventfd(0, EFD_NONBLOCK);
r = posix_memalign(&info->ring, 4096, vring_size(num, 4096));
assert(r >= 0);
- memset(info->ring, 0, vring_size(num, 4096));
- vring_init(&info->vring, num, info->ring, 4096);
- info->vq =
- __vring_new_virtqueue(info->idx, info->vring, &dev->vdev, true,
- false, vq_notify, vq_callback, "test");
- assert(info->vq);
- info->vq->priv = info;
+ vq_reset(info, num, &dev->vdev);
vhost_vq_setup(dev, info);
dev->fds[info->idx].fd = info->call;
dev->fds[info->idx].events = POLLIN;
--
2.18.1

2020-04-18 10:25:55

by Eugenio Perez Martin

[permalink] [raw]
Subject: [PATCH v3 7/8] tools/virtio: Reset index in virtio_test --reset.

This way behavior for vhost is more like a VM.

Signed-off-by: Eugenio Pérez <[email protected]>
---
tools/virtio/virtio_test.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index bc16c818bda3..82902fc3ba2a 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -48,6 +48,7 @@ struct vdev_info {

static const struct vhost_vring_file no_backend = { .fd = -1 },
backend = { .fd = 1 };
+static const struct vhost_vring_state null_state = {};

bool vq_notify(struct virtqueue *vq)
{
@@ -173,14 +174,19 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
unsigned len;
long long spurious = 0;
const bool random_batch = batch == RANDOM_BATCH;
+
r = ioctl(dev->control, VHOST_TEST_RUN, &test);
assert(r >= 0);
+ if (!reset_n) {
+ next_reset = INT_MAX;
+ }
+
for (;;) {
virtqueue_disable_cb(vq->vq);
completed_before = completed;
started_before = started;
do {
- const bool reset = reset_n && completed > next_reset;
+ const bool reset = completed > next_reset;
if (random_batch)
batch = (random() % vq->vring.num) + 1;

@@ -223,10 +229,24 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
}

if (reset) {
+ struct vhost_vring_state s = { .index = 0 };
+
+ vq_reset(vq, vq->vring.num, &dev->vdev);
+
+ r = ioctl(dev->control, VHOST_GET_VRING_BASE,
+ &s);
+ assert(!r);
+
+ s.num = 0;
+ r = ioctl(dev->control, VHOST_SET_VRING_BASE,
+ &null_state);
+ assert(!r);
+
r = ioctl(dev->control, VHOST_TEST_SET_BACKEND,
&backend);
assert(!r);

+ started = completed;
while (completed > next_reset)
next_reset += completed;
}
@@ -248,7 +268,9 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
test = 0;
r = ioctl(dev->control, VHOST_TEST_RUN, &test);
assert(r >= 0);
- fprintf(stderr, "spurious wakeups: 0x%llx\n", spurious);
+ fprintf(stderr,
+ "spurious wakeups: 0x%llx started=0x%lx completed=0x%lx\n",
+ spurious, started, completed);
}

const char optstring[] = "h";
--
2.18.1

2020-04-18 10:26:15

by Eugenio Perez Martin

[permalink] [raw]
Subject: [PATCH v3 5/8] tools/virtio: Use __vring_new_virtqueue in virtio_test.c

As updated in ("2a2d1382fe9d virtio: Add improved queue allocation API")

Signed-off-by: Eugenio Pérez <[email protected]>
---
tools/virtio/virtio_test.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 6bc3e172cc9b..0a247ba3b899 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -105,10 +105,9 @@ static void vq_info_add(struct vdev_info *dev, int num)
assert(r >= 0);
memset(info->ring, 0, vring_size(num, 4096));
vring_init(&info->vring, num, info->ring, 4096);
- info->vq = vring_new_virtqueue(info->idx,
- info->vring.num, 4096, &dev->vdev,
- true, false, info->ring,
- vq_notify, vq_callback, "test");
+ info->vq =
+ __vring_new_virtqueue(info->idx, info->vring, &dev->vdev, true,
+ false, vq_notify, vq_callback, "test");
assert(info->vq);
info->vq->priv = info;
vhost_vq_setup(dev, info);
--
2.18.1

2020-04-18 10:27:25

by Eugenio Perez Martin

[permalink] [raw]
Subject: [PATCH v3 3/8] tools/virtio: Add --batch=random option

So we can test with non-deterministic batches in flight.

Signed-off-by: Eugenio Pérez <[email protected]>
---
tools/virtio/virtio_test.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index c30de9088f3c..4a2b9d11f287 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -19,6 +19,8 @@
#include <linux/virtio_ring.h>
#include "../../drivers/vhost/test.h"

+#define RANDOM_BATCH -1
+
/* Unused */
void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end;

@@ -161,6 +163,7 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
int r, test = 1;
unsigned len;
long long spurious = 0;
+ const bool random_batch = batch == RANDOM_BATCH;
r = ioctl(dev->control, VHOST_TEST_RUN, &test);
assert(r >= 0);
for (;;) {
@@ -168,6 +171,9 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
completed_before = completed;
started_before = started;
do {
+ if (random_batch)
+ batch = (random() % vq->vring.num) + 1;
+
while (started < bufs &&
(started - completed) < batch) {
sg_init_one(&sl, dev->buf, dev->buf_size);
@@ -275,7 +281,7 @@ static void help(void)
" [--no-event-idx]"
" [--no-virtio-1]"
" [--delayed-interrupt]"
- " [--batch=N]"
+ " [--batch=random/N]"
"\n");
}

@@ -312,9 +318,13 @@ int main(int argc, char **argv)
delayed = true;
break;
case 'b':
- batch = strtol(optarg, NULL, 10);
- assert(batch > 0);
- assert(batch < (long)INT_MAX + 1);
+ if (0 == strcmp(optarg, "random")) {
+ batch = RANDOM_BATCH;
+ } else {
+ batch = strtol(optarg, NULL, 10);
+ assert(batch > 0);
+ assert(batch < (long)INT_MAX + 1);
+ }
break;
default:
assert(0);
--
2.18.1

2020-04-18 10:27:32

by Eugenio Perez Martin

[permalink] [raw]
Subject: [PATCH v3 1/8] vhost: Not cleaning batched descs in VHOST_SET_VRING_BASE ioctl

They are cleaned in vhost_vq_set_backend, which can be called with an
active backend. To set and remove backends already clean batched
descriptors, so to do it here is completely redundant.

Fixes: ("e7539c20a4a6 vhost: batching fetches")

Signed-off-by: Eugenio Pérez <[email protected]>
---
drivers/vhost/vhost.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 0395229486a9..882d0df57e24 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1579,7 +1579,6 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
vq->last_avail_idx = s.num;
/* Forget the cached index value. */
vq->avail_idx = vq->last_avail_idx;
- vq->ndescs = vq->first_desc = 0;
break;
case VHOST_GET_VRING_BASE:
s.index = idx;
--
2.18.1

2020-04-18 10:28:43

by Eugenio Perez Martin

[permalink] [raw]
Subject: [PATCH v3 4/8] tools/virtio: Add --reset

Currently, it only removes and add backend, but it will reset vq
position in future commits.

Signed-off-by: Eugenio Pérez <[email protected]>
---
drivers/vhost/test.c | 57 ++++++++++++++++++++++++++++++++++++++
drivers/vhost/test.h | 1 +
tools/virtio/virtio_test.c | 41 ++++++++++++++++++++++++---
3 files changed, 95 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 02806d6f84ef..6aed0cab8b17 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -263,9 +263,62 @@ static int vhost_test_set_features(struct vhost_test *n, u64 features)
return 0;
}

+static long vhost_test_set_backend(struct vhost_test *n, unsigned index, int fd)
+{
+ static void *backend;
+
+ const bool enable = fd != -1;
+ struct vhost_virtqueue *vq;
+ int r;
+
+ mutex_lock(&n->dev.mutex);
+ r = vhost_dev_check_owner(&n->dev);
+ if (r)
+ goto err;
+
+ if (index >= VHOST_TEST_VQ_MAX) {
+ r = -ENOBUFS;
+ goto err;
+ }
+ vq = &n->vqs[index];
+ mutex_lock(&vq->mutex);
+
+ /* Verify that ring has been setup correctly. */
+ if (!vhost_vq_access_ok(vq)) {
+ r = -EFAULT;
+ goto err_vq;
+ }
+ if (!enable) {
+ vhost_poll_stop(&vq->poll);
+ backend = vhost_vq_get_backend(vq);
+ vhost_vq_set_backend(vq, NULL);
+ } else {
+ vhost_vq_set_backend(vq, backend);
+ r = vhost_vq_init_access(vq);
+ if (r == 0)
+ r = vhost_poll_start(&vq->poll, vq->kick);
+ }
+
+ mutex_unlock(&vq->mutex);
+
+ if (enable) {
+ vhost_test_flush_vq(n, index);
+ }
+
+ mutex_unlock(&n->dev.mutex);
+ return 0;
+
+err_vq:
+ mutex_unlock(&vq->mutex);
+err:
+ mutex_unlock(&n->dev.mutex);
+ return r;
+}
+
static long vhost_test_ioctl(struct file *f, unsigned int ioctl,
unsigned long arg)
{
+ struct vhost_vring_file backend;
struct vhost_test *n = f->private_data;
void __user *argp = (void __user *)arg;
u64 __user *featurep = argp;
@@ -277,6 +330,10 @@ static long vhost_test_ioctl(struct file *f, unsigned int ioctl,
if (copy_from_user(&test, argp, sizeof test))
return -EFAULT;
return vhost_test_run(n, test);
+ case VHOST_TEST_SET_BACKEND:
+ if (copy_from_user(&backend, argp, sizeof backend))
+ return -EFAULT;
+ return vhost_test_set_backend(n, backend.index, backend.fd);
case VHOST_GET_FEATURES:
features = VHOST_FEATURES;
if (copy_to_user(featurep, &features, sizeof features))
diff --git a/drivers/vhost/test.h b/drivers/vhost/test.h
index 7dd265bfdf81..822bc4bee03a 100644
--- a/drivers/vhost/test.h
+++ b/drivers/vhost/test.h
@@ -4,5 +4,6 @@

/* Start a given test on the virtio null device. 0 stops all tests. */
#define VHOST_TEST_RUN _IOW(VHOST_VIRTIO, 0x31, int)
+#define VHOST_TEST_SET_BACKEND _IOW(VHOST_VIRTIO, 0x32, int)

#endif
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 4a2b9d11f287..6bc3e172cc9b 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -46,6 +46,9 @@ struct vdev_info {
struct vhost_memory *mem;
};

+static const struct vhost_vring_file no_backend = { .fd = -1 },
+ backend = { .fd = 1 };
+
bool vq_notify(struct virtqueue *vq)
{
struct vq_info *info = vq->priv;
@@ -155,10 +158,10 @@ static void wait_for_interrupt(struct vdev_info *dev)
}

static void run_test(struct vdev_info *dev, struct vq_info *vq,
- bool delayed, int batch, int bufs)
+ bool delayed, int batch, int reset_n, int bufs)
{
struct scatterlist sl;
- long started = 0, completed = 0;
+ long started = 0, completed = 0, next_reset = reset_n;
long completed_before, started_before;
int r, test = 1;
unsigned len;
@@ -171,6 +174,7 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
completed_before = completed;
started_before = started;
do {
+ const bool reset = reset_n && completed > next_reset;
if (random_batch)
batch = (random() % vq->vring.num) + 1;

@@ -200,12 +204,26 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
if (started >= bufs)
r = -1;

+ if (reset) {
+ r = ioctl(dev->control, VHOST_TEST_SET_BACKEND,
+ &no_backend);
+ assert(!r);
+ }
+
/* Flush out completed bufs if any */
while (virtqueue_get_buf(vq->vq, &len)) {
++completed;
r = 0;
}

+ if (reset) {
+ r = ioctl(dev->control, VHOST_TEST_SET_BACKEND,
+ &backend);
+ assert(!r);
+
+ while (completed > next_reset)
+ next_reset += completed;
+ }
} while (r == 0);
if (completed == completed_before && started == started_before)
++spurious;
@@ -270,6 +288,11 @@ const struct option longopts[] = {
.val = 'b',
.has_arg = required_argument,
},
+ {
+ .name = "reset",
+ .val = 'r',
+ .has_arg = optional_argument,
+ },
{
}
};
@@ -282,6 +305,7 @@ static void help(void)
" [--no-virtio-1]"
" [--delayed-interrupt]"
" [--batch=random/N]"
+ " [--reset=N]"
"\n");
}

@@ -290,7 +314,7 @@ int main(int argc, char **argv)
struct vdev_info dev;
unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
(1ULL << VIRTIO_RING_F_EVENT_IDX) | (1ULL << VIRTIO_F_VERSION_1);
- long batch = 1;
+ long batch = 1, reset = 0;
int o;
bool delayed = false;

@@ -326,6 +350,15 @@ int main(int argc, char **argv)
assert(batch < (long)INT_MAX + 1);
}
break;
+ case 'r':
+ if (!optarg) {
+ reset = 1;
+ } else {
+ reset = strtol(optarg, NULL, 10);
+ assert(reset > 0);
+ assert(reset < (long)INT_MAX + 1);
+ }
+ break;
default:
assert(0);
break;
@@ -335,6 +368,6 @@ int main(int argc, char **argv)
done:
vdev_info_init(&dev, features);
vq_info_add(&dev, 256);
- run_test(&dev, &dev.vqs[0], delayed, batch, 0x100000);
+ run_test(&dev, &dev.vqs[0], delayed, batch, reset, 0x100000);
return 0;
}
--
2.18.1

2020-04-18 10:28:44

by Eugenio Perez Martin

[permalink] [raw]
Subject: [PATCH v3 2/8] tools/virtio: Add --batch option

This allow to test vhost having >1 buffers in flight

Signed-off-by: Eugenio Pérez <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Michael S. Tsirkin <[email protected]>
---
tools/virtio/virtio_test.c | 47 ++++++++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index b427def67e7e..c30de9088f3c 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#define _GNU_SOURCE
#include <getopt.h>
+#include <limits.h>
#include <string.h>
#include <poll.h>
#include <sys/eventfd.h>
@@ -152,11 +153,11 @@ static void wait_for_interrupt(struct vdev_info *dev)
}

static void run_test(struct vdev_info *dev, struct vq_info *vq,
- bool delayed, int bufs)
+ bool delayed, int batch, int bufs)
{
struct scatterlist sl;
long started = 0, completed = 0;
- long completed_before;
+ long completed_before, started_before;
int r, test = 1;
unsigned len;
long long spurious = 0;
@@ -165,28 +166,42 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
for (;;) {
virtqueue_disable_cb(vq->vq);
completed_before = completed;
+ started_before = started;
do {
- if (started < bufs) {
+ while (started < bufs &&
+ (started - completed) < batch) {
sg_init_one(&sl, dev->buf, dev->buf_size);
r = virtqueue_add_outbuf(vq->vq, &sl, 1,
dev->buf + started,
GFP_ATOMIC);
- if (likely(r == 0)) {
- ++started;
- if (unlikely(!virtqueue_kick(vq->vq)))
+ if (unlikely(r != 0)) {
+ if (r == -ENOSPC &&
+ started > started_before)
+ r = 0;
+ else
r = -1;
+ break;
}
- } else
+
+ ++started;
+
+ if (unlikely(!virtqueue_kick(vq->vq))) {
+ r = -1;
+ break;
+ }
+ }
+
+ if (started >= bufs)
r = -1;

/* Flush out completed bufs if any */
- if (virtqueue_get_buf(vq->vq, &len)) {
+ while (virtqueue_get_buf(vq->vq, &len)) {
++completed;
r = 0;
}

} while (r == 0);
- if (completed == completed_before)
+ if (completed == completed_before && started == started_before)
++spurious;
assert(completed <= bufs);
assert(started <= bufs);
@@ -244,6 +259,11 @@ const struct option longopts[] = {
.name = "no-delayed-interrupt",
.val = 'd',
},
+ {
+ .name = "batch",
+ .val = 'b',
+ .has_arg = required_argument,
+ },
{
}
};
@@ -255,6 +275,7 @@ static void help(void)
" [--no-event-idx]"
" [--no-virtio-1]"
" [--delayed-interrupt]"
+ " [--batch=N]"
"\n");
}

@@ -263,6 +284,7 @@ int main(int argc, char **argv)
struct vdev_info dev;
unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
(1ULL << VIRTIO_RING_F_EVENT_IDX) | (1ULL << VIRTIO_F_VERSION_1);
+ long batch = 1;
int o;
bool delayed = false;

@@ -289,6 +311,11 @@ int main(int argc, char **argv)
case 'D':
delayed = true;
break;
+ case 'b':
+ batch = strtol(optarg, NULL, 10);
+ assert(batch > 0);
+ assert(batch < (long)INT_MAX + 1);
+ break;
default:
assert(0);
break;
@@ -298,6 +325,6 @@ int main(int argc, char **argv)
done:
vdev_info_init(&dev, features);
vq_info_add(&dev, 256);
- run_test(&dev, &dev.vqs[0], delayed, 0x100000);
+ run_test(&dev, &dev.vqs[0], delayed, batch, 0x100000);
return 0;
}
--
2.18.1