Re: [PATCH v2 1/5] virtio: add functions for piecewise addition of buffers
flat view
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2012-12-18 13:33:10
Also in:
kvm, linux-scsi, lkml
Some comments without arguing about whether the performance benefit is worth it. On Tue, Dec 18, 2012 at 01:32:48PM +0100, Paolo Bonzini wrote:
quoted hunk ↗ jump to hunk
diff --git a/include/linux/virtio.h b/include/linux/virtio.h index cf8adb1..39d56c4 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h@@ -7,6 +7,7 @@ #include <linux/spinlock.h> #include <linux/device.h> #include <linux/mod_devicetable.h> +#include <linux/dma-direction.h> #include <linux/gfp.h> /**@@ -40,6 +41,26 @@ int virtqueue_add_buf(struct virtqueue *vq, void *data, gfp_t gfp); +struct virtqueue_buf { + struct virtqueue *vq; + struct vring_desc *indirect, *tail;
This is wrong: virtio.h does not include virito_ring.h, and it shouldn't by design depend on it.
quoted hunk ↗ jump to hunk
+ int head; +}; +
Can't we track state internally to the virtqueue? Exposing it seems to buy us nothing since you can't call add_buf between start and end anyway.
quoted hunk ↗ jump to hunk
+int virtqueue_start_buf(struct virtqueue *_vq, + struct virtqueue_buf *buf, + void *data, + unsigned int count, + unsigned int count_sg, + gfp_t gfp); + +void virtqueue_add_sg(struct virtqueue_buf *buf, + struct scatterlist sgl[], + unsigned int count, + enum dma_data_direction dir); +
And idea: in practice virtio scsi seems to always call sg_init_one, no? So how about we pass in void* or something and avoid using sg and count? This would make it useful for -net BTW.
+void virtqueue_end_buf(struct virtqueue_buf *buf); + void virtqueue_kick(struct virtqueue *vq); bool virtqueue_kick_prepare(struct virtqueue *vq); -- 1.7.1