Commit d791cfcb authored by Laurent Vivier's avatar Laurent Vivier Committed by Michael S. Tsirkin

virtio_console: allocate inbufs in add_port() only if it is needed

When we hot unplug a virtserialport and then try to hot plug again,
it fails:

(qemu) chardev-add socket,id=serial0,path=/tmp/serial0,server,nowait
(qemu) device_add virtserialport,bus=virtio-serial0.0,nr=2,\
                  chardev=serial0,id=serial0,name=serial0
(qemu) device_del serial0
(qemu) device_add virtserialport,bus=virtio-serial0.0,nr=2,\
                  chardev=serial0,id=serial0,name=serial0
kernel error:
  virtio-ports vport2p2: Error allocating inbufs
qemu error:
  virtio-serial-bus: Guest failure in adding port 2 for device \
                     virtio-serial0.0

This happens because buffers for the in_vq are allocated when the port is
added but are not released when the port is unplugged.

They are only released when virtconsole is removed (see a7a69ec0)

To avoid the problem and to be symmetric, we could allocate all the buffers
in init_vqs() as they are released in remove_vqs(), but it sounds like
a waste of memory.

Rather than that, this patch changes add_port() logic to ignore ENOSPC
error in fill_queue(), which means queue has already been filled.

Fixes: a7a69ec0 ("virtio_console: free buffers after reset")
Cc: mst@redhat.com
Cc: stable@vger.kernel.org
Signed-off-by: default avatarLaurent Vivier <lvivier@redhat.com>
Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
parent f7728002
...@@ -1325,24 +1325,24 @@ static void set_console_size(struct port *port, u16 rows, u16 cols) ...@@ -1325,24 +1325,24 @@ static void set_console_size(struct port *port, u16 rows, u16 cols)
port->cons.ws.ws_col = cols; port->cons.ws.ws_col = cols;
} }
static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock) static int fill_queue(struct virtqueue *vq, spinlock_t *lock)
{ {
struct port_buffer *buf; struct port_buffer *buf;
unsigned int nr_added_bufs; int nr_added_bufs;
int ret; int ret;
nr_added_bufs = 0; nr_added_bufs = 0;
do { do {
buf = alloc_buf(vq->vdev, PAGE_SIZE, 0); buf = alloc_buf(vq->vdev, PAGE_SIZE, 0);
if (!buf) if (!buf)
break; return -ENOMEM;
spin_lock_irq(lock); spin_lock_irq(lock);
ret = add_inbuf(vq, buf); ret = add_inbuf(vq, buf);
if (ret < 0) { if (ret < 0) {
spin_unlock_irq(lock); spin_unlock_irq(lock);
free_buf(buf, true); free_buf(buf, true);
break; return ret;
} }
nr_added_bufs++; nr_added_bufs++;
spin_unlock_irq(lock); spin_unlock_irq(lock);
...@@ -1362,7 +1362,6 @@ static int add_port(struct ports_device *portdev, u32 id) ...@@ -1362,7 +1362,6 @@ static int add_port(struct ports_device *portdev, u32 id)
char debugfs_name[16]; char debugfs_name[16];
struct port *port; struct port *port;
dev_t devt; dev_t devt;
unsigned int nr_added_bufs;
int err; int err;
port = kmalloc(sizeof(*port), GFP_KERNEL); port = kmalloc(sizeof(*port), GFP_KERNEL);
...@@ -1421,11 +1420,13 @@ static int add_port(struct ports_device *portdev, u32 id) ...@@ -1421,11 +1420,13 @@ static int add_port(struct ports_device *portdev, u32 id)
spin_lock_init(&port->outvq_lock); spin_lock_init(&port->outvq_lock);
init_waitqueue_head(&port->waitqueue); init_waitqueue_head(&port->waitqueue);
/* Fill the in_vq with buffers so the host can send us data. */ /* We can safely ignore ENOSPC because it means
nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); * the queue already has buffers. Buffers are removed
if (!nr_added_bufs) { * only by virtcons_remove(), not by unplug_port()
*/
err = fill_queue(port->in_vq, &port->inbuf_lock);
if (err < 0 && err != -ENOSPC) {
dev_err(port->dev, "Error allocating inbufs\n"); dev_err(port->dev, "Error allocating inbufs\n");
err = -ENOMEM;
goto free_device; goto free_device;
} }
...@@ -2059,14 +2060,11 @@ static int virtcons_probe(struct virtio_device *vdev) ...@@ -2059,14 +2060,11 @@ static int virtcons_probe(struct virtio_device *vdev)
INIT_WORK(&portdev->control_work, &control_work_handler); INIT_WORK(&portdev->control_work, &control_work_handler);
if (multiport) { if (multiport) {
unsigned int nr_added_bufs;
spin_lock_init(&portdev->c_ivq_lock); spin_lock_init(&portdev->c_ivq_lock);
spin_lock_init(&portdev->c_ovq_lock); spin_lock_init(&portdev->c_ovq_lock);
nr_added_bufs = fill_queue(portdev->c_ivq, err = fill_queue(portdev->c_ivq, &portdev->c_ivq_lock);
&portdev->c_ivq_lock); if (err < 0) {
if (!nr_added_bufs) {
dev_err(&vdev->dev, dev_err(&vdev->dev,
"Error allocating buffers for control queue\n"); "Error allocating buffers for control queue\n");
/* /*
...@@ -2077,7 +2075,7 @@ static int virtcons_probe(struct virtio_device *vdev) ...@@ -2077,7 +2075,7 @@ static int virtcons_probe(struct virtio_device *vdev)
VIRTIO_CONSOLE_DEVICE_READY, 0); VIRTIO_CONSOLE_DEVICE_READY, 0);
/* Device was functional: we need full cleanup. */ /* Device was functional: we need full cleanup. */
virtcons_remove(vdev); virtcons_remove(vdev);
return -ENOMEM; return err;
} }
} else { } else {
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment