From b40000325044433cd350725e2025214ae48b17fd Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Fri, 13 Apr 2018 16:37:04 +0300 Subject: [PATCH 1/7] virtio_balloon: add array of stat names Jason Wang points out that it's very hard for users to build an array of stat names. The naive thing is to use VIRTIO_BALLOON_S_NR but that breaks if we add more stats - as done e.g. recently by commit 6c64fe7f2 ("virtio_balloon: export hugetlb page allocation counts"). Let's add an array of reasonably readable names. Fixes: 6c64fe7f2 ("virtio_balloon: export hugetlb page allocation counts") Cc: Jason Wang Signed-off-by: Michael S. Tsirkin Reviewed-by: Jonathan Helman --- include/uapi/linux/virtio_balloon.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h index 40297a3181ed..13b8cb563892 100644 --- a/include/uapi/linux/virtio_balloon.h +++ b/include/uapi/linux/virtio_balloon.h @@ -57,6 +57,21 @@ struct virtio_balloon_config { #define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Hugetlb page allocation failures */ #define VIRTIO_BALLOON_S_NR 10 +#define VIRTIO_BALLOON_S_NAMES_WITH_PREFIX(VIRTIO_BALLOON_S_NAMES_prefix) { \ + VIRTIO_BALLOON_S_NAMES_prefix "swap-in", \ + VIRTIO_BALLOON_S_NAMES_prefix "swap-out", \ + VIRTIO_BALLOON_S_NAMES_prefix "major-faults", \ + VIRTIO_BALLOON_S_NAMES_prefix "minor-faults", \ + VIRTIO_BALLOON_S_NAMES_prefix "free-memory", \ + VIRTIO_BALLOON_S_NAMES_prefix "total-memory", \ + VIRTIO_BALLOON_S_NAMES_prefix "available-memory", \ + VIRTIO_BALLOON_S_NAMES_prefix "disk-caches", \ + VIRTIO_BALLOON_S_NAMES_prefix "hugetlb-allocations", \ + VIRTIO_BALLOON_S_NAMES_prefix "hugetlb-failures" \ +} + +#define VIRTIO_BALLOON_S_NAMES VIRTIO_BALLOON_S_NAMES_WITH_PREFIX("") + /* * Memory statistics structure. * Driver fills an array of these structures and passes to device. From 2855b33514d290c51d52d94e25d3ef942cd4d578 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Fri, 20 Apr 2018 19:54:23 +0300 Subject: [PATCH 2/7] virtio_console: don't tie bufs to a vq an allocated buffer doesn't need to be tied to a vq - only vq->vdev is ever used. Pass the function the just what it needs - the vdev. Cc: stable@vger.kernel.org Signed-off-by: Michael S. Tsirkin --- drivers/char/virtio_console.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 468f06134012..3e56f328b4cb 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -422,7 +422,7 @@ static void reclaim_dma_bufs(void) } } -static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, +static struct port_buffer *alloc_buf(struct virtio_device *vdev, size_t buf_size, int pages) { struct port_buffer *buf; @@ -445,16 +445,16 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, return buf; } - if (is_rproc_serial(vq->vdev)) { + if (is_rproc_serial(vdev)) { /* * Allocate DMA memory from ancestor. When a virtio * device is created by remoteproc, the DMA memory is * associated with the grandparent device: * vdev => rproc => platform-dev. */ - if (!vq->vdev->dev.parent || !vq->vdev->dev.parent->parent) + if (!vdev->dev.parent || !vdev->dev.parent->parent) goto free_buf; - buf->dev = vq->vdev->dev.parent->parent; + buf->dev = vdev->dev.parent->parent; /* Increase device refcnt to avoid freeing it */ get_device(buf->dev); @@ -838,7 +838,7 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf, count = min((size_t)(32 * 1024), count); - buf = alloc_buf(port->out_vq, count, 0); + buf = alloc_buf(port->portdev->vdev, count, 0); if (!buf) return -ENOMEM; @@ -957,7 +957,7 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe, if (ret < 0) goto error_out; - buf = alloc_buf(port->out_vq, 0, pipe->nrbufs); + buf = alloc_buf(port->portdev->vdev, 0, pipe->nrbufs); if (!buf) { ret = -ENOMEM; goto error_out; @@ -1374,7 +1374,7 @@ static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock) nr_added_bufs = 0; do { - buf = alloc_buf(vq, PAGE_SIZE, 0); + buf = alloc_buf(vq->vdev, PAGE_SIZE, 0); if (!buf) break; From 24a7e4d20783c0514850f24a5c41ede46ab058f0 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Fri, 20 Apr 2018 20:22:40 +0300 Subject: [PATCH 3/7] virtio: add ability to iterate over vqs For cleanup it's helpful to be able to simply scan all vqs and discard all data. Add an iterator to do that. Cc: stable@vger.kernel.org Signed-off-by: Michael S. Tsirkin --- include/linux/virtio.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/virtio.h b/include/linux/virtio.h index 988c7355bc22..fa1b5da2804e 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -157,6 +157,9 @@ int virtio_device_freeze(struct virtio_device *dev); int virtio_device_restore(struct virtio_device *dev); #endif +#define virtio_device_for_each_vq(vdev, vq) \ + list_for_each_entry(vq, &vdev->vqs, list) + /** * virtio_driver - operations for a virtio I/O driver * @driver: underlying device driver (populate name and owner). From a7a69ec0d8e4a58be7db88d33cbfa2912807bb2b Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Fri, 20 Apr 2018 20:24:23 +0300 Subject: [PATCH 4/7] virtio_console: free buffers after reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Console driver is out of spec. The spec says: A driver MUST NOT decrement the available idx on a live virtqueue (ie. there is no way to “unexpose” buffers). and it does exactly that by trying to detach unused buffers without doing a device reset first. Defer detaching the buffers until device unplug. Of course this means we might get an interrupt for a vq without an attached port now. Handle that by discarding the consumed buffer. Reported-by: Tiwei Bie Fixes: b3258ff1d6 ("virtio: Decrement avail idx on buffer detach") Cc: stable@vger.kernel.org Signed-off-by: Michael S. Tsirkin --- drivers/char/virtio_console.c | 49 +++++++++++++++++------------------ 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 3e56f328b4cb..26a66ffd943e 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1402,7 +1402,6 @@ static int add_port(struct ports_device *portdev, u32 id) { char debugfs_name[16]; struct port *port; - struct port_buffer *buf; dev_t devt; unsigned int nr_added_bufs; int err; @@ -1513,8 +1512,6 @@ static int add_port(struct ports_device *portdev, u32 id) return 0; free_inbufs: - while ((buf = virtqueue_detach_unused_buf(port->in_vq))) - free_buf(buf, true); free_device: device_destroy(pdrvdata.class, port->dev->devt); free_cdev: @@ -1539,34 +1536,14 @@ static void remove_port(struct kref *kref) static void remove_port_data(struct port *port) { - struct port_buffer *buf; - spin_lock_irq(&port->inbuf_lock); /* Remove unused data this port might have received. */ discard_port_data(port); spin_unlock_irq(&port->inbuf_lock); - /* Remove buffers we queued up for the Host to send us data in. */ - do { - spin_lock_irq(&port->inbuf_lock); - buf = virtqueue_detach_unused_buf(port->in_vq); - spin_unlock_irq(&port->inbuf_lock); - if (buf) - free_buf(buf, true); - } while (buf); - spin_lock_irq(&port->outvq_lock); reclaim_consumed_buffers(port); spin_unlock_irq(&port->outvq_lock); - - /* Free pending buffers from the out-queue. */ - do { - spin_lock_irq(&port->outvq_lock); - buf = virtqueue_detach_unused_buf(port->out_vq); - spin_unlock_irq(&port->outvq_lock); - if (buf) - free_buf(buf, true); - } while (buf); } /* @@ -1791,13 +1768,24 @@ static void control_work_handler(struct work_struct *work) spin_unlock(&portdev->c_ivq_lock); } +static void flush_bufs(struct virtqueue *vq, bool can_sleep) +{ + struct port_buffer *buf; + unsigned int len; + + while ((buf = virtqueue_get_buf(vq, &len))) + free_buf(buf, can_sleep); +} + static void out_intr(struct virtqueue *vq) { struct port *port; port = find_port_by_vq(vq->vdev->priv, vq); - if (!port) + if (!port) { + flush_bufs(vq, false); return; + } wake_up_interruptible(&port->waitqueue); } @@ -1808,8 +1796,10 @@ static void in_intr(struct virtqueue *vq) unsigned long flags; port = find_port_by_vq(vq->vdev->priv, vq); - if (!port) + if (!port) { + flush_bufs(vq, false); return; + } spin_lock_irqsave(&port->inbuf_lock, flags); port->inbuf = get_inbuf(port); @@ -1984,6 +1974,15 @@ static const struct file_operations portdev_fops = { static void remove_vqs(struct ports_device *portdev) { + struct virtqueue *vq; + + virtio_device_for_each_vq(portdev->vdev, vq) { + struct port_buffer *buf; + + flush_bufs(vq, true); + while ((buf = virtqueue_detach_unused_buf(vq))) + free_buf(buf, true); + } portdev->vdev->config->del_vqs(portdev->vdev); kfree(portdev->in_vqs); kfree(portdev->out_vqs); From 61a8950c5c5708cf2068b29ffde94e454e528208 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Fri, 20 Apr 2018 20:49:04 +0300 Subject: [PATCH 5/7] virtio_console: drop custom control queue cleanup We now cleanup all VQs on device removal - no need to handle the control VQ specially. Cc: stable@vger.kernel.org Signed-off-by: Michael S. Tsirkin --- drivers/char/virtio_console.c | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 26a66ffd943e..2d87ce555140 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1988,21 +1988,6 @@ static void remove_vqs(struct ports_device *portdev) kfree(portdev->out_vqs); } -static void remove_controlq_data(struct ports_device *portdev) -{ - struct port_buffer *buf; - unsigned int len; - - if (!use_multiport(portdev)) - return; - - while ((buf = virtqueue_get_buf(portdev->c_ivq, &len))) - free_buf(buf, true); - - while ((buf = virtqueue_detach_unused_buf(portdev->c_ivq))) - free_buf(buf, true); -} - /* * Once we're further in boot, we get probed like any other virtio * device. @@ -2163,7 +2148,6 @@ static void virtcons_remove(struct virtio_device *vdev) * have to just stop using the port, as the vqs are going * away. */ - remove_controlq_data(portdev); remove_vqs(portdev); kfree(portdev); } @@ -2208,7 +2192,6 @@ static int virtcons_freeze(struct virtio_device *vdev) */ if (use_multiport(portdev)) virtqueue_disable_cb(portdev->c_ivq); - remove_controlq_data(portdev); list_for_each_entry(port, &portdev->ports, list) { virtqueue_disable_cb(port->in_vq); From aa44ec867030a72e8aa127977e37dec551d8df19 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Fri, 20 Apr 2018 20:51:18 +0300 Subject: [PATCH 6/7] virtio_console: move removal code Will make it reusable for error handling. Cc: stable@vger.kernel.org Signed-off-by: Michael S. Tsirkin --- drivers/char/virtio_console.c | 72 +++++++++++++++++------------------ 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 2d87ce555140..e8480fe2e1d8 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1988,6 +1988,42 @@ static void remove_vqs(struct ports_device *portdev) kfree(portdev->out_vqs); } +static void virtcons_remove(struct virtio_device *vdev) +{ + struct ports_device *portdev; + struct port *port, *port2; + + portdev = vdev->priv; + + spin_lock_irq(&pdrvdata_lock); + list_del(&portdev->list); + spin_unlock_irq(&pdrvdata_lock); + + /* Disable interrupts for vqs */ + vdev->config->reset(vdev); + /* Finish up work that's lined up */ + if (use_multiport(portdev)) + cancel_work_sync(&portdev->control_work); + else + cancel_work_sync(&portdev->config_work); + + list_for_each_entry_safe(port, port2, &portdev->ports, list) + unplug_port(port); + + unregister_chrdev(portdev->chr_major, "virtio-portsdev"); + + /* + * When yanking out a device, we immediately lose the + * (device-side) queues. So there's no point in keeping the + * guest side around till we drop our final reference. This + * also means that any ports which are in an open state will + * have to just stop using the port, as the vqs are going + * away. + */ + remove_vqs(portdev); + kfree(portdev); +} + /* * Once we're further in boot, we get probed like any other virtio * device. @@ -2116,42 +2152,6 @@ fail: return err; } -static void virtcons_remove(struct virtio_device *vdev) -{ - struct ports_device *portdev; - struct port *port, *port2; - - portdev = vdev->priv; - - spin_lock_irq(&pdrvdata_lock); - list_del(&portdev->list); - spin_unlock_irq(&pdrvdata_lock); - - /* Disable interrupts for vqs */ - vdev->config->reset(vdev); - /* Finish up work that's lined up */ - if (use_multiport(portdev)) - cancel_work_sync(&portdev->control_work); - else - cancel_work_sync(&portdev->config_work); - - list_for_each_entry_safe(port, port2, &portdev->ports, list) - unplug_port(port); - - unregister_chrdev(portdev->chr_major, "virtio-portsdev"); - - /* - * When yanking out a device, we immediately lose the - * (device-side) queues. So there's no point in keeping the - * guest side around till we drop our final reference. This - * also means that any ports which are in an open state will - * have to just stop using the port, as the vqs are going - * away. - */ - remove_vqs(portdev); - kfree(portdev); -} - static struct virtio_device_id id_table[] = { { VIRTIO_ID_CONSOLE, VIRTIO_DEV_ANY_ID }, { 0 }, From 5c60300d68da32ca77f7f978039dc72bfc78b06b Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Fri, 20 Apr 2018 21:00:13 +0300 Subject: [PATCH 7/7] virtio_console: reset on out of memory When out of memory and we can't add ctrl vq buffers, probe fails. Unfortunately the error handling is out of spec: it calls del_vqs without bothering to reset the device first. To fix, call the full cleanup function in this case. Cc: stable@vger.kernel.org Signed-off-by: Michael S. Tsirkin --- drivers/char/virtio_console.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index e8480fe2e1d8..21085515814f 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -2090,6 +2090,7 @@ static int virtcons_probe(struct virtio_device *vdev) spin_lock_init(&portdev->ports_lock); INIT_LIST_HEAD(&portdev->ports); + INIT_LIST_HEAD(&portdev->list); virtio_device_ready(portdev->vdev); @@ -2107,8 +2108,15 @@ static int virtcons_probe(struct virtio_device *vdev) if (!nr_added_bufs) { dev_err(&vdev->dev, "Error allocating buffers for control queue\n"); - err = -ENOMEM; - goto free_vqs; + /* + * The host might want to notify mgmt sw about device + * add failure. + */ + __send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID, + VIRTIO_CONSOLE_DEVICE_READY, 0); + /* Device was functional: we need full cleanup. */ + virtcons_remove(vdev); + return -ENOMEM; } } else { /* @@ -2139,11 +2147,6 @@ static int virtcons_probe(struct virtio_device *vdev) return 0; -free_vqs: - /* The host might want to notify mgmt sw about device add failure */ - __send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID, - VIRTIO_CONSOLE_DEVICE_READY, 0); - remove_vqs(portdev); free_chrdev: unregister_chrdev(portdev->chr_major, "virtio-portsdev"); free: