Skip to contentSkip to conversation

virtio: virtqueue ops cleanup

PATCH In review Michael S. Tsirkin 2 participants 12 messages Apr 12, 2010 – Apr 20, 2010

Subsystems (11)
Network drivers Block layer Char and misc drivers virtio core +7 more subsystems
lore
Share & tools
lore

Michael S. Tsirkin

author maintainer
#0 lore raw

cover virtio: virtqueue ops cleanup

virtqueue ops were introduced in the hope that we'll have multiple implementations besides virtio_ring, but none have surfaced so far, and given that existing virtio ring is deployed in production we are likely stuck with it now, so this layer just adds complexity and overhead.
Further, the need to pass vq twice to each call (as in dev->vq->vq_ops->kick(dev->vq) ) adds potential for cut and paste errors.

This patchset does the following:
- add inline wrappers converting the above to virtqueue_kick(dev->vq)
- convert all users to this API
- remove vq_ops indirection, implementing virtqueue_xx directly in virtio_ring

Note that if we ever want to add another vring implementation, we'll only need to revert the last patch in the series, devices won't have to change.

Signature · 18 lines
MST


Michael S. Tsirkin (6):
  virtio: add virtqueue_ vq_ops wrappers
  virtio_balloon: use virtqueue_xxx wrappers
  virtio_console: use virtqueue_xxx wrappers
  virtio_blk: use virtqueue_xxx wrappers
  virtio_net: use virtqueue_xxx wrappers
  virtio_ring: remove a level of indirection

 drivers/block/virtio_blk.c      |    6 ++--
 drivers/char/virtio_console.c   |   32 +++++++++++++-------------
 drivers/net/virtio_net.c        |   46 +++++++++++++++++++-------------------
 drivers/virtio/virtio_balloon.c |   17 ++++++-------
 drivers/virtio/virtio_ring.c    |   36 +++++++++++++-----------------
 include/linux/virtio.h          |   43 +++++++++++++++++------------------
 6 files changed, 87 insertions(+), 93 deletions(-)
author maintainer
#1 lore raw

PATCH 1/6 virtio: add virtqueue_ vq_ops wrappers

Add inline functions that wrap vq->vq_ops-> calls

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 include/linux/virtio.h |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)
include/linux/virtio.h +34 -0
index 40d1709..18ccc68 100644
@@ -77,6 +77,40 @@ struct virtqueue_ops {
 	void *(*detach_unused_buf)(struct virtqueue *vq);
 };
 
+static inline int virtqueue_add_buf(struct virtqueue *vq,
+				    struct scatterlist sg[],
+				    unsigned int out_num,
+				    unsigned int in_num,
+				    void *data)
+{
+	return vq->vq_ops->add_buf(vq, sg, out_num, in_num, data);
+}
+
+static inline int void virtqueue_kick(struct virtqueue *vq)
+{
+	return vq->vq_ops->kick(vq);
+}
+
+static inline void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len)
+{
+	return vq->vq_ops->get_buf(vq, len);
+}
+
+static inline void virtqueue_disable_cb(struct virtqueue *vq)
+{
+	vq->vq_ops->disable_cb(vq);
+}
+
+static inline bool virtqueue_enable_cb(struct virtqueue *vq)
+{
+	return vq->vq_ops->enable_cb(vq);
+}
+
+static inline void *virtqueue_detach_unused_buf(struct virtqueue *vq)
+{
+	return vq->vq_ops->detach_unused_buf(vq);
+}
+
 /**
  * virtio_device - representation of a device using virtio
  * @index: unique position on the virtio bus
Signature · 1 line
1.7.0.2.280.gc6f05
author maintainer
#2 lore raw

PATCH 2/6 virtio_balloon: use virtqueue_xxx wrappers

In reply to Michael S. Tsirkin

Switch virtio_balloon to new virtqueue_xxx wrappers.

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 drivers/virtio/virtio_balloon.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)
drivers/virtio/virtio_balloon.c +8 -9
index 3aed388..6caab1a 100644
@@ -75,7 +75,7 @@ static void balloon_ack(struct virtqueue *vq)
 	struct virtio_balloon *vb;
 	unsigned int len;
 
-	vb = vq->vq_ops->get_buf(vq, &len);
+	vb = virtqueue_get_buf(vq, &len);
 	if (vb)
 		complete(&vb->acked);
 }

@@ -89,9 +89,9 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
 	init_completion(&vb->acked);
 
 	/* We should always be able to add one buffer to an empty queue. */
-	if (vq->vq_ops->add_buf(vq, &sg, 1, 0, vb) < 0)
+	if (virtqueue_add_buf(vq, &sg, 1, 0, vb) < 0)
 		BUG();
-	vq->vq_ops->kick(vq);
+	virtqueue_kick(vq);
 
 	/* When host has read buffer, this completes via balloon_ack */
 	wait_for_completion(&vb->acked);

@@ -203,7 +203,7 @@ static void stats_request(struct virtqueue *vq)
 	struct virtio_balloon *vb;
 	unsigned int len;
 
-	vb = vq->vq_ops->get_buf(vq, &len);
+	vb = virtqueue_get_buf(vq, &len);
 	if (!vb)
 		return;
 	vb->need_stats_update = 1;

@@ -220,9 +220,9 @@ static void stats_handle_request(struct virtio_balloon *vb)
 
 	vq = vb->stats_vq;
 	sg_init_one(&sg, vb->stats, sizeof(vb->stats));
-	if (vq->vq_ops->add_buf(vq, &sg, 1, 0, vb) < 0)
+	if (virtqueue_add_buf(vq, &sg, 1, 0, vb) < 0)
 		BUG();
-	vq->vq_ops->kick(vq);
+	virtqueue_kick(vq);
 }
 
 static void virtballoon_changed(struct virtio_device *vdev)

@@ -313,10 +313,9 @@ static int virtballoon_probe(struct virtio_device *vdev)
 		 * use it to signal us later.
 		 */
 		sg_init_one(&sg, vb->stats, sizeof vb->stats);
-		if (vb->stats_vq->vq_ops->add_buf(vb->stats_vq,
-						  &sg, 1, 0, vb) < 0)
+		if (virtqueue_add_buf(vb->stats_vq, &sg, 1, 0, vb) < 0)
 			BUG();
-		vb->stats_vq->vq_ops->kick(vb->stats_vq);
+		virtqueue_kick(vb->stats_vq);
 	}
 
 	vb->thread = kthread_run(balloon, vb, "vballoon");
Signature · 1 line
1.7.0.2.280.gc6f05
author maintainer
#3 lore raw

PATCH 3/6 virtio_console: use virtqueue_xxx wrappers

In reply to Michael S. Tsirkin

Switch virtio_console to new virtqueue_xxx wrappers.

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 drivers/char/virtio_console.c |   32 ++++++++++++++++----------------
 1 files changed, 16 insertions(+), 16 deletions(-)
drivers/char/virtio_console.c +16 -16
index 196428c..48ce834 100644
@@ -328,7 +328,7 @@ static void *get_inbuf(struct port *port)
 	unsigned int len;
 
 	vq = port->in_vq;
-	buf = vq->vq_ops->get_buf(vq, &len);
+	buf = virtqueue_get_buf(vq, &len);
 	if (buf) {
 		buf->len = len;
 		buf->offset = 0;

@@ -349,8 +349,8 @@ static int add_inbuf(struct virtqueue *vq, struct port_buffer *buf)
 
 	sg_init_one(sg, buf->buf, buf->size);
 
-	ret = vq->vq_ops->add_buf(vq, sg, 0, 1, buf);
-	vq->vq_ops->kick(vq);
+	ret = virtqueue_add_buf(vq, sg, 0, 1, buf);
+	virtqueue_kick(vq);
 	return ret;
 }
 

@@ -366,7 +366,7 @@ static void discard_port_data(struct port *port)
 	if (port->inbuf)
 		buf = port->inbuf;
 	else
-		buf = vq->vq_ops->get_buf(vq, &len);
+		buf = virtqueue_get_buf(vq, &len);
 
 	ret = 0;
 	while (buf) {

@@ -374,7 +374,7 @@ static void discard_port_data(struct port *port)
 			ret++;
 			free_buf(buf);
 		}
-		buf = vq->vq_ops->get_buf(vq, &len);
+		buf = virtqueue_get_buf(vq, &len);
 	}
 	port->inbuf = NULL;
 	if (ret)

@@ -421,9 +421,9 @@ static ssize_t send_control_msg(struct port *port, unsigned int event,
 	vq = port->portdev->c_ovq;
 
 	sg_init_one(sg, &cpkt, sizeof(cpkt));
-	if (vq->vq_ops->add_buf(vq, sg, 1, 0, &cpkt) >= 0) {
-		vq->vq_ops->kick(vq);
-		while (!vq->vq_ops->get_buf(vq, &len))
+	if (virtqueue_add_buf(vq, sg, 1, 0, &cpkt) >= 0) {
+		virtqueue_kick(vq);
+		while (!virtqueue_get_buf(vq, &len))
 			cpu_relax();
 	}
 	return 0;

@@ -439,10 +439,10 @@ static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
 	out_vq = port->out_vq;
 
 	sg_init_one(sg, in_buf, in_count);
-	ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, in_buf);
+	ret = virtqueue_add_buf(out_vq, sg, 1, 0, in_buf);
 
 	/* Tell Host to go! */
-	out_vq->vq_ops->kick(out_vq);
+	virtqueue_kick(out_vq);
 
 	if (ret < 0) {
 		in_count = 0;

@@ -450,7 +450,7 @@ static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
 	}
 
 	/* Wait till the host acknowledges it pushed out the data we sent. */
-	while (!out_vq->vq_ops->get_buf(out_vq, &len))
+	while (!virtqueue_get_buf(out_vq, &len))
 		cpu_relax();
 fail:
 	/* We're expected to return the amount of data we wrote */

@@ -901,7 +901,7 @@ static int remove_port(struct port *port)
 	discard_port_data(port);
 
 	/* Remove buffers we queued up for the Host to send us data in. */
-	while ((buf = port->in_vq->vq_ops->detach_unused_buf(port->in_vq)))
+	while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
 		free_buf(buf);
 
 	kfree(port->name);

@@ -1030,7 +1030,7 @@ static void control_work_handler(struct work_struct *work)
 	vq = portdev->c_ivq;
 
 	spin_lock(&portdev->cvq_lock);
-	while ((buf = vq->vq_ops->get_buf(vq, &len))) {
+	while ((buf = virtqueue_get_buf(vq, &len))) {
 		spin_unlock(&portdev->cvq_lock);
 
 		buf->len = len;

@@ -1224,7 +1224,7 @@ static int add_port(struct ports_device *portdev, u32 id)
 	return 0;
 
 free_inbufs:
-	while ((buf = port->in_vq->vq_ops->detach_unused_buf(port->in_vq)))
+	while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
 		free_buf(buf);
 free_device:
 	device_destroy(pdrvdata.class, port->dev->devt);

@@ -1536,10 +1536,10 @@ static void virtcons_remove(struct virtio_device *vdev)
 
 	unregister_chrdev(portdev->chr_major, "virtio-portsdev");
 
-	while ((buf = portdev->c_ivq->vq_ops->get_buf(portdev->c_ivq, &len)))
+	while ((buf = virtqueue_get_buf(portdev->c_ivq, &len)))
 		free_buf(buf);
 
-	while ((buf = portdev->c_ivq->vq_ops->detach_unused_buf(portdev->c_ivq)))
+	while ((buf = virtqueue_detach_unused_buf(portdev->c_ivq)))
 		free_buf(buf);
 
 	vdev->config->del_vqs(vdev);
Signature · 1 line
1.7.0.2.280.gc6f05
author maintainer
#4 lore raw

PATCH 4/6 virtio_blk: use virtqueue_xxx wrappers

In reply to Michael S. Tsirkin

Switch virtio_blk to new virtqueue_xxx wrappers.

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 drivers/block/virtio_blk.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/block/virtio_blk.c +3 -3
index 2138a7a..1fb4f20 100644
@@ -50,7 +50,7 @@ static void blk_done(struct virtqueue *vq)
 	unsigned long flags;
 
 	spin_lock_irqsave(&vblk->lock, flags);
-	while ((vbr = vblk->vq->vq_ops->get_buf(vblk->vq, &len)) != NULL) {
+	while ((vbr = virtqueue_get_buf(vblk->vq, &len)) != NULL) {
 		int error;
 
 		switch (vbr->status) {

@@ -151,7 +151,7 @@ static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
 		}
 	}
 
-	if (vblk->vq->vq_ops->add_buf(vblk->vq, vblk->sg, out, in, vbr) < 0) {
+	if (virtqueue_add_buf(vblk->vq, vblk->sg, out, in, vbr) < 0) {
 		mempool_free(vbr, vblk->pool);
 		return false;
 	}

@@ -180,7 +180,7 @@ static void do_virtblk_request(struct request_queue *q)
 	}
 
 	if (issued)
-		vblk->vq->vq_ops->kick(vblk->vq);
+		virtqueue_kick(vblk->vq);
 }
 
 static void virtblk_prepare_flush(struct request_queue *q, struct request *req)
Signature · 1 line
1.7.0.2.280.gc6f05
author maintainer
#5 lore raw

PATCH 5/6 virtio_net: use virtqueue_xxx wrappers

In reply to Michael S. Tsirkin

Switch virtio_net to new virtqueue_xxx wrappers.

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 drivers/net/virtio_net.c |   46 +++++++++++++++++++++++-----------------------
 1 files changed, 23 insertions(+), 23 deletions(-)
drivers/net/virtio_net.c +23 -23
index 6fb783c..e00944b 100644
@@ -119,7 +119,7 @@ static void skb_xmit_done(struct virtqueue *svq)
 	struct virtnet_info *vi = svq->vdev->priv;
 
 	/* Suppress further interrupts. */
-	svq->vq_ops->disable_cb(svq);
+	virtqueue_disable_cb(svq);
 
 	/* We were probably waiting for more output buffers. */
 	netif_wake_queue(vi->dev);

@@ -207,7 +207,7 @@ static int receive_mergeable(struct virtnet_info *vi, struct sk_buff *skb)
 			return -EINVAL;
 		}
 
-		page = vi->rvq->vq_ops->get_buf(vi->rvq, &len);
+		page = virtqueue_get_buf(vi->rvq, &len);
 		if (!page) {
 			pr_debug("%s: rx error: %d buffers missing\n",
 				 skb->dev->name, hdr->mhdr.num_buffers);

@@ -338,7 +338,7 @@ static int add_recvbuf_small(struct virtnet_info *vi, gfp_t gfp)
 
 	skb_to_sgvec(skb, sg + 1, 0, skb->len);
 
-	err = vi->rvq->vq_ops->add_buf(vi->rvq, sg, 0, 2, skb);
+	err = virtqueue_add_buf(vi->rvq, sg, 0, 2, skb);
 	if (err < 0)
 		dev_kfree_skb(skb);
 

@@ -384,7 +384,7 @@ static int add_recvbuf_big(struct virtnet_info *vi, gfp_t gfp)
 
 	/* chain first in list head */
 	first->private = (unsigned long)list;
-	err = vi->rvq->vq_ops->add_buf(vi->rvq, sg, 0, MAX_SKB_FRAGS + 2,
+	err = virtqueue_add_buf(vi->rvq, sg, 0, MAX_SKB_FRAGS + 2,
 				       first);
 	if (err < 0)
 		give_pages(vi, first);

@@ -404,7 +404,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, gfp_t gfp)
 
 	sg_init_one(&sg, page_address(page), PAGE_SIZE);
 
-	err = vi->rvq->vq_ops->add_buf(vi->rvq, &sg, 0, 1, page);
+	err = virtqueue_add_buf(vi->rvq, &sg, 0, 1, page);
 	if (err < 0)
 		give_pages(vi, page);
 

@@ -433,7 +433,7 @@ static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
 	} while (err > 0);
 	if (unlikely(vi->num > vi->max))
 		vi->max = vi->num;
-	vi->rvq->vq_ops->kick(vi->rvq);
+	virtqueue_kick(vi->rvq);
 	return !oom;
 }
 

@@ -442,7 +442,7 @@ static void skb_recv_done(struct virtqueue *rvq)
 	struct virtnet_info *vi = rvq->vdev->priv;
 	/* Schedule NAPI, Suppress further interrupts if successful. */
 	if (napi_schedule_prep(&vi->napi)) {
-		rvq->vq_ops->disable_cb(rvq);
+		virtqueue_disable_cb(rvq);
 		__napi_schedule(&vi->napi);
 	}
 }

@@ -471,7 +471,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
 
 again:
 	while (received < budget &&
-	       (buf = vi->rvq->vq_ops->get_buf(vi->rvq, &len)) != NULL) {
+	       (buf = virtqueue_get_buf(vi->rvq, &len)) != NULL) {
 		receive_buf(vi->dev, buf, len);
 		--vi->num;
 		received++;

@@ -485,9 +485,9 @@ again:
 	/* Out of packets? */
 	if (received < budget) {
 		napi_complete(napi);
-		if (unlikely(!vi->rvq->vq_ops->enable_cb(vi->rvq)) &&
+		if (unlikely(!virtqueue_enable_cb(vi->rvq)) &&
 		    napi_schedule_prep(napi)) {
-			vi->rvq->vq_ops->disable_cb(vi->rvq);
+			virtqueue_disable_cb(vi->rvq);
 			__napi_schedule(napi);
 			goto again;
 		}

@@ -501,7 +501,7 @@ static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
 	struct sk_buff *skb;
 	unsigned int len, tot_sgs = 0;
 
-	while ((skb = vi->svq->vq_ops->get_buf(vi->svq, &len)) != NULL) {
+	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
 		pr_debug("Sent skb %p\n", skb);
 		vi->dev->stats.tx_bytes += skb->len;
 		vi->dev->stats.tx_packets++;

@@ -557,7 +557,7 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
 		sg_set_buf(sg, &hdr->hdr, sizeof hdr->hdr);
 
 	hdr->num_sg = skb_to_sgvec(skb, sg+1, 0, skb->len) + 1;
-	return vi->svq->vq_ops->add_buf(vi->svq, sg, hdr->num_sg, 0, skb);
+	return virtqueue_add_buf(vi->svq, sg, hdr->num_sg, 0, skb);
 }
 
 static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)

@@ -576,14 +576,14 @@ again:
 	if (unlikely(capacity < 0)) {
 		netif_stop_queue(dev);
 		dev_warn(&dev->dev, "Unexpected full queue\n");
-		if (unlikely(!vi->svq->vq_ops->enable_cb(vi->svq))) {
-			vi->svq->vq_ops->disable_cb(vi->svq);
+		if (unlikely(!virtqueue_enable_cb(vi->svq))) {
+			virtqueue_disable_cb(vi->svq);
 			netif_start_queue(dev);
 			goto again;
 		}
 		return NETDEV_TX_BUSY;
 	}
-	vi->svq->vq_ops->kick(vi->svq);
+	virtqueue_kick(vi->svq);
 
 	/* Don't wait up for transmitted skbs to be freed. */
 	skb_orphan(skb);

@@ -593,12 +593,12 @@ again:
 	 * before it gets out of hand.  Naturally, this wastes entries. */
 	if (capacity < 2+MAX_SKB_FRAGS) {
 		netif_stop_queue(dev);
-		if (unlikely(!vi->svq->vq_ops->enable_cb(vi->svq))) {
+		if (unlikely(!virtqueue_enable_cb(vi->svq))) {
 			/* More just got used, free them then recheck. */
 			capacity += free_old_xmit_skbs(vi);
 			if (capacity >= 2+MAX_SKB_FRAGS) {
 				netif_start_queue(dev);
-				vi->svq->vq_ops->disable_cb(vi->svq);
+				virtqueue_disable_cb(vi->svq);
 			}
 		}
 	}

@@ -643,7 +643,7 @@ static int virtnet_open(struct net_device *dev)
 	 * now.  virtnet_poll wants re-enable the queue, so we disable here.
 	 * We synchronize against interrupts via NAPI_STATE_SCHED */
 	if (napi_schedule_prep(&vi->napi)) {
-		vi->rvq->vq_ops->disable_cb(vi->rvq);
+		virtqueue_disable_cb(vi->rvq);
 		__napi_schedule(&vi->napi);
 	}
 	return 0;

@@ -680,15 +680,15 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
 		sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
 	sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
 
-	BUG_ON(vi->cvq->vq_ops->add_buf(vi->cvq, sg, out, in, vi) < 0);
+	BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi) < 0);
 
-	vi->cvq->vq_ops->kick(vi->cvq);
+	virtqueue_kick(vi->cvq);
 
 	/*
 	 * Spin for a response, the kick causes an ioport write, trapping
 	 * into the hypervisor, so the request should be handled immediately.
 	 */
-	while (!vi->cvq->vq_ops->get_buf(vi->cvq, &tmp))
+	while (!virtqueue_get_buf(vi->cvq, &tmp))
 		cpu_relax();
 
 	return status == VIRTIO_NET_OK;

@@ -1004,13 +1004,13 @@ static void free_unused_bufs(struct virtnet_info *vi)
 {
 	void *buf;
 	while (1) {
-		buf = vi->svq->vq_ops->detach_unused_buf(vi->svq);
+		buf = virtqueue_detach_unused_buf(vi->svq);
 		if (!buf)
 			break;
 		dev_kfree_skb(buf);
 	}
 	while (1) {
-		buf = vi->rvq->vq_ops->detach_unused_buf(vi->rvq);
+		buf = virtqueue_detach_unused_buf(vi->rvq);
 		if (!buf)
 			break;
 		if (vi->mergeable_rx_bufs || vi->big_packets)
Signature · 1 line
1.7.0.2.280.gc6f05
author maintainer
#6 lore raw

PATCH 6/6 virtio_ring: remove a level of indirection

In reply to Michael S. Tsirkin

We have a single virtqueue_ops implementation, and it seems unlikely we'll get another one at this point. So let's remove an unnecessary level of indirection: it would be very easy to re-add it if another implementation surfaces.

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 drivers/virtio/virtio_ring.c |   36 +++++++++------------
 include/linux/virtio.h       |   71 ++++++++++-------------------------------
 2 files changed, 34 insertions(+), 73 deletions(-)
drivers/virtio/virtio_ring.c +16 -20
index 0f90634..0717b5b 100644
@@ -155,11 +155,11 @@ static int vring_add_indirect(struct vring_virtqueue *vq,
 	return head;
 }
 
-static int vring_add_buf(struct virtqueue *_vq,
-			 struct scatterlist sg[],
-			 unsigned int out,
-			 unsigned int in,
-			 void *data)
+int virtqueue_add_buf(struct virtqueue *_vq,
+		  struct scatterlist sg[],
+		  unsigned int out,
+		  unsigned int in,
+		  void *data)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 	unsigned int i, avail, head, uninitialized_var(prev);

@@ -232,8 +232,9 @@ add_head:
 		return vq->num_free ? vq->vring.num : 0;
 	return vq->num_free;
 }
+EXPORT_SYMBOL_GPL(virtqueue_add_buf);
 
-static void vring_kick(struct virtqueue *_vq)
+void virtqueue_kick(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 	START_USE(vq);

@@ -253,6 +254,7 @@ static void vring_kick(struct virtqueue *_vq)
 
 	END_USE(vq);
 }
+EXPORT_SYMBOL_GPL(virtqueue_kick);
 
 static void detach_buf(struct vring_virtqueue *vq, unsigned int head)
 {

@@ -284,7 +286,7 @@ static inline bool more_used(const struct vring_virtqueue *vq)
 	return vq->last_used_idx != vq->vring.used->idx;
 }
 
-static void *vring_get_buf(struct virtqueue *_vq, unsigned int *len)
+void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 	void *ret;

@@ -325,15 +327,17 @@ static void *vring_get_buf(struct virtqueue *_vq, unsigned int *len)
 	END_USE(vq);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(virtqueue_get_buf);
 
-static void vring_disable_cb(struct virtqueue *_vq)
+void virtqueue_disable_cb(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 
 	vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;
 }
+EXPORT_SYMBOL_GPL(virtqueue_disable_cb);
 
-static bool vring_enable_cb(struct virtqueue *_vq)
+bool virtqueue_enable_cb(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 

@@ -351,8 +355,9 @@ static bool vring_enable_cb(struct virtqueue *_vq)
 	END_USE(vq);
 	return true;
 }
+EXPORT_SYMBOL_GPL(virtqueue_enable_cb);
 
-static void *vring_detach_unused_buf(struct virtqueue *_vq)
+void *virtqueue_detach_unused_buf(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 	unsigned int i;

@@ -375,6 +380,7 @@ static void *vring_detach_unused_buf(struct virtqueue *_vq)
 	END_USE(vq);
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(virtqueue_detach_unused_buf);
 
 irqreturn_t vring_interrupt(int irq, void *_vq)
 {

@@ -396,15 +402,6 @@ irqreturn_t vring_interrupt(int irq, void *_vq)
 }
 EXPORT_SYMBOL_GPL(vring_interrupt);
 
-static struct virtqueue_ops vring_vq_ops = {
-	.add_buf = vring_add_buf,
-	.get_buf = vring_get_buf,
-	.kick = vring_kick,
-	.disable_cb = vring_disable_cb,
-	.enable_cb = vring_enable_cb,
-	.detach_unused_buf = vring_detach_unused_buf,
-};
-
 struct virtqueue *vring_new_virtqueue(unsigned int num,
 				      unsigned int vring_align,
 				      struct virtio_device *vdev,

@@ -429,7 +426,6 @@ struct virtqueue *vring_new_virtqueue(unsigned int num,
 	vring_init(&vq->vring, num, pages, vring_align);
 	vq->vq.callback = callback;
 	vq->vq.vdev = vdev;
-	vq->vq.vq_ops = &vring_vq_ops;
 	vq->vq.name = name;
 	vq->notify = notify;
 	vq->broken = false;
include/linux/virtio.h +18 -53

119 lines not on this page · raw message

Signature · 1 line
1.7.0.2.280.gc6f05
author maintainer
#7 lore raw
In reply to Michael S. Tsirkin
Quoted 16 lines · Michael S. Tsirkin

virtqueue ops were introduced in the hope that we'll have multiple implementations besides virtio_ring, but none have surfaced so far, and given that existing virtio ring is deployed in production we are likely stuck with it now, so this layer just adds complexity and overhead.
Further, the need to pass vq twice to each call (as in dev->vq->vq_ops->kick(dev->vq) ) adds potential for cut and paste errors.

This patchset does the following:
- add inline wrappers converting the above to virtqueue_kick(dev->vq)
- convert all users to this API
- remove vq_ops indirection, implementing virtqueue_xx directly in virtio_ring

Quoted 3 lines

Note that if we ever want to add another vring implementation, we'll only need to revert the last patch in the series, devices won't have to change.

Looks like I missed virtio-rng and trans_virtio.
So the following are needed as well.

Signature · 1 line
MST
author maintainer
#8 lore raw

PATCH 1/1 virtio-rng: use virtqueue_xxx wrappers

Switch virtio-rng to new virtqueue_xxx wrappers.

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 drivers/char/hw_random/virtio-rng.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/char/hw_random/virtio-rng.c +3 -3
index 64fe0a7..75f1cbd 100644
@@ -32,7 +32,7 @@ static bool busy;
 static void random_recv_done(struct virtqueue *vq)
 {
 	/* We can get spurious callbacks, e.g. shared IRQs + virtio_pci. */
-	if (!vq->vq_ops->get_buf(vq, &data_avail))
+	if (!virtqueue_get_buf(vq, &data_avail))
 		return;
 
 	complete(&have_data);

@@ -46,10 +46,10 @@ static void register_buffer(u8 *buf, size_t size)
 	sg_init_one(&sg, buf, size);
 
 	/* There should always be room for one buffer. */
-	if (vq->vq_ops->add_buf(vq, &sg, 0, 1, buf) < 0)
+	if (virtqueue_add_buf(vq, &sg, 0, 1, buf) < 0)
 		BUG();
 
-	vq->vq_ops->kick(vq);
+	virtqueue_kick(vq);
 }
 
 static int virtio_read(struct hwrng *rng, void *buf, size_t size, bool wait)
Signature · 1 line
1.7.0.2.280.gc6f05
author maintainer
#9 lore raw

PATCH 1/1 trans_virtio: use virtqueue_xxx wrappers

In reply to Michael S. Tsirkin

Switch trans_virtio to new virtqueue_xxx wrappers.

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 net/9p/trans_virtio.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
net/9p/trans_virtio.c +3 -3
index 7eb78ec..dcfbe99 100644
@@ -137,7 +137,7 @@ static void req_done(struct virtqueue *vq)
 
 	P9_DPRINTK(P9_DEBUG_TRANS, ": request done\n");
 
-	while ((rc = chan->vq->vq_ops->get_buf(chan->vq, &len)) != NULL) {
+	while ((rc = virtqueue_get_buf(chan->vq, &len)) != NULL) {
 		P9_DPRINTK(P9_DEBUG_TRANS, ": rc %p\n", rc);
 		P9_DPRINTK(P9_DEBUG_TRANS, ": lookup tag %d\n", rc->tag);
 		req = p9_tag_lookup(chan->client, rc->tag);

@@ -209,13 +209,13 @@ p9_virtio_request(struct p9_client *client, struct p9_req_t *req)
 
 	req->status = REQ_STATUS_SENT;
 
-	if (chan->vq->vq_ops->add_buf(chan->vq, chan->sg, out, in, req->tc) < 0) {
+	if (virtqueue_add_buf(chan->vq, chan->sg, out, in, req->tc) < 0) {
 		P9_DPRINTK(P9_DEBUG_TRANS,
 			"9p debug: virtio rpc add_buf returned failure");
 		return -EIO;
 	}
 
-	chan->vq->vq_ops->kick(chan->vq);
+	virtqueue_kick(chan->vq);
 
 	P9_DPRINTK(P9_DEBUG_TRANS, "9p debug: virtio request kicked\n");
 	return 0;
Signature · 1 line
1.7.0.2.280.gc6f05
#10 lore raw

Re: [PATCH] trans_virtio: use virtqueue_xxx wrappers

Quoted 1 line · Michael S. Tsirkin

Switch trans_virtio to new virtqueue_xxx wrappers.

Thanks, I've applied all these.

Cheers,
Rusty.

author maintainer
#11 lore raw
In reply to Michael S. Tsirkin
Quoted 5 lines · Michael S. Tsirkin

This patchset does the following:
- add inline wrappers converting the above to virtqueue_kick(dev->vq)
- convert all users to this API
- remove vq_ops indirection, implementing virtqueue_xx directly in virtio_ring

Ping.
what's the thinking on this patchset? The reason I want it in now is because I have several improvements for the APIs in mind, and with less layers it's much easier to extend things.

Signature · 1 line
MST