Messages in this thread Patch in this message | | | From | Rusty Russell <> | Subject | Re: [PATCH 4/4] virtio_ring: unify direct/indirect code paths. | Date | Fri, 30 May 2014 15:51:35 +0930 |
| |
Rusty Russell <rusty@rustcorp.com.au> writes: > "Michael S. Tsirkin" <mst@redhat.com> writes: >> On Thu, May 29, 2014 at 04:56:45PM +0930, Rusty Russell wrote: >>> virtqueue_add() populates the virtqueue descriptor table from the sgs >>> given. If it uses an indirect descriptor table, then it puts a single >>> descriptor in the descriptor table pointing to the kmalloc'ed indirect >>> table where the sg is populated. >>> + for (i = 0; i < total_sg; i++) >>> + desc[i].next = i+1; >>> + return desc; >> >> Hmm we are doing an extra walk over descriptors here. >> This might hurt performance esp for big descriptors. > > Yes, this needs to be benchmarked; since it's cache hot my gut feel is > that it's a NOOP, but on modern machines my gut feel is always wrong.
CC's trimmed.
Well, I was almost right about being wrong.
I wrote a userspace virtio_ring microbench which does 10000000 virtqueue_add_outbuf() calls (which go indirect) and not much else.
Read as <MIN>-<MAX>(<MEAN>+/-<STDDEV>): Current kernel: 936153354- 967745359(9.44739e+08+/-6.1e+06)ns Using sg_next: 1061485790-1104800648(1.08254e+09+/-6.6e+06)ns Unifying indirect path: 1214289435-1272686712(1.22564e+09+/-8e+06)ns Using indirect flag: 1125610268-1183528965(1.14172e+09+/-8e+06)ns
Of course this might be lost in the noise on real networking, so that's my job on Monday.
Subject: vring_bench: simple benchmark for adding descriptors to a virtqueue.
This userspace benchark uses the kernel code to add 8 16-element scatterlists to a virtqueue, then consume them and start again.
For example: $ for i in `seq 10`; do ./vring_bench; done | stats --trim-outliers 936153354-967745359(9.44739e+08+/-6.1e+06)ns 9999872 returned
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/tools/virtio/.gitignore b/tools/virtio/.gitignore index 1cfbb0157a46..ff32cca971d8 100644 --- a/tools/virtio/.gitignore +++ b/tools/virtio/.gitignore @@ -1,3 +1,4 @@ *.d virtio_test vringh_test +vring_bench diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile index 3187c62d9814..103101273049 100644 --- a/tools/virtio/Makefile +++ b/tools/virtio/Makefile @@ -1,6 +1,7 @@ all: test mod -test: virtio_test vringh_test +test: virtio_test vringh_test vring_bench virtio_test: virtio_ring.o virtio_test.o +vring_bench: virtio_ring.o vring_bench.o vringh_test: vringh_test.o vringh.o virtio_ring.o CFLAGS += -g -O2 -Wall -I. -I ../../usr/include/ -Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE @@ -9,6 +10,6 @@ mod: ${MAKE} -C `pwd`/../.. M=`pwd`/vhost_test .PHONY: all test mod clean clean: - ${RM} *.o vringh_test virtio_test vhost_test/*.o vhost_test/.*.cmd \ + ${RM} *.o vringh_test virtio_test vring_bench vhost_test/*.o vhost_test/.*.cmd \ vhost_test/Module.symvers vhost_test/modules.order *.d -include *.d diff --git a/tools/virtio/linux/kernel.h b/tools/virtio/linux/kernel.h index fba705963968..8dcff8e3374c 100644 --- a/tools/virtio/linux/kernel.h +++ b/tools/virtio/linux/kernel.h @@ -109,4 +109,7 @@ static inline void free_page(unsigned long addr) (void) (&_min1 == &_min2); \ _min1 < _min2 ? _min1 : _min2; }) +/* Just make it compile */ +#define list_for_each_entry(iter, list, member) + #endif /* KERNEL_H */ diff --git a/tools/virtio/vring_bench.c b/tools/virtio/vring_bench.c new file mode 100644 index 000000000000..0d7544fd26ad --- /dev/null +++ b/tools/virtio/vring_bench.c @@ -0,0 +1,125 @@ +#define _GNU_SOURCE +#include <time.h> +#include <getopt.h> +#include <string.h> +#include <poll.h> +#include <sys/eventfd.h> +#include <stdlib.h> +#include <assert.h> +#include <unistd.h> +#include <sys/ioctl.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <fcntl.h> +#include <stdbool.h> +#include <linux/virtio.h> +#include <linux/virtio_ring.h> + +/* Unused */ +void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end; + +static struct vring vring; +static uint16_t last_avail_idx; +static unsigned int returned; + +static bool vq_notify(struct virtqueue *vq) +{ + /* "Use" them all. */ + while (vring.avail->idx != last_avail_idx) { + unsigned int i, head; + + i = last_avail_idx++ & (vring.num - 1); + head = vring.avail->ring[i]; + assert(head < vring.num); + + i = vring.used->idx & (vring.num - 1); + vring.used->ring[i].id = head; + vring.used->ring[i].len = 0; + vring.used->idx++; + } + return true; +} + +static void vq_callback(struct virtqueue *vq) +{ + unsigned int len; + void *p; + + while ((p = virtqueue_get_buf(vq, &len)) != NULL) + returned++; +} + +/* Ring size 128, just like qemu uses */ +#define VRING_NUM 128 +#define SG_SIZE 16 + +static inline struct timespec time_sub(struct timespec recent, + struct timespec old) +{ + struct timespec diff; + + diff.tv_sec = recent.tv_sec - old.tv_sec; + if (old.tv_nsec > recent.tv_nsec) { + diff.tv_sec--; + diff.tv_nsec = 1000000000 + recent.tv_nsec - old.tv_nsec; + } else + diff.tv_nsec = recent.tv_nsec - old.tv_nsec; + + return diff; +} + +static struct timespec time_now(void) +{ + struct timespec ret; + clock_gettime(CLOCK_REALTIME, &ret); + return ret; +} + +static inline uint64_t time_to_nsec(struct timespec t) +{ + uint64_t nsec; + + nsec = t.tv_nsec + (uint64_t)t.tv_sec * 1000000000; + return nsec; +} + +int main(int argc, char *argv[]) +{ + struct virtqueue *vq; + struct virtio_device vdev; + void *ring; + unsigned int i, num; + int e; + struct scatterlist sg[SG_SIZE]; + struct timespec start; + + sg_init_table(sg, SG_SIZE); + + e = posix_memalign(&ring, 4096, vring_size(VRING_NUM, 4096)); + assert(e >= 0); + + vdev.features[0] = (1UL << VIRTIO_RING_F_INDIRECT_DESC) | + (1UL << VIRTIO_RING_F_EVENT_IDX); + + vq = vring_new_virtqueue(0, VRING_NUM, 4096, &vdev, true, ring, + vq_notify, vq_callback, "benchmark"); + assert(vq); + vring_init(&vring, VRING_NUM, ring, 4096); + + num = atoi(argv[1] ?: "10000000"); + + start = time_now(); + for (i = 0; i < num; i++) { + again: + e = virtqueue_add_outbuf(vq, sg, SG_SIZE, sg, GFP_ATOMIC); + if (e < 0) { + virtqueue_kick(vq); + vring_interrupt(0, vq); + goto again; + } + } + printf("%lluns\n", + (long long)time_to_nsec(time_sub(time_now(), start))); + printf("%u returned\n", returned); + return 0; +}
| |