| From 83fb0cd2b9b368c95adaab7b9bfb06fc40acbb94 Mon Sep 17 00:00:00 2001 |
| From: Laurent Vivier <lvivier@redhat.com> |
| Date: Thu, 14 Nov 2019 13:25:48 +0100 |
| Subject: [PATCH] virtio_console: allocate inbufs in add_port() only if it is |
| needed |
| |
| commit d791cfcbf98191122af70b053a21075cb450d119 upstream. |
| |
| 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 a7a69ec0d8e4) |
| |
| 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: a7a69ec0d8e4 ("virtio_console: free buffers after reset") |
| Cc: mst@redhat.com |
| Cc: stable@vger.kernel.org |
| Signed-off-by: Laurent Vivier <lvivier@redhat.com> |
| Signed-off-by: Michael S. Tsirkin <mst@redhat.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c |
| index 7270e7b69262..3259426f01dc 100644 |
| --- a/drivers/char/virtio_console.c |
| +++ b/drivers/char/virtio_console.c |
| @@ -1325,24 +1325,24 @@ static void set_console_size(struct port *port, u16 rows, u16 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; |
| - unsigned int nr_added_bufs; |
| + int nr_added_bufs; |
| int ret; |
| |
| nr_added_bufs = 0; |
| do { |
| buf = alloc_buf(vq->vdev, PAGE_SIZE, 0); |
| if (!buf) |
| - break; |
| + return -ENOMEM; |
| |
| spin_lock_irq(lock); |
| ret = add_inbuf(vq, buf); |
| if (ret < 0) { |
| spin_unlock_irq(lock); |
| free_buf(buf, true); |
| - break; |
| + return ret; |
| } |
| nr_added_bufs++; |
| spin_unlock_irq(lock); |
| @@ -1362,7 +1362,6 @@ static int add_port(struct ports_device *portdev, u32 id) |
| char debugfs_name[16]; |
| struct port *port; |
| dev_t devt; |
| - unsigned int nr_added_bufs; |
| int err; |
| |
| port = kmalloc(sizeof(*port), GFP_KERNEL); |
| @@ -1421,11 +1420,13 @@ static int add_port(struct ports_device *portdev, u32 id) |
| spin_lock_init(&port->outvq_lock); |
| init_waitqueue_head(&port->waitqueue); |
| |
| - /* Fill the in_vq with buffers so the host can send us data. */ |
| - nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); |
| - if (!nr_added_bufs) { |
| + /* We can safely ignore ENOSPC because it means |
| + * the queue already has buffers. Buffers are removed |
| + * 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"); |
| - err = -ENOMEM; |
| goto free_device; |
| } |
| |
| @@ -2059,14 +2060,11 @@ static int virtcons_probe(struct virtio_device *vdev) |
| INIT_WORK(&portdev->control_work, &control_work_handler); |
| |
| if (multiport) { |
| - unsigned int nr_added_bufs; |
| - |
| spin_lock_init(&portdev->c_ivq_lock); |
| spin_lock_init(&portdev->c_ovq_lock); |
| |
| - nr_added_bufs = fill_queue(portdev->c_ivq, |
| - &portdev->c_ivq_lock); |
| - if (!nr_added_bufs) { |
| + err = fill_queue(portdev->c_ivq, &portdev->c_ivq_lock); |
| + if (err < 0) { |
| dev_err(&vdev->dev, |
| "Error allocating buffers for control queue\n"); |
| /* |
| @@ -2077,7 +2075,7 @@ static int virtcons_probe(struct virtio_device *vdev) |
| VIRTIO_CONSOLE_DEVICE_READY, 0); |
| /* Device was functional: we need full cleanup. */ |
| virtcons_remove(vdev); |
| - return -ENOMEM; |
| + return err; |
| } |
| } else { |
| /* |
| -- |
| 2.7.4 |
| |