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.
This patchset commit messages contains references to commits under
"for_linus" tag and references to commits in for_linus..mst/vhost.
They are fixes, so probably it is better just to squash if possible:
("7c48601a3d4d tools/virtio: Add --reset=random"): Already in for_linus
("af3756cfed9a vhost: batching fetches"): Only in vhost branch, not in
for_linus.
Thanks!
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
503b1b3efb47e267001beba8e0759c15fa3e9be7 in
git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git.
Eugenio Pérez (8):
tools/virtio: fix virtio_test.c indentation
vhost: Not cleaning batched descs in VHOST_SET_VRING_BASE ioctl
vhost: Replace vq->private_data access by backend accesors
vhost: Fix bad order in vhost_test_set_backend at enable
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 | 8 ++---
drivers/vhost/vhost.c | 1 -
tools/virtio/linux/kernel.h | 7 +----
tools/virtio/linux/virtio.h | 5 ++--
tools/virtio/virtio_test.c | 58 +++++++++++++++++++++++++++----------
tools/virtio/vringh_test.c | 2 ++
6 files changed, 51 insertions(+), 30 deletions(-)
--
2.18.1
The reset was not done properly: A init call was given with no active
backend. This solves that.
Fixes: ("7c48601a3d4d tools/virtio: Add --reset=random")
Signed-off-by: Eugenio Pérez <[email protected]>
---
drivers/vhost/test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 789c096e454b..6aed0cab8b17 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -293,8 +293,8 @@ static long vhost_test_set_backend(struct vhost_test *n, unsigned index, int fd)
backend = vhost_vq_get_backend(vq);
vhost_vq_set_backend(vq, NULL);
} else {
- r = vhost_vq_init_access(vq);
vhost_vq_set_backend(vq, backend);
+ r = vhost_vq_init_access(vq);
if (r == 0)
r = vhost_poll_start(&vq->poll, vq->kick);
}
--
2.18.1
This way behavior for vhost is more like a VM.
Signed-off-by: Eugenio Pérez <[email protected]>
---
tools/virtio/virtio_test.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 18d5347003eb..dca64d36a882 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -20,7 +20,6 @@
#include "../../drivers/vhost/test.h"
#define RANDOM_BATCH -1
-#define RANDOM_RESET -1
/* Unused */
void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end;
@@ -49,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)
{
@@ -174,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;
@@ -224,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;
}
@@ -249,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";
@@ -312,7 +333,7 @@ static void help(void)
" [--no-virtio-1]"
" [--delayed-interrupt]"
" [--batch=random/N]"
- " [--reset=random/N]"
+ " [--reset=N]"
"\n");
}
@@ -360,11 +381,9 @@ int main(int argc, char **argv)
case 'r':
if (!optarg) {
reset = 1;
- } else if (0 == strcmp(optarg, "random")) {
- reset = RANDOM_RESET;
} else {
reset = strtol(optarg, NULL, 10);
- assert(reset >= 0);
+ assert(reset > 0);
assert(reset < (long)INT_MAX + 1);
}
break;
--
2.18.1
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 d9827b640c21..18d5347003eb 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -95,6 +95,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_legacy_size(num, 4096));
+ vring_legacy_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];
@@ -104,13 +117,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_legacy_size(num, 4096));
assert(r >= 0);
- memset(info->ring, 0, vring_legacy_size(num, 4096));
- vring_legacy_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
Just removing extra whitespace.
Signed-off-by: Eugenio Pérez <[email protected]>
---
tools/virtio/virtio_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 25be607d8711..1d5144590df6 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -222,7 +222,7 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
&backend);
assert(!r);
- while (completed > next_reset)
+ while (completed > next_reset)
next_reset += completed;
}
} while (r == 0);
--
2.18.1
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: ("af3756cfed9a 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
It should not make any significant difference but reduce stub code.
Signed-off-by: Eugenio Pérez <[email protected]>
---
tools/virtio/linux/kernel.h | 7 +------
tools/virtio/linux/virtio.h | 5 ++---
tools/virtio/virtio_test.c | 1 +
tools/virtio/vringh_test.c | 2 ++
4 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/tools/virtio/linux/kernel.h b/tools/virtio/linux/kernel.h
index 6683b4a70b05..caab980211a6 100644
--- a/tools/virtio/linux/kernel.h
+++ b/tools/virtio/linux/kernel.h
@@ -11,6 +11,7 @@
#include <linux/compiler.h>
#include <linux/types.h>
+#include <linux/list.h>
#include <linux/printk.h>
#include <linux/bug.h>
#include <errno.h>
@@ -135,10 +136,4 @@ static inline void free_page(unsigned long addr)
(void) (&_min1 == &_min2); \
_min1 < _min2 ? _min1 : _min2; })
-/* TODO: empty stubs for now. Broken but enough for virtio_ring.c */
-#define list_add_tail(a, b) do {} while (0)
-#define list_del(a) do {} while (0)
-#define list_for_each_entry(a, b, c) while (0)
-/* end of stubs */
-
#endif /* KERNEL_H */
diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index b751350d4ce8..5d90254ddae4 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -11,12 +11,11 @@ struct device {
struct virtio_device {
struct device dev;
u64 features;
+ struct list_head vqs;
};
struct virtqueue {
- /* TODO: commented as list macros are empty stubs for now.
- * Broken but enough for virtio_ring.c
- * struct list_head list; */
+ struct list_head list;
void (*callback)(struct virtqueue *vq);
const char *name;
struct virtio_device *vdev;
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index dca64d36a882..c0b924b41a1d 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -129,6 +129,7 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features)
int r;
memset(dev, 0, sizeof *dev);
dev->vdev.features = features;
+ INIT_LIST_HEAD(&dev->vdev.vqs);
dev->buf_size = 1024;
dev->buf = malloc(dev->buf_size);
assert(dev->buf);
diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 8ee2c9a6ad46..b88b0337fcfd 100644
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -307,6 +307,7 @@ static int parallel_test(u64 features,
close(to_host[0]);
gvdev.vdev.features = features;
+ INIT_LIST_HEAD(&gvdev.vdev.vqs);
gvdev.to_host_fd = to_host[1];
gvdev.notifies = 0;
@@ -453,6 +454,7 @@ int main(int argc, char *argv[])
getrange = getrange_iov;
vdev.features = 0;
+ INIT_LIST_HEAD(&vdev.vqs);
while (argv[1]) {
if (strcmp(argv[1], "--indirect") == 0)
--
2.18.1
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 1d5144590df6..d9827b640c21 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -106,10 +106,9 @@ static void vq_info_add(struct vdev_info *dev, int num)
assert(r >= 0);
memset(info->ring, 0, vring_legacy_size(num, 4096));
vring_legacy_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
This function still places backend directly in private_data, instead of
use the accesors created on ("cbfc8f21b49a vhost: Create accessors for
virtqueues private_data"). Using accesor.
Fixes: ("7ce8cc28ce48 tools/virtio: Add --reset=random")
Signed-off-by: Eugenio Pérez <[email protected]>
---
drivers/vhost/test.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 251ca723ac3f..789c096e454b 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -265,7 +265,7 @@ static int vhost_test_set_features(struct vhost_test *n, u64 features)
static long vhost_test_set_backend(struct vhost_test *n, unsigned index, int fd)
{
- static void *private_data;
+ static void *backend;
const bool enable = fd != -1;
struct vhost_virtqueue *vq;
@@ -290,11 +290,11 @@ static long vhost_test_set_backend(struct vhost_test *n, unsigned index, int fd)
}
if (!enable) {
vhost_poll_stop(&vq->poll);
- private_data = vq->private_data;
- vq->private_data = NULL;
+ backend = vhost_vq_get_backend(vq);
+ vhost_vq_set_backend(vq, NULL);
} else {
r = vhost_vq_init_access(vq);
- vq->private_data = private_data;
+ vhost_vq_set_backend(vq, backend);
if (r == 0)
r = vhost_poll_start(&vq->poll, vq->kick);
}
--
2.18.1
On Thu, Apr 16, 2020 at 09:56:40AM +0200, Eugenio P?rez wrote:
> As updated in ("2a2d1382fe9d virtio: Add improved queue allocation API")
>
> Signed-off-by: Eugenio P?rez <[email protected]>
Pls add motivation for these changes.
> ---
> 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 1d5144590df6..d9827b640c21 100644
> --- a/tools/virtio/virtio_test.c
> +++ b/tools/virtio/virtio_test.c
> @@ -106,10 +106,9 @@ static void vq_info_add(struct vdev_info *dev, int num)
> assert(r >= 0);
> memset(info->ring, 0, vring_legacy_size(num, 4096));
> vring_legacy_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
On Thu, Apr 16, 2020 at 09:56:42AM +0200, Eugenio P?rez wrote:
> This way behavior for vhost is more like a VM.
>
> Signed-off-by: Eugenio P?rez <[email protected]>
I dropped --reset from 5.7 since Linus felt it's unappropriate.
I guess I should squash this in with --reset?
> ---
> tools/virtio/virtio_test.c | 33 ++++++++++++++++++++++++++-------
> 1 file changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
> index 18d5347003eb..dca64d36a882 100644
> --- a/tools/virtio/virtio_test.c
> +++ b/tools/virtio/virtio_test.c
> @@ -20,7 +20,6 @@
> #include "../../drivers/vhost/test.h"
>
> #define RANDOM_BATCH -1
> -#define RANDOM_RESET -1
>
> /* Unused */
> void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end;
> @@ -49,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)
> {
> @@ -174,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;
>
> @@ -224,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;
> }
> @@ -249,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";
> @@ -312,7 +333,7 @@ static void help(void)
> " [--no-virtio-1]"
> " [--delayed-interrupt]"
> " [--batch=random/N]"
> - " [--reset=random/N]"
> + " [--reset=N]"
> "\n");
> }
>
> @@ -360,11 +381,9 @@ int main(int argc, char **argv)
> case 'r':
> if (!optarg) {
> reset = 1;
> - } else if (0 == strcmp(optarg, "random")) {
> - reset = RANDOM_RESET;
> } else {
> reset = strtol(optarg, NULL, 10);
> - assert(reset >= 0);
> + assert(reset > 0);
> assert(reset < (long)INT_MAX + 1);
> }
> break;
> --
> 2.18.1
On Fri, Apr 17, 2020 at 12:34 AM Michael S. Tsirkin <[email protected]> wrote:
>
> On Thu, Apr 16, 2020 at 09:56:42AM +0200, Eugenio Pérez wrote:
> > This way behavior for vhost is more like a VM.
> >
> > Signed-off-by: Eugenio Pérez <[email protected]>
>
> I dropped --reset from 5.7 since Linus felt it's unappropriate.
> I guess I should squash this in with --reset?
>
Yes please.
If you prefer I can do it using the base you want, so all commits
messages are right.
Thanks!
> > ---
> > tools/virtio/virtio_test.c | 33 ++++++++++++++++++++++++++-------
> > 1 file changed, 26 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
> > index 18d5347003eb..dca64d36a882 100644
> > --- a/tools/virtio/virtio_test.c
> > +++ b/tools/virtio/virtio_test.c
> > @@ -20,7 +20,6 @@
> > #include "../../drivers/vhost/test.h"
> >
> > #define RANDOM_BATCH -1
> > -#define RANDOM_RESET -1
> >
> > /* Unused */
> > void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end;
> > @@ -49,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)
> > {
> > @@ -174,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;
> >
> > @@ -224,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;
> > }
> > @@ -249,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";
> > @@ -312,7 +333,7 @@ static void help(void)
> > " [--no-virtio-1]"
> > " [--delayed-interrupt]"
> > " [--batch=random/N]"
> > - " [--reset=random/N]"
> > + " [--reset=N]"
> > "\n");
> > }
> >
> > @@ -360,11 +381,9 @@ int main(int argc, char **argv)
> > case 'r':
> > if (!optarg) {
> > reset = 1;
> > - } else if (0 == strcmp(optarg, "random")) {
> > - reset = RANDOM_RESET;
> > } else {
> > reset = strtol(optarg, NULL, 10);
> > - assert(reset >= 0);
> > + assert(reset > 0);
> > assert(reset < (long)INT_MAX + 1);
> > }
> > break;
> > --
> > 2.18.1
>
On Fri, Apr 17, 2020 at 09:04:04AM +0200, Eugenio Perez Martin wrote:
> On Fri, Apr 17, 2020 at 12:34 AM Michael S. Tsirkin <[email protected]> wrote:
> >
> > On Thu, Apr 16, 2020 at 09:56:42AM +0200, Eugenio P?rez wrote:
> > > This way behavior for vhost is more like a VM.
> > >
> > > Signed-off-by: Eugenio P?rez <[email protected]>
> >
> > I dropped --reset from 5.7 since Linus felt it's unappropriate.
> > I guess I should squash this in with --reset?
> >
>
> Yes please.
>
> If you prefer I can do it using the base you want, so all commits
> messages are right.
>
> Thanks!
OK so I dropped new tests from vhost for now, pls rebased on
top of that.
Thanks!
> > > ---
> > > tools/virtio/virtio_test.c | 33 ++++++++++++++++++++++++++-------
> > > 1 file changed, 26 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
> > > index 18d5347003eb..dca64d36a882 100644
> > > --- a/tools/virtio/virtio_test.c
> > > +++ b/tools/virtio/virtio_test.c
> > > @@ -20,7 +20,6 @@
> > > #include "../../drivers/vhost/test.h"
> > >
> > > #define RANDOM_BATCH -1
> > > -#define RANDOM_RESET -1
> > >
> > > /* Unused */
> > > void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end;
> > > @@ -49,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)
> > > {
> > > @@ -174,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;
> > >
> > > @@ -224,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;
> > > }
> > > @@ -249,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";
> > > @@ -312,7 +333,7 @@ static void help(void)
> > > " [--no-virtio-1]"
> > > " [--delayed-interrupt]"
> > > " [--batch=random/N]"
> > > - " [--reset=random/N]"
> > > + " [--reset=N]"
> > > "\n");
> > > }
> > >
> > > @@ -360,11 +381,9 @@ int main(int argc, char **argv)
> > > case 'r':
> > > if (!optarg) {
> > > reset = 1;
> > > - } else if (0 == strcmp(optarg, "random")) {
> > > - reset = RANDOM_RESET;
> > > } else {
> > > reset = strtol(optarg, NULL, 10);
> > > - assert(reset >= 0);
> > > + assert(reset > 0);
> > > assert(reset < (long)INT_MAX + 1);
> > > }
> > > break;
> > > --
> > > 2.18.1
> >
On Fri, Apr 17, 2020 at 12:33 AM Michael S. Tsirkin <[email protected]> wrote:
>
> On Thu, Apr 16, 2020 at 09:56:40AM +0200, Eugenio Pérez wrote:
> > As updated in ("2a2d1382fe9d virtio: Add improved queue allocation API")
> >
> > Signed-off-by: Eugenio Pérez <[email protected]>
>
> Pls add motivation for these changes.
>
The original motivation was to make code as close as possible to
virtio_net. Also, it skips a (probably not expensive) initialization
in vring_new_virtqueue.
With the recent events, I think that this could be useful to test when
userspace and kernel use different struct layout, maybe with some
sanitizer. I can drop it if you don't see it the same way (or if I
didn't understand the problem and this does not help).
Thanks!
> > ---
> > 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 1d5144590df6..d9827b640c21 100644
> > --- a/tools/virtio/virtio_test.c
> > +++ b/tools/virtio/virtio_test.c
> > @@ -106,10 +106,9 @@ static void vq_info_add(struct vdev_info *dev, int num)
> > assert(r >= 0);
> > memset(info->ring, 0, vring_legacy_size(num, 4096));
> > vring_legacy_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
>