lkml.org 
[lkml]   [2018]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/6] virtio_console: free buffers after reset
From
Date


On 2018年04月21日 02:18, Michael S. Tsirkin wrote:
> 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 <tiwei.bie@intel.com>
> Fixes: b3258ff1d6 ("virtio: Decrement avail idx on buffer detach")
> CC: stable@kernel.org
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

I wonder whether or not we can have some BUG_ON() in
virtqueue_detach_unused_buf() to detect such bugs (e.g by checking status?).

Thanks

> ---
> 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 3e56f32..26a66ff 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);

\
 
 \ /
  Last update: 2018-04-24 04:41    [W:0.242 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site