* [PATCH] vhost: error handling fix
From: Michael S. Tsirkin @ 2010-09-03 10:25 UTC (permalink / raw)
To: Michael S. Tsirkin, Tejun Heo, Eric Dumazet, Ingo Molnar, kvm,
virtualization@
vhost should set worker to NULL on cgroups attach failure,
so that we won't try to destroy the worker again on close.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Dave, no need to bother with this one unless you really want to -
I'll put it on my tree.
This is a follow up to Eric's patch - fixes error handling in a similar way.
drivers/vhost/vhost.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 7c75dce..666aa36 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -298,6 +298,7 @@ static long vhost_dev_set_owner(struct vhost_dev *dev)
return 0;
err_cgroup:
kthread_stop(worker);
+ dev->worker = NULL;
err_worker:
if (dev->mm)
mmput(dev->mm);
--
1.7.2.rc0.14.g41c1c
^ permalink raw reply related
* Re: [PATCH 5/5] staging: hv: Gracefully handle SCSI resets - RESEND
From: Greg KH @ 2010-09-02 20:08 UTC (permalink / raw)
To: Joe Perches
Cc: devel, haiyangz, gregkh, Hank Janssen, linux-kernel,
virtualization
In-Reply-To: <1283358535.1797.174.camel@Joe-Laptop>
On Wed, Sep 01, 2010 at 09:28:53AM -0700, Joe Perches wrote:
> On Tue, 2010-08-31 at 11:22 -0700, Hank Janssen wrote:
> > If we get a SCSI host bus reset we now gracefully handle it, and we
> > take the device offline. This before sometimes caused hangs.
> > ---
> > drivers/staging/hv/storvsc.c | 36 +++++++++++++++++++++++++++++++++++-
> []
> > + /*
> > + * Wait for traffic in transit to complete
> > + */
> > + while (atomic_read(&storDevice->NumOutstandingRequests))
> > + udelay(1000);
> > +
>
> Is it useful to have a maximum check or timeout?
> Maybe use usleep_range?
Yeah, that's not good, you need to have some way to get out of here in
case something bad happens and that number never goes down.
Why not just use a completion function instead of a busy loop?
thanks,
greg k-h
^ permalink raw reply
* [PATCH 3/3] virtio: console: Send SIGIO in case of port unplug
From: Amit Shah @ 2010-09-02 13:17 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283433264.git.amit.shah@redhat.com>
If a port has registered for SIGIO signals, let the application
know that the port is getting unplugged.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index a750d2b..f7adfd3 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1244,6 +1244,9 @@ static void unplug_port(struct port *port)
port->guest_connected = false;
port->host_connected = false;
wake_up_interruptible(&port->waitqueue);
+
+ /* Let the app know the port is going down. */
+ send_sigio_to_port(port);
}
if (is_console_port(port)) {
--
1.7.2.2
^ permalink raw reply related
* [PATCH 2/3] virtio: console: Send SIGIO on new data arrival on ports
From: Amit Shah @ 2010-09-02 13:17 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283433264.git.amit.shah@redhat.com>
Send a SIGIO signal when new data arrives on a port. This is sent only
when the process has requested for the signal to be sent using fcntl().
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index afbbc54..a750d2b 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1468,6 +1468,9 @@ static void in_intr(struct virtqueue *vq)
wake_up_interruptible(&port->waitqueue);
+ /* Send a SIGIO indicating new data in case the process asked for it */
+ send_sigio_to_port(port);
+
if (is_console_port(port) && hvc_poll(port->cons.hvc))
hvc_kick();
}
--
1.7.2.2
^ permalink raw reply related
* [PATCH 1/3] virtio: console: Send SIGIO to processes that request it for host events
From: Amit Shah @ 2010-09-02 13:17 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283433264.git.amit.shah@redhat.com>
A process can request for SIGIO on host connect / disconnect events
using the O_ASYNC file flag using fcntl().
If that's requested, and if the guest-side connection for the port is
open, any host-side open/close events for that port will raise a SIGIO.
The process can then use poll() within the signal handler to find out
which port triggered the signal.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 793817b..afbbc54 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -196,6 +196,9 @@ struct port {
/* The 'name' of the port that we expose via sysfs properties */
char *name;
+ /* We can notify apps of host connect / disconnect events via SIGIO */
+ struct fasync_struct *async_queue;
+
/* The 'id' to identify the port with the Host */
u32 id;
@@ -800,6 +803,14 @@ out:
return ret;
}
+static int port_fops_fasync(int fd, struct file *filp, int mode)
+{
+ struct port *port;
+
+ port = filp->private_data;
+ return fasync_helper(fd, filp, mode, &port->async_queue);
+}
+
/*
* The file operations that we support: programs in the guest can open
* a console device, read from it, write to it, poll for data and
@@ -813,6 +824,7 @@ static const struct file_operations port_fops = {
.write = port_fops_write,
.poll = port_fops_poll,
.release = port_fops_release,
+ .fasync = port_fops_fasync,
};
/*
@@ -1071,6 +1083,12 @@ static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock)
return nr_added_bufs;
}
+static void send_sigio_to_port(struct port *port)
+{
+ if (port->async_queue && port->guest_connected)
+ kill_fasync(&port->async_queue, SIGIO, POLL_OUT);
+}
+
static int add_port(struct ports_device *portdev, u32 id)
{
char debugfs_name[16];
@@ -1093,6 +1111,7 @@ static int add_port(struct ports_device *portdev, u32 id)
port->name = NULL;
port->inbuf = NULL;
port->cons.hvc = NULL;
+ port->async_queue = NULL;
port->cons.ws.ws_row = port->cons.ws.ws_col = 0;
@@ -1347,6 +1366,12 @@ static void handle_control_message(struct ports_device *portdev,
spin_lock_irq(&port->outvq_lock);
reclaim_consumed_buffers(port);
spin_unlock_irq(&port->outvq_lock);
+
+ /*
+ * If the guest is connected, it'll be interested in
+ * knowing the host connection state changed.
+ */
+ send_sigio_to_port(port);
break;
case VIRTIO_CONSOLE_PORT_NAME:
/*
--
1.7.2.2
^ permalink raw reply related
* [PATCH 0/3] virtio: console: async notifications for host connect / disconnect
From: Amit Shah @ 2010-09-02 13:17 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
Hey Rusty,
This patchset is on top of the previous one.
It sends a SIGIO signal to apps that request signals for host
activity. SIGIO is sent on host connect, disconnect as well as
hot-unplug (which can be seen as a special case of host disconnect).
Tested using several testcases in the test-virtserial repo:
http://fedorapeople.org/gitweb?p=amitshah/public_git/test-virtserial.git
Please apply.
Thanks,
Amit.
Amit Shah (3):
virtio: console: Send SIGIO to processes that request it for host
events
virtio: console: Send SIGIO on new data arrival on ports
virtio: console: Send SIGIO in case of port unplug
drivers/char/virtio_console.c | 31 +++++++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
--
1.7.2.2
^ permalink raw reply
* [PATCH 14/14] virtio: console: Reference counting portdev structs is not needed
From: Amit Shah @ 2010-09-02 13:08 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
Explain in a comment why there's no need to reference-count the portdev
struct: when a device is yanked out, we can't do anything more with it
anyway so just give up doing anything more with the data or the vqs and
exit cleanly.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 1e00221..793817b 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1723,6 +1723,14 @@ static void virtcons_remove(struct virtio_device *vdev)
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.
+ */
if (use_multiport(portdev)) {
struct port_buffer *buf;
unsigned int len;
--
1.7.2.2
^ permalink raw reply related
* [PATCH 13/14] virtio: console: Add reference counting for port struct
From: Amit Shah @ 2010-09-02 13:08 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
When a port got hot-unplugged, when a port was open, any file operation
after the unplugging resulted in a crash. This is fixed by ref-counting
the port structure, and releasing it only when the file is closed.
This splits the unplug operation in two parts: first marks the port
as unavailable, removes all the buffers in the vqs and removes the port
from the per-device list of ports. The second stage, invoked when all
references drop to zero, releases the chardev and frees all other memory.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 80 ++++++++++++++++++++++++++++++++--------
1 files changed, 64 insertions(+), 16 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 0c4dc26..1e00221 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -187,6 +187,9 @@ struct port {
struct cdev *cdev;
struct device *dev;
+ /* Reference-counting to handle port hot-unplugs and file operations */
+ struct kref kref;
+
/* A waitqueue for poll() or blocking read operations */
wait_queue_head_t waitqueue;
@@ -710,6 +713,8 @@ static unsigned int port_fops_poll(struct file *filp, poll_table *wait)
return ret;
}
+static void remove_port(struct kref *kref);
+
static int port_fops_release(struct inode *inode, struct file *filp)
{
struct port *port;
@@ -730,6 +735,16 @@ static int port_fops_release(struct inode *inode, struct file *filp)
reclaim_consumed_buffers(port);
spin_unlock_irq(&port->outvq_lock);
+ /*
+ * Locks aren't necessary here as a port can't be opened after
+ * unplug, and if a port isn't unplugged, a kref would already
+ * exist for the port. Plus, taking ports_lock here would
+ * create a dependency on other locks taken by functions
+ * inside remove_port if we're the last holder of the port,
+ * creating many problems.
+ */
+ kref_put(&port->kref, remove_port);
+
return 0;
}
@@ -742,6 +757,11 @@ static int port_fops_open(struct inode *inode, struct file *filp)
port = find_port_by_devt(cdev->dev);
filp->private_data = port;
+ /* Prevent against a port getting hot-unplugged at the same time */
+ spin_lock_irq(&port->portdev->ports_lock);
+ kref_get(&port->kref);
+ spin_unlock_irq(&port->portdev->ports_lock);
+
/*
* Don't allow opening of console port devices -- that's done
* via /dev/hvc
@@ -776,6 +796,7 @@ static int port_fops_open(struct inode *inode, struct file *filp)
return 0;
out:
+ kref_put(&port->kref, remove_port);
return ret;
}
@@ -1064,6 +1085,7 @@ static int add_port(struct ports_device *portdev, u32 id)
err = -ENOMEM;
goto fail;
}
+ kref_init(&port->kref);
port->portdev = portdev;
port->id = id;
@@ -1168,22 +1190,43 @@ fail:
return err;
}
-/* Remove all port-specific data. */
-static void remove_port(struct port *port)
+/* No users remain, remove all port-specific data. */
+static void remove_port(struct kref *kref)
+{
+ struct port *port;
+
+ port = container_of(kref, struct port, kref);
+
+ sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
+ device_destroy(pdrvdata.class, port->dev->devt);
+ cdev_del(port->cdev);
+
+ kfree(port->name);
+
+ debugfs_remove(port->debugfs_file);
+
+ kfree(port);
+}
+
+/*
+ * Port got unplugged. Remove port from portdev's list and drop the
+ * kref reference. If no userspace has this port opened, it will
+ * result in immediate removal the port.
+ */
+static void unplug_port(struct port *port)
{
struct port_buffer *buf;
+ spin_lock_irq(&port->portdev->ports_lock);
+ list_del(&port->list);
+ spin_unlock_irq(&port->portdev->ports_lock);
+
if (port->guest_connected) {
port->guest_connected = false;
port->host_connected = false;
wake_up_interruptible(&port->waitqueue);
- send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 0);
}
- spin_lock_irq(&port->portdev->ports_lock);
- list_del(&port->list);
- spin_unlock_irq(&port->portdev->ports_lock);
-
if (is_console_port(port)) {
spin_lock_irq(&pdrvdata_lock);
list_del(&port->cons.list);
@@ -1201,9 +1244,6 @@ static void remove_port(struct port *port)
hvc_remove(port->cons.hvc);
#endif
}
- sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
- device_destroy(pdrvdata.class, port->dev->devt);
- cdev_del(port->cdev);
/* Remove unused data this port might have received. */
discard_port_data(port);
@@ -1214,11 +1254,19 @@ static void remove_port(struct port *port)
while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
free_buf(buf);
- kfree(port->name);
-
- debugfs_remove(port->debugfs_file);
+ /*
+ * We should just assume the device itself has gone off --
+ * else a close on an open port later will try to send out a
+ * control message.
+ */
+ port->portdev = NULL;
- kfree(port);
+ /*
+ * Locks around here are not necessary - a port can't be
+ * opened after we removed the port struct from ports_list
+ * above.
+ */
+ kref_put(&port->kref, remove_port);
}
/* Any private messages that the Host and Guest want to share */
@@ -1257,7 +1305,7 @@ static void handle_control_message(struct ports_device *portdev,
add_port(portdev, cpkt->id);
break;
case VIRTIO_CONSOLE_PORT_REMOVE:
- remove_port(port);
+ unplug_port(port);
break;
case VIRTIO_CONSOLE_CONSOLE_PORT:
if (!cpkt->value)
@@ -1671,7 +1719,7 @@ static void virtcons_remove(struct virtio_device *vdev)
cancel_work_sync(&portdev->control_work);
list_for_each_entry_safe(port, port2, &portdev->ports, list)
- remove_port(port);
+ unplug_port(port);
unregister_chrdev(portdev->chr_major, "virtio-portsdev");
--
1.7.2.2
^ permalink raw reply related
* [PATCH 12/14] virtio: console: Use cdev_alloc() instead of cdev_init()
From: Amit Shah @ 2010-09-02 12:50 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
This moves to using cdev on the heap instead of it being embedded in the
ports struct. This helps individual refcounting and will allow us to
properly remove cdev structs after hot-unplugs and close operations.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 20 +++++++++++++-------
1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index af44332..0c4dc26 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -184,7 +184,7 @@ struct port {
struct console cons;
/* Each port associates with a separate char device */
- struct cdev cdev;
+ struct cdev *cdev;
struct device *dev;
/* A waitqueue for poll() or blocking read operations */
@@ -235,7 +235,7 @@ static struct port *find_port_by_devt_in_portdev(struct ports_device *portdev,
spin_lock_irqsave(&portdev->ports_lock, flags);
list_for_each_entry(port, &portdev->ports, list)
- if (port->cdev.dev == dev)
+ if (port->cdev->dev == dev)
goto out;
port = NULL;
out:
@@ -1081,14 +1081,20 @@ static int add_port(struct ports_device *portdev, u32 id)
port->in_vq = portdev->in_vqs[port->id];
port->out_vq = portdev->out_vqs[port->id];
- cdev_init(&port->cdev, &port_fops);
+ port->cdev = cdev_alloc();
+ if (!port->cdev) {
+ dev_err(&port->portdev->vdev->dev, "Error allocating cdev\n");
+ err = -ENOMEM;
+ goto free_port;
+ }
+ port->cdev->ops = &port_fops;
devt = MKDEV(portdev->chr_major, id);
- err = cdev_add(&port->cdev, devt, 1);
+ err = cdev_add(port->cdev, devt, 1);
if (err < 0) {
dev_err(&port->portdev->vdev->dev,
"Error %d adding cdev for port %u\n", err, id);
- goto free_port;
+ goto free_cdev;
}
port->dev = device_create(pdrvdata.class, &port->portdev->vdev->dev,
devt, port, "vport%up%u",
@@ -1153,7 +1159,7 @@ free_inbufs:
free_device:
device_destroy(pdrvdata.class, port->dev->devt);
free_cdev:
- cdev_del(&port->cdev);
+ cdev_del(port->cdev);
free_port:
kfree(port);
fail:
@@ -1197,7 +1203,7 @@ static void remove_port(struct port *port)
}
sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
device_destroy(pdrvdata.class, port->dev->devt);
- cdev_del(&port->cdev);
+ cdev_del(port->cdev);
/* Remove unused data this port might have received. */
discard_port_data(port);
--
1.7.2.2
^ permalink raw reply related
* [PATCH 11/14] virtio: console: Add a find_port_by_devt() function
From: Amit Shah @ 2010-09-02 12:50 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
To convert to using cdev as a pointer to avoid kref troubles, we have to
use a different method to get to a port from an inode than the current
container_of method.
Add find_port_by_devt() that looks up all portdevs and ports with those
portdevs to find the right port.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 37 ++++++++++++++++++++++++++++++++++++-
1 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index a37a868..af44332 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -227,6 +227,41 @@ out:
return port;
}
+static struct port *find_port_by_devt_in_portdev(struct ports_device *portdev,
+ dev_t dev)
+{
+ struct port *port;
+ unsigned long flags;
+
+ spin_lock_irqsave(&portdev->ports_lock, flags);
+ list_for_each_entry(port, &portdev->ports, list)
+ if (port->cdev.dev == dev)
+ goto out;
+ port = NULL;
+out:
+ spin_unlock_irqrestore(&portdev->ports_lock, flags);
+
+ return port;
+}
+
+static struct port *find_port_by_devt(dev_t dev)
+{
+ struct ports_device *portdev;
+ struct port *port;
+ unsigned long flags;
+
+ spin_lock_irqsave(&pdrvdata_lock, flags);
+ list_for_each_entry(portdev, &pdrvdata.portdevs, list) {
+ port = find_port_by_devt_in_portdev(portdev, dev);
+ if (port)
+ goto out;
+ }
+ port = NULL;
+out:
+ spin_unlock_irqrestore(&pdrvdata_lock, flags);
+ return port;
+}
+
static struct port *find_port_by_id(struct ports_device *portdev, u32 id)
{
struct port *port;
@@ -704,7 +739,7 @@ static int port_fops_open(struct inode *inode, struct file *filp)
struct port *port;
int ret;
- port = container_of(cdev, struct port, cdev);
+ port = find_port_by_devt(cdev->dev);
filp->private_data = port;
/*
--
1.7.2.2
^ permalink raw reply related
* [PATCH 10/14] virtio: console: Add a list of portdevs that are active
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
The virtio_console.c driver is capable of handling multiple devices at a
time. Maintain a list of devices for future traversal.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index d689c76..a37a868 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -48,6 +48,9 @@ struct ports_driver_data {
/* Used for exporting per-port information to debugfs */
struct dentry *debugfs_dir;
+ /* List of all the devices we're handling */
+ struct list_head portdevs;
+
/* Number of devices this driver is handling */
unsigned int index;
@@ -108,6 +111,9 @@ struct port_buffer {
* ports for that device (vdev->priv).
*/
struct ports_device {
+ /* Next portdev in the list, head is in the pdrvdata struct */
+ struct list_head list;
+
/*
* Workqueue handlers where we process deferred work after
* notification
@@ -1584,6 +1590,10 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
add_port(portdev, 0);
}
+ spin_lock_irq(&pdrvdata_lock);
+ list_add_tail(&portdev->list, &pdrvdata.portdevs);
+ spin_unlock_irq(&pdrvdata_lock);
+
__send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
VIRTIO_CONSOLE_DEVICE_READY, 1);
return 0;
@@ -1610,6 +1620,10 @@ static void virtcons_remove(struct virtio_device *vdev)
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 */
@@ -1676,6 +1690,7 @@ static int __init init(void)
PTR_ERR(pdrvdata.debugfs_dir));
}
INIT_LIST_HEAD(&pdrvdata.consoles);
+ INIT_LIST_HEAD(&pdrvdata.portdevs);
return register_virtio_driver(&virtio_console);
}
--
1.7.2.2
^ permalink raw reply related
* [PATCH 09/14] virtio: console: open: Use a common path for error handling
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
Just re-arrange code for future patches.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 0c4ee9b1..d689c76 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -696,6 +696,7 @@ static int port_fops_open(struct inode *inode, struct file *filp)
{
struct cdev *cdev = inode->i_cdev;
struct port *port;
+ int ret;
port = container_of(cdev, struct port, cdev);
filp->private_data = port;
@@ -704,14 +705,17 @@ static int port_fops_open(struct inode *inode, struct file *filp)
* Don't allow opening of console port devices -- that's done
* via /dev/hvc
*/
- if (is_console_port(port))
- return -ENXIO;
+ if (is_console_port(port)) {
+ ret = -ENXIO;
+ goto out;
+ }
/* Allow only one process to open a particular port at a time */
spin_lock_irq(&port->inbuf_lock);
if (port->guest_connected) {
spin_unlock_irq(&port->inbuf_lock);
- return -EMFILE;
+ ret = -EMFILE;
+ goto out;
}
port->guest_connected = true;
@@ -730,6 +734,8 @@ static int port_fops_open(struct inode *inode, struct file *filp)
send_control_msg(filp->private_data, VIRTIO_CONSOLE_PORT_OPEN, 1);
return 0;
+out:
+ return ret;
}
/*
--
1.7.2.2
^ permalink raw reply related
* [PATCH 08/14] virtio: console: remove_port() should return void
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
When a port is removed, we have to assume the port is gone. So a
success/failure return value doesn't make sense.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index ecadfa2..0c4ee9b1 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1116,7 +1116,7 @@ fail:
}
/* Remove all port-specific data. */
-static int remove_port(struct port *port)
+static void remove_port(struct port *port)
{
struct port_buffer *buf;
@@ -1166,7 +1166,6 @@ static int remove_port(struct port *port)
debugfs_remove(port->debugfs_file);
kfree(port);
- return 0;
}
/* Any private messages that the Host and Guest want to share */
--
1.7.2.2
^ permalink raw reply related
* [PATCH 07/14] virtio: console: Make write() return -ENODEV on hot-unplug
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
When a port is hot-unplugged while an app was blocked on a write() call,
the call was unblocked but would not get an error returned.
Return -ENODEV to ensure the app knows the port has gone away.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 7fa3c26..ecadfa2 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -619,6 +619,9 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
if (ret < 0)
return ret;
}
+ /* Port got hot-unplugged. */
+ if (!port->guest_connected)
+ return -ENODEV;
count = min((size_t)(32 * 1024), count);
--
1.7.2.2
^ permalink raw reply related
* [PATCH 06/14] virtio: console: Make read() return -ENODEV on hot-unplug
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
When a port is hot-unplugged while an app was blocked on a read() call,
the call was unblocked but would not get an error returned.
Return -ENODEV to ensure the app knows the port has gone away.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index c8a5678..7fa3c26 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -579,6 +579,9 @@ static ssize_t port_fops_read(struct file *filp, char __user *ubuf,
if (ret < 0)
return ret;
}
+ /* Port got hot-unplugged. */
+ if (!port->guest_connected)
+ return -ENODEV;
/*
* We could've received a disconnection message while we were
* waiting for more data.
--
1.7.2.2
^ permalink raw reply related
* [PATCH 05/14] virtio: console: Unblock poll on port hot-unplug
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
When a port is hot-unplugged while an app is blocked on poll(), unblock
the poll() and return.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 488d250..c8a5678 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -648,6 +648,10 @@ static unsigned int port_fops_poll(struct file *filp, poll_table *wait)
port = filp->private_data;
poll_wait(filp, &port->waitqueue, wait);
+ if (!port->guest_connected) {
+ /* Port got unplugged */
+ return POLLHUP;
+ }
ret = 0;
if (port->inbuf)
ret |= POLLIN | POLLRDNORM;
--
1.7.2.2
^ permalink raw reply related
* [PATCH 04/14] virtio: console: Un-block reads on chardev close
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
If a chardev is closed, any blocked read / poll calls should just return
and not attempt to use other state.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 5cd5374..488d250 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -525,6 +525,10 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
/* The condition that must be true for polling to end */
static bool will_read_block(struct port *port)
{
+ if (!port->guest_connected) {
+ /* Port got hot-unplugged. Let's exit. */
+ return false;
+ }
return !port_has_data(port) && port->host_connected;
}
--
1.7.2.2
^ permalink raw reply related
* [PATCH 03/14] virtio: console: Check if portdev is valid in send_control_msg()
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
A portdev may have been hot-unplugged while a port was open()ed. Skip
sending control messages when the portdev isn't valid.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 0f115b4..5cd5374 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -410,7 +410,10 @@ static ssize_t __send_control_msg(struct ports_device *portdev, u32 port_id,
static ssize_t send_control_msg(struct port *port, unsigned int event,
unsigned int value)
{
- return __send_control_msg(port->portdev, port->id, event, value);
+ /* Did the port get unplugged before userspace closed it? */
+ if (port->portdev)
+ return __send_control_msg(port->portdev, port->id, event, value);
+ return 0;
}
/* Callers must take the port->outvq_lock */
--
1.7.2.2
^ permalink raw reply related
* [PATCH 02/14] virtio: console: Remove control vq data only if using multiport support
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
If a portdev isn't using multiport support, it won't have any control vq
data to remove.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 32267d4..0f115b4 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1585,8 +1585,6 @@ static void virtcons_remove(struct virtio_device *vdev)
{
struct ports_device *portdev;
struct port *port, *port2;
- struct port_buffer *buf;
- unsigned int len;
portdev = vdev->priv;
@@ -1600,11 +1598,16 @@ static void virtcons_remove(struct virtio_device *vdev)
unregister_chrdev(portdev->chr_major, "virtio-portsdev");
- while ((buf = virtqueue_get_buf(portdev->c_ivq, &len)))
- free_buf(buf);
+ if (use_multiport(portdev)) {
+ struct port_buffer *buf;
+ unsigned int len;
- while ((buf = virtqueue_detach_unused_buf(portdev->c_ivq)))
- free_buf(buf);
+ while ((buf = virtqueue_get_buf(portdev->c_ivq, &len)))
+ free_buf(buf);
+
+ while ((buf = virtqueue_detach_unused_buf(portdev->c_ivq)))
+ free_buf(buf);
+ }
vdev->config->del_vqs(vdev);
kfree(portdev->in_vqs);
--
1.7.2.2
^ permalink raw reply related
* [PATCH 01/14] virtio: console: Reset vdev before removing device
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
In-Reply-To: <cover.1283430900.git.amit.shah@redhat.com>
The virtqueues should be disabled before attempting to remove the
device.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 942a982..32267d4 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1590,6 +1590,9 @@ static void virtcons_remove(struct virtio_device *vdev)
portdev = vdev->priv;
+ /* Disable interrupts for vqs */
+ vdev->config->reset(vdev);
+ /* Finish up work that's lined up */
cancel_work_sync(&portdev->control_work);
list_for_each_entry_safe(port, port2, &portdev->ports, list)
--
1.7.2.2
^ permalink raw reply related
* [PATCH 00/14] virtio: console: Hot-unplug fixes
From: Amit Shah @ 2010-09-02 12:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Amit Shah, Virtualization List
Hey Rusty,
These are the patches that rework a few bits to make hot-unplug while
ports are open not crash apps (or kernels).
The problem is when hot-unplug is performed when a port is open, the
cdev struct is kept around by the file pointers and when the app later
does a 'close', things go boom-boom.
This patch series makes sure port as well as device hot-unplug is now
safe to perform at any time: when ports are open, not open, hvc
consoles are active, inactive, module removal, applications doing
blocking read/write/poll calls while port / devices are going away...
Ideally this patchset would be applicable for 2.6.36 as well as
2.6.35, but I'll let you decide as it's quite big.
Amit Shah (14):
virtio: console: Reset vdev before removing device
virtio: console: Remove control vq data only if using multiport
support
virtio: console: Check if portdev is valid in send_control_msg()
virtio: console: Unblock reads on chardev close
virtio: console: Unblock poll on port hot-unplug
virtio: console: Make read() return -ENODEV on hot-unplug
virtio: console: Make write() return -ENODEV on hot-unplug
virtio: console: remove_port() should return void
virtio: console: open: Use a common path for error handling
virtio: console: Add a list of portdevs that are active
virtio: console: Add a find_port_by_devt() function
virtio: console: Use cdev_alloc() instead of cdev_init()
virtio: console: Add reference counting for port struct
virtio: console: Reference counting portdev structs is not needed
drivers/char/virtio_console.c | 206 ++++++++++++++++++++++++++++++++++-------
1 files changed, 173 insertions(+), 33 deletions(-)
--
1.7.2.2
^ permalink raw reply
* Re: [PATCH 2/5] staging: hv: Fixed lockup problem with bounce_buffer scatter list - RESEND
From: Joe Perches @ 2010-09-01 22:45 UTC (permalink / raw)
To: Hank Janssen; +Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang
In-Reply-To: <4C7D601B.3060708@sailtheuniverse.com>
On Tue, 2010-08-31 at 13:03 -0700, Hank Janssen wrote:
> On 09/01/2010 09:38 AM, Joe Perches wrote:
> > On Tue, 2010-08-31 at 11:13 -0700, Hank Janssen wrote:
> >> diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
> >> - request->DataBuffer.PfnArray[i] =
> >> - page_to_pfn(sg_page((&sgl[i])));
> >> + request->DataBuffer.PfnArray[i] =
> >> + page_to_pfn(sg_page((&sgl[i])));
> > I did a bit of a doubletake reading this last change.
> > This isn't actually a part of your fix and it introduces
> > a whitespace error.
> I will resubmit this patch and clean up the whitespace error.
>
> When change the code I normally also change the indentation to the
> correct format for that area of the code. That is how the whitespace
> snuck in there.
At least mention the whitespace change in the description
or perhaps use a different coding style for readability
that uses temporaries to avoid the split lines.
Something like:
drivers/staging/hv/storvsc_drv.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 075b61b..874891b 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -611,6 +611,8 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
struct storvsc_driver_object *storvsc_drv_obj =
&storvsc_drv_ctx->drv_obj;
struct hv_storvsc_request *request;
+ struct hv_multipage_buffer *rmpb;
+ u64 *rmpba;
struct storvsc_cmd_request *cmd_request;
unsigned int request_size = 0;
int i;
@@ -693,8 +695,10 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
request->SenseBuffer = scmnd->sense_buffer;
request->SenseBufferSize = SCSI_SENSE_BUFFERSIZE;
+ rmpb = &request->DataBuffer;
+ rmpba = rmpb->PfnArray;
- request->DataBuffer.Length = scsi_bufflen(scmnd);
+ rmpb->Length = scsi_bufflen(scmnd);
if (scsi_sg_count(scmnd)) {
sgl = (struct scatterlist *)scsi_sglist(scmnd);
@@ -733,20 +737,17 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
sgl = cmd_request->bounce_sgl;
}
- request->DataBuffer.Offset = sgl[0].offset;
+ rmpb->Offset = sgl[0].offset;
for (i = 0; i < scsi_sg_count(scmnd); i++) {
DPRINT_DBG(STORVSC_DRV, "sgl[%d] len %d offset %d\n",
i, sgl[i].length, sgl[i].offset);
- request->DataBuffer.PfnArray[i] =
- page_to_pfn(sg_page((&sgl[i])));
+ *rmpba++ = page_to_pfn(sg_page((&sgl[i])));
}
} else if (scsi_sglist(scmnd)) {
/* ASSERT(scsi_bufflen(scmnd) <= PAGE_SIZE); */
- request->DataBuffer.Offset =
- virt_to_phys(scsi_sglist(scmnd)) & (PAGE_SIZE-1);
- request->DataBuffer.PfnArray[0] =
- virt_to_phys(scsi_sglist(scmnd)) >> PAGE_SHIFT;
+ rmpb->Offset = virt_to_phys(scsi_sglist(scmnd)) & (PAGE_SIZE-1);
+ *rmpba = virt_to_phys(scsi_sglist(scmnd)) >> PAGE_SHIFT;
}
retry_request:
^ permalink raw reply related
* [PATCH 2/5] staging: hv: Fixed lockup problem with bounce_buffer scatter list - Cleaned up
From: Hank Janssen @ 2010-09-01 18:10 UTC (permalink / raw)
To: hjanssen, gregkh, devel, virtualization, linux-kernel,
Haiyang Zhang
Cc: Joe Perches
Fixed lockup problem with bounce_buffer scatter list which caused
crashes in heavy loads. And minor code indentation cleanup in effected
area.
Removed whitespace and noted minor indentation changes in description as
pointed out by Joe Perches. (Thanks for reviewing Joe)
Signed-off-by:Hank Janssen <hjanssen@microsoft.com
Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com
---
drivers/staging/hv/storvsc_drv.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 3b9ccb0..169d701 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -615,6 +615,7 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
unsigned int request_size = 0;
int i;
struct scatterlist *sgl;
+ unsigned int sg_count = 0;
DPRINT_DBG(STORVSC_DRV, "scmnd %p dir %d, use_sg %d buf %p len %d "
"queue depth %d tagged %d", scmnd, scmnd->sc_data_direction,
@@ -697,6 +698,7 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
request->DataBuffer.Length = scsi_bufflen(scmnd);
if (scsi_sg_count(scmnd)) {
sgl = (struct scatterlist *)scsi_sglist(scmnd);
+ sg_count = scsi_sg_count(scmnd);
/* check if we need to bounce the sgl */
if (do_bounce_buffer(sgl, scsi_sg_count(scmnd)) != -1) {
@@ -731,15 +733,16 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
scsi_sg_count(scmnd));
sgl = cmd_request->bounce_sgl;
+ sg_count = cmd_request->bounce_sgl_count;
}
request->DataBuffer.Offset = sgl[0].offset;
- for (i = 0; i < scsi_sg_count(scmnd); i++) {
+ for (i = 0; i < sg_count; i++) {
DPRINT_DBG(STORVSC_DRV, "sgl[%d] len %d offset %d\n",
i, sgl[i].length, sgl[i].offset);
- request->DataBuffer.PfnArray[i] =
- page_to_pfn(sg_page((&sgl[i])));
+ request->DataBuffer.PfnArray[i] =
+ page_to_pfn(sg_page((&sgl[i])));
}
} else if (scsi_sglist(scmnd)) {
/* ASSERT(scsi_bufflen(scmnd) <= PAGE_SIZE); */
--
1.6.0.2
^ permalink raw reply related
* Re: [PATCH 2/5] staging: hv: Fixed lockup problem with bounce_buffer scatter list - RESEND
From: Joe Perches @ 2010-09-01 16:38 UTC (permalink / raw)
To: Hank Janssen; +Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang
In-Reply-To: <4C7D4667.5040102@sailtheuniverse.com>
On Tue, 2010-08-31 at 11:13 -0700, Hank Janssen wrote:
> diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
[]
> - request->DataBuffer.PfnArray[i] =
> - page_to_pfn(sg_page((&sgl[i])));
> + request->DataBuffer.PfnArray[i] =
> + page_to_pfn(sg_page((&sgl[i])));
I did a bit of a doubletake reading this last change.
This isn't actually a part of your fix and it introduces
a whitespace error.
^ permalink raw reply
* Re: [PATCH 5/5] staging: hv: Gracefully handle SCSI resets - RESEND
From: Joe Perches @ 2010-09-01 16:28 UTC (permalink / raw)
To: Hank Janssen; +Cc: gregkh, linux-kernel, virtualization, devel, haiyangz
In-Reply-To: <4C7D4848.6090409@sailtheuniverse.com>
On Tue, 2010-08-31 at 11:22 -0700, Hank Janssen wrote:
> If we get a SCSI host bus reset we now gracefully handle it, and we
> take the device offline. This before sometimes caused hangs.
> ---
> drivers/staging/hv/storvsc.c | 36 +++++++++++++++++++++++++++++++++++-
[]
> + /*
> + * Wait for traffic in transit to complete
> + */
> + while (atomic_read(&storDevice->NumOutstandingRequests))
> + udelay(1000);
> +
Is it useful to have a maximum check or timeout?
Maybe use usleep_range?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox