Original virtio_test only use add one descriptor for each io event, thus code of descriptor chain and indirection have not been tested(one descriptor will not use indirect feature even indirect feature has been specified). In fact it would have not been possible for vhost_test to access to the indirect descriptor table, because it's impossible for virtio_ring.c to allocate it.
This series using descriptor chain and enable indirection feature. And through gcov we find the code coverage has been improved(not high for virtio_ring.c because virtio_test only test split virtqueue):
+------------+-------------+-------------+
| |virtio_test.c|virtio_ring.c|
+------------+-------------+-------------+
| original | 72.32% | 24.71% |
+------------+-------------+-------------+
| current | 75% | 28.05% |
+------------+-------------+-------------+
Guo Zhi (4):
virtio_test: kick vhost for a batch of descriptors
virtio_test: move magic number in code as defined constant
virtio_test: use random length scatterlists to test descriptor chain
virtio_test: enable indirection feature
tools/virtio/virtio_test.c | 86 ++++++++++++++++++++++++++++----------
1 file changed, 64 insertions(+), 22 deletions(-)
--
2.17.1
Prior implementation only use one descriptor for each io event, which
does't test code of descriptor chain. More importantly, one descriptor
will not use indirect feature even indirect feature is specified. Use
random length scatterlists here to test descriptor chain.
Signed-off-by: Guo Zhi <[email protected]>
---
tools/virtio/virtio_test.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 1ecd64271..363695b33 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -20,6 +20,7 @@
#include "../../drivers/vhost/test.h"
#define RANDOM_BATCH -1
+#define MAX_SG_FRAGS 8UL
#define ALIGN 4096
#define RINGSIZE 256
#define TEST_BUF_NUM 0x100000
@@ -172,7 +173,8 @@ 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 reset_n, int bufs)
{
- struct scatterlist sl;
+ struct scatterlist sg[MAX_SG_FRAGS];
+ int sg_size = 0;
long started = 0, completed = 0, next_reset = reset_n;
long completed_before, started_before;
int r, test = 1;
@@ -197,8 +199,11 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
while (started < bufs &&
(started - completed) < batch) {
- sg_init_one(&sl, dev->buf, dev->buf_size);
- r = virtqueue_add_outbuf(vq->vq, &sl, 1,
+ sg_size = random() % (MAX_SG_FRAGS - 1) + 1;
+ sg_init_table(sg, sg_size);
+ for (int i = 0; i < sg_size; ++i)
+ sg_set_buf(&sg[i], dev->buf + i, 0x1);
+ r = virtqueue_add_outbuf(vq->vq, sg, sg_size,
dev->buf + started,
GFP_ATOMIC);
if (unlikely(r != 0)) {
--
2.17.1
Only kick vhost when the batch finishes.
Signed-off-by: Guo Zhi <[email protected]>
---
tools/virtio/virtio_test.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 23f142af5..95f78b311 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -208,11 +208,10 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
}
++started;
-
- if (unlikely(!virtqueue_kick(vq->vq))) {
- r = -1;
- break;
- }
+ }
+ if (unlikely(!virtqueue_kick(vq->vq))) {
+ r = -1;
+ break;
}
if (started >= bufs)
--
2.17.1
On Thu, Jul 07, 2022 at 10:44:08AM +0800, Guo Zhi wrote:
> Prior implementation only use one descriptor for each io event, which
> does't test code of descriptor chain. More importantly, one descriptor
> will not use indirect feature even indirect feature is specified. Use
> random length scatterlists here to test descriptor chain.
>
> Signed-off-by: Guo Zhi <[email protected]>
> ---
> tools/virtio/virtio_test.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
> index 1ecd64271..363695b33 100644
> --- a/tools/virtio/virtio_test.c
> +++ b/tools/virtio/virtio_test.c
> @@ -20,6 +20,7 @@
> #include "../../drivers/vhost/test.h"
>
> #define RANDOM_BATCH -1
> +#define MAX_SG_FRAGS 8UL
> #define ALIGN 4096
> #define RINGSIZE 256
> #define TEST_BUF_NUM 0x100000
> @@ -172,7 +173,8 @@ 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 reset_n, int bufs)
> {
> - struct scatterlist sl;
> + struct scatterlist sg[MAX_SG_FRAGS];
> + int sg_size = 0;
> long started = 0, completed = 0, next_reset = reset_n;
> long completed_before, started_before;
> int r, test = 1;
> @@ -197,8 +199,11 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
>
> while (started < bufs &&
> (started - completed) < batch) {
> - sg_init_one(&sl, dev->buf, dev->buf_size);
> - r = virtqueue_add_outbuf(vq->vq, &sl, 1,
> + sg_size = random() % (MAX_SG_FRAGS - 1) + 1;
> + sg_init_table(sg, sg_size);
> + for (int i = 0; i < sg_size; ++i)
> + sg_set_buf(&sg[i], dev->buf + i, 0x1);
> + r = virtqueue_add_outbuf(vq->vq, sg, sg_size,
> dev->buf + started,
> GFP_ATOMIC);
> if (unlikely(r != 0)) {
random on data path is pretty expensive.
I would suggest get an array size from user (and maybe a seed?) and
pregenerate some numbers, then reuse.
> --
> 2.17.1
On 2022/7/7 13:16, Michael S. Tsirkin wrote:
> On Thu, Jul 07, 2022 at 10:44:08AM +0800, Guo Zhi wrote:
>> Prior implementation only use one descriptor for each io event, which
>> does't test code of descriptor chain. More importantly, one descriptor
>> will not use indirect feature even indirect feature is specified. Use
>> random length scatterlists here to test descriptor chain.
>>
>> Signed-off-by: Guo Zhi <[email protected]>
>> ---
>> tools/virtio/virtio_test.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
>> index 1ecd64271..363695b33 100644
>> --- a/tools/virtio/virtio_test.c
>> +++ b/tools/virtio/virtio_test.c
>> @@ -20,6 +20,7 @@
>> #include "../../drivers/vhost/test.h"
>>
>> #define RANDOM_BATCH -1
>> +#define MAX_SG_FRAGS 8UL
>> #define ALIGN 4096
>> #define RINGSIZE 256
>> #define TEST_BUF_NUM 0x100000
>> @@ -172,7 +173,8 @@ 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 reset_n, int bufs)
>> {
>> - struct scatterlist sl;
>> + struct scatterlist sg[MAX_SG_FRAGS];
>> + int sg_size = 0;
>> long started = 0, completed = 0, next_reset = reset_n;
>> long completed_before, started_before;
>> int r, test = 1;
>> @@ -197,8 +199,11 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
>>
>> while (started < bufs &&
>> (started - completed) < batch) {
>> - sg_init_one(&sl, dev->buf, dev->buf_size);
>> - r = virtqueue_add_outbuf(vq->vq, &sl, 1,
>> + sg_size = random() % (MAX_SG_FRAGS - 1) + 1;
>> + sg_init_table(sg, sg_size);
>> + for (int i = 0; i < sg_size; ++i)
>> + sg_set_buf(&sg[i], dev->buf + i, 0x1);
>> + r = virtqueue_add_outbuf(vq->vq, sg, sg_size,
>> dev->buf + started,
>> GFP_ATOMIC);
>> if (unlikely(r != 0)) {
> random on data path is pretty expensive.
> I would suggest get an array size from user (and maybe a seed?) and
> pregenerate some numbers, then reuse.
SGTM, I will prepare a random number array before add outbuf begin.
>
>> --
>> 2.17.1