FAILED: patch "[PATCH] virtio_console: allocate inbufs in add_port() only if it is" failed to apply to 4.9-stable tree
Subsystems (3)
[email protected]
maintainerThe patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From d791cfcbf98191122af70b053a21075cb450d119 Mon Sep 17 00:00:00 2001
From: Laurent Vivier <[email protected]>
Date: Thu, 14 Nov 2019 13:25:48 +0100
Subject: [PATCH] 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 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.
drivers/char/virtio_console.c +13 -15
@@ -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 {/*
Sasha Levin
maintainerQuoted 42 lines · On Mon, Nov 25, 2019 at 06:44:05PM +0100, [email protected] wrote:
The patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From
d791cfcbf98191122af70b053a21075cb450d119Mon Sep 17 00:00:00 2001
From: Laurent Vivier <[email protected]>
Date: Thu, 14 Nov 2019 13:25:48 +0100
Subject: [PATCH]virtio_console: allocate inbufs inadd_port()only if it is neededWhen 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_addvirtserialport,bus=virtio-serial0.0,nr=2,\
chardev=serial0,id=serial0,name=serial0
(qemu)device_delserial0
(qemu)device_addvirtserialport,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.0This happens because buffers for the
in_vqare 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 inremove_vqs(), but it sounds like a waste of memory.
Quoted 8 lines
Rather than that, this patch changes
add_port()logic to ignore ENOSPC error infill_queue(), which means queue has already been filled.Fixes a7a69ec0d8e4 ("virtio_console: free buffers after reset")Signed-off-by Laurent Vivier <[email protected]>Signed-off-by Michael S. Tsirkin <[email protected]>
I took the following two commits as well and queued all 3 for 4.9 and 4.4:
2855b33514d2 ("virtio_console: don't tie bufs to a vq") 5c60300d68da ("virtio_console: reset on out of memory")
Signature · 2 lines
Thanks, Sasha