[PATCH] virtio: console: Fixes from linux-next

Amit Shah amit.shah at redhat.com
Mon Feb 15 13:50:58 UTC 2010


The virtio-console code in Rusty's tree has been updated with the following
changes:
- Initialize the 'data' entry for virtio's virtqueues
- Fix some memleaks on port hot-unplug
- Allow module unload
- Allocate more buffers for the host to send data
- Add myself to MAINTAINERS for virtio-console.

Please merge this patch with the rollup patch for the F13 and rawhide branches.

Signed-off-by: Amit Shah <amit.shah at redhat.com>
---
 MAINTAINERS                   |    6 ++
 drivers/char/virtio_console.c |  132 +++++++++++++++++++++++++++++++---------
 drivers/virtio/virtio_ring.c  |    5 +-
 3 files changed, 112 insertions(+), 31 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 03f38c1..3118dfa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2394,6 +2394,12 @@ L:	linuxppc-dev at ozlabs.org
 S:	Odd Fixes
 F:	drivers/char/hvc_*
 
+VIRTIO CONSOLE DRIVER
+M:	Amit Shah <amit.shah at redhat.com>
+L:	virtualization at lists.linux-foundation.org
+S:	Maintained
+F:	drivers/char/virtio_console.c
+
 GSPCA FINEPIX SUBDRIVER
 M:	Frank Zago <frank at zago.net>
 L:	linux-media at vger.kernel.org
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index a3f1f73..213373b 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -330,6 +330,7 @@ static void discard_port_data(struct port *port)
 	struct port_buffer *buf;
 	struct virtqueue *vq;
 	unsigned int len;
+	int ret;
 
 	vq = port->in_vq;
 	if (port->inbuf)
@@ -337,16 +338,18 @@ static void discard_port_data(struct port *port)
 	else
 		buf = vq->vq_ops->get_buf(vq, &len);
 
-	if (!buf)
-		return;
-
-	if (add_inbuf(vq, buf) < 0) {
-		buf->len = buf->offset = 0;
-		dev_warn(port->dev, "Error adding buffer back to vq\n");
-		return;
+	ret = 0;
+	while (buf) {
+		if (add_inbuf(vq, buf) < 0) {
+			ret++;
+			free_buf(buf);
+		}
+		buf = vq->vq_ops->get_buf(vq, &len);
 	}
-
 	port->inbuf = NULL;
+	if (ret)
+		dev_warn(port->dev, "Errors adding %d buffers back to vq\n",
+			 ret);
 }
 
 static bool port_has_data(struct port *port)
@@ -354,12 +357,19 @@ static bool port_has_data(struct port *port)
 	unsigned long flags;
 	bool ret;
 
-	ret = false;
 	spin_lock_irqsave(&port->inbuf_lock, flags);
-	if (port->inbuf)
+	if (port->inbuf) {
 		ret = true;
+		goto out;
+	}
+	port->inbuf = get_inbuf(port);
+	if (port->inbuf) {
+		ret = true;
+		goto out;
+	}
+	ret = false;
+out:
 	spin_unlock_irqrestore(&port->inbuf_lock, flags);
-
 	return ret;
 }
 
@@ -838,6 +848,8 @@ static const struct file_operations port_debugfs_ops = {
 /* Remove all port-specific data. */
 static int remove_port(struct port *port)
 {
+	struct port_buffer *buf;
+
 	spin_lock_irq(&port->portdev->ports_lock);
 	list_del(&port->list);
 	spin_unlock_irq(&port->portdev->ports_lock);
@@ -851,14 +863,17 @@ static int remove_port(struct port *port)
 	if (port->guest_connected)
 		send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 0);
 
-	while (port->in_vq->vq_ops->detach_unused_buf(port->in_vq))
-		;
-
 	sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
 	device_destroy(pdrvdata.class, port->dev->devt);
 	cdev_del(&port->cdev);
 
+	/* Remove unused data this port might have received. */
 	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)))
+		free_buf(buf);
+
 	kfree(port->name);
 
 	debugfs_remove(port->debugfs_file);
@@ -1006,7 +1021,8 @@ static void in_intr(struct virtqueue *vq)
 		return;
 
 	spin_lock_irqsave(&port->inbuf_lock, flags);
-	port->inbuf = get_inbuf(port);
+	if (!port->inbuf)
+		port->inbuf = get_inbuf(port);
 
 	/*
 	 * Don't queue up data when port is closed.  This condition
@@ -1052,32 +1068,37 @@ static void config_intr(struct virtio_device *vdev)
 	resize_console(find_port_by_id(portdev, 0));
 }
 
-static void fill_queue(struct virtqueue *vq, spinlock_t *lock)
+static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
 {
 	struct port_buffer *buf;
-	int ret;
+	unsigned int ret;
+	int err;
 
+	ret = 0;
 	do {
 		buf = alloc_buf(PAGE_SIZE);
 		if (!buf)
 			break;
 
 		spin_lock_irq(lock);
-		ret = add_inbuf(vq, buf);
-		if (ret < 0) {
+		err = add_inbuf(vq, buf);
+		if (err < 0) {
 			spin_unlock_irq(lock);
 			free_buf(buf);
 			break;
 		}
+		ret++;
 		spin_unlock_irq(lock);
-	} while (ret > 0);
+	} while (err > 0);
+
+	return ret;
 }
 
 static int add_port(struct ports_device *portdev, u32 id)
 {
 	char debugfs_name[16];
 	struct port *port;
-	struct port_buffer *inbuf;
+	struct port_buffer *buf;
 	dev_t devt;
 	int err;
 
@@ -1122,22 +1143,21 @@ static int add_port(struct ports_device *portdev, u32 id)
 	spin_lock_init(&port->inbuf_lock);
 	init_waitqueue_head(&port->waitqueue);
 
-	inbuf = alloc_buf(PAGE_SIZE);
-	if (!inbuf) {
+	/* Fill the in_vq with buffers so the host can send us data. */
+	err = fill_queue(port->in_vq, &port->inbuf_lock);
+	if (!err) {
+		dev_err(port->dev, "Error allocating inbufs\n");
 		err = -ENOMEM;
 		goto free_device;
 	}
 
-	/* Register the input buffer the first time. */
-	add_inbuf(port->in_vq, inbuf);
-
 	/*
 	 * If we're not using multiport support, this has to be a console port
 	 */
 	if (!use_multiport(port->portdev)) {
 		err = init_port_console(port);
 		if (err)
-			goto free_inbuf;
+			goto free_inbufs;
 	}
 
 	spin_lock_irq(&portdev->ports_lock);
@@ -1165,8 +1185,9 @@ static int add_port(struct ports_device *portdev, u32 id)
 	}
 	return 0;
 
-free_inbuf:
-	free_buf(inbuf);
+free_inbufs:
+	while ((buf = port->in_vq->vq_ops->detach_unused_buf(port->in_vq)))
+		free_buf(buf);
 free_device:
 	device_destroy(pdrvdata.class, port->dev->devt);
 free_cdev:
@@ -1425,7 +1446,13 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 		INIT_WORK(&portdev->control_work, &control_work_handler);
 		INIT_WORK(&portdev->config_work, &config_work_handler);
 
-		fill_queue(portdev->c_ivq, &portdev->cvq_lock);
+		err = fill_queue(portdev->c_ivq, &portdev->cvq_lock);
+		if (!err) {
+			dev_err(&vdev->dev,
+				"Error allocating buffers for control queue\n");
+			err = -ENOMEM;
+			goto free_vqs;
+		}
 	}
 
 	for (i = 0; i < portdev->config.nr_ports; i++)
@@ -1435,6 +1462,10 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	early_put_chars = NULL;
 	return 0;
 
+free_vqs:
+	vdev->config->del_vqs(vdev);
+	kfree(portdev->in_vqs);
+	kfree(portdev->out_vqs);
 free_chrdev:
 	unregister_chrdev(portdev->chr_major, "virtio-portsdev");
 free:
@@ -1443,6 +1474,36 @@ fail:
 	return err;
 }
 
+static void virtcons_remove(struct virtio_device *vdev)
+{
+	struct ports_device *portdev;
+	struct port *port, *port2;
+	struct port_buffer *buf;
+	unsigned int len;
+
+	portdev = vdev->priv;
+
+	cancel_work_sync(&portdev->control_work);
+	cancel_work_sync(&portdev->config_work);
+
+	list_for_each_entry_safe(port, port2, &portdev->ports, list)
+		remove_port(port);
+
+	unregister_chrdev(portdev->chr_major, "virtio-portsdev");
+
+	while ((buf = portdev->c_ivq->vq_ops->get_buf(portdev->c_ivq, &len)))
+		free_buf(buf);
+
+	while ((buf = portdev->c_ivq->vq_ops->detach_unused_buf(portdev->c_ivq)))
+		free_buf(buf);
+
+	vdev->config->del_vqs(vdev);
+	kfree(portdev->in_vqs);
+	kfree(portdev->out_vqs);
+
+	kfree(portdev);
+}
+
 static struct virtio_device_id id_table[] = {
 	{ VIRTIO_ID_CONSOLE, VIRTIO_DEV_ANY_ID },
 	{ 0 },
@@ -1460,6 +1521,7 @@ static struct virtio_driver virtio_console = {
 	.driver.owner =	THIS_MODULE,
 	.id_table =	id_table,
 	.probe =	virtcons_probe,
+	.remove =	virtcons_remove,
 	.config_changed = config_intr,
 };
 
@@ -1483,7 +1545,17 @@ static int __init init(void)
 
 	return register_virtio_driver(&virtio_console);
 }
+
+static void __exit fini(void)
+{
+	unregister_virtio_driver(&virtio_console);
+
+	class_destroy(pdrvdata.class);
+	if (pdrvdata.debugfs_dir)
+		debugfs_remove_recursive(pdrvdata.debugfs_dir);
+}
 module_init(init);
+module_exit(fini);
 
 MODULE_DEVICE_TABLE(virtio, id_table);
 MODULE_DESCRIPTION("Virtio console driver");
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 71929ee..9bcfe95 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -431,8 +431,11 @@ struct virtqueue *vring_new_virtqueue(unsigned int num,
 	/* Put everything in free lists. */
 	vq->num_free = num;
 	vq->free_head = 0;
-	for (i = 0; i < num-1; i++)
+	for (i = 0; i < num-1; i++) {
 		vq->vring.desc[i].next = i+1;
+		vq->data[i] = NULL;
+	}
+	vq->data[i] = NULL;
 
 	return &vq->vq;
 }
-- 
1.6.2.5



More information about the kernel mailing list