firewire: Use only a wait queue and terminate poll and read on device removal.
authorKristian Høgsberg <krh@redhat.com>
Wed, 7 Mar 2007 17:12:48 +0000 (12:12 -0500)
committerStefan Richter <stefanr@s5r6.in-berlin.de>
Fri, 9 Mar 2007 21:03:11 +0000 (22:03 +0100)
Drop the event list semaphore and only use the wait queue and the list
to synchronize queue access.  Break out of a poll or read whenever
the device is disconnected.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
drivers/firewire/fw-device-cdev.c
drivers/firewire/fw-device.c
drivers/firewire/fw-device.h

index 54ef27b2adb5ac2cf5bcfb6e7c5bbbef64d67528..2d84284f76058e959ff4236b673a366c41087b3f 100644 (file)
@@ -76,7 +76,6 @@ struct client {
        struct list_head request_list;
        u32 request_serial;
        struct list_head event_list;
-       struct semaphore event_list_sem;
        wait_queue_head_t wait;
 
        struct fw_iso_context *iso_context;
@@ -114,7 +113,6 @@ static int fw_device_op_open(struct inode *inode, struct file *file)
 
        client->device = fw_device_get(device);
        INIT_LIST_HEAD(&client->event_list);
-       sema_init(&client->event_list_sem, 0);
        INIT_LIST_HEAD(&client->handler_list);
        INIT_LIST_HEAD(&client->request_list);
        spin_lock_init(&client->lock);
@@ -142,38 +140,41 @@ static void queue_event(struct client *client, struct event *event,
        spin_lock_irqsave(&client->lock, flags);
 
        list_add_tail(&event->link, &client->event_list);
-
-       up(&client->event_list_sem);
        wake_up_interruptible(&client->wait);
 
        spin_unlock_irqrestore(&client->lock, flags);
 }
 
-static int dequeue_event(struct client *client, char __user *buffer, size_t count)
+static int
+dequeue_event(struct client *client, char __user *buffer, size_t count)
 {
        unsigned long flags;
        struct event *event;
        size_t size, total;
-       int i, retval = -EFAULT;
+       int i, retval;
 
-       if (down_interruptible(&client->event_list_sem) < 0)
-               return -EINTR;
+       retval = wait_event_interruptible(client->wait,
+                                         !list_empty(&client->event_list) ||
+                                         fw_device_is_shutdown(client->device));
+       if (retval < 0)
+               return retval;
 
-       spin_lock_irqsave(&client->lock, flags);
+       if (list_empty(&client->event_list) &&
+                      fw_device_is_shutdown(client->device))
+               return -ENODEV;
 
+       spin_lock_irqsave(&client->lock, flags);
        event = container_of(client->event_list.next, struct event, link);
        list_del(&event->link);
-
        spin_unlock_irqrestore(&client->lock, flags);
 
-       if (buffer == NULL)
-               goto out;
-
        total = 0;
        for (i = 0; i < ARRAY_SIZE(event->v) && total < count; i++) {
                size = min(event->v[i].size, count - total);
-               if (copy_to_user(buffer + total, event->v[i].data, size))
+               if (copy_to_user(buffer + total, event->v[i].data, size)) {
+                       retval = -EFAULT;
                        goto out;
+               }
                total += size;
        }
        retval = total;
@@ -208,6 +209,22 @@ fill_bus_reset_event(struct fw_cdev_event_bus_reset *event,
        event->generation    = card->generation;
 }
 
+static void
+for_each_client(struct fw_device *device,
+               void (*callback)(struct client *client))
+{
+       struct fw_card *card = device->card;
+       struct client *c;
+       unsigned long flags;
+
+       spin_lock_irqsave(&card->lock, flags);
+
+       list_for_each_entry(c, &device->client_list, link)
+               callback(c);
+
+       spin_unlock_irqrestore(&card->lock, flags);
+}
+
 static void
 queue_bus_reset_event(struct client *client)
 {
@@ -228,16 +245,17 @@ queue_bus_reset_event(struct client *client)
 
 void fw_device_cdev_update(struct fw_device *device)
 {
-       struct fw_card *card = device->card;
-       struct client *c;
-       unsigned long flags;
-
-       spin_lock_irqsave(&card->lock, flags);
+       for_each_client(device, queue_bus_reset_event);
+}
 
-       list_for_each_entry(c, &device->client_list, link)
-               queue_bus_reset_event(c);
+static void wake_up_client(struct client *client)
+{
+       wake_up_interruptible(&client->wait);
+}
 
-       spin_unlock_irqrestore(&card->lock, flags);
+void fw_device_cdev_remove(struct fw_device *device)
+{
+       for_each_client(device, wake_up_client);
 }
 
 static int ioctl_get_info(struct client *client, void __user *arg)
@@ -731,8 +749,9 @@ static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma)
 static int fw_device_op_release(struct inode *inode, struct file *file)
 {
        struct client *client = file->private_data;
-       struct address_handler *h, *next;
+       struct address_handler *h, *next_h;
        struct request *r, *next_r;
+       struct event *e, *next_e;
        unsigned long flags;
 
        if (client->buffer.pages)
@@ -741,7 +760,7 @@ static int fw_device_op_release(struct inode *inode, struct file *file)
        if (client->iso_context)
                fw_iso_context_destroy(client->iso_context);
 
-       list_for_each_entry_safe(h, next, &client->handler_list, link) {
+       list_for_each_entry_safe(h, next_h, &client->handler_list, link) {
                fw_core_remove_address_handler(&h->handler);
                kfree(h);
        }
@@ -755,8 +774,8 @@ static int fw_device_op_release(struct inode *inode, struct file *file)
        /* TODO: wait for all transactions to finish so
         * complete_transaction doesn't try to queue up responses
         * after we free client. */
-       while (!list_empty(&client->event_list))
-               dequeue_event(client, NULL, 0);
+       list_for_each_entry_safe(e, next_e, &client->event_list, link)
+               kfree(e);
 
        spin_lock_irqsave(&client->device->card->lock, flags);
        list_del(&client->link);
@@ -771,13 +790,16 @@ static int fw_device_op_release(struct inode *inode, struct file *file)
 static unsigned int fw_device_op_poll(struct file *file, poll_table * pt)
 {
        struct client *client = file->private_data;
+       unsigned int mask = 0;
 
        poll_wait(file, &client->wait, pt);
 
+       if (fw_device_is_shutdown(client->device))
+               mask |= POLLHUP | POLLERR;
        if (!list_empty(&client->event_list))
-               return POLLIN | POLLRDNORM;
-       else
-               return 0;
+               mask |= POLLIN | POLLRDNORM;
+
+       return mask;
 }
 
 const struct file_operations fw_device_ops = {
index 4ade867db88800fd71ebc2d2f893bf1f513a69b0..4877cdbc58cca8e7b5757234e9de81c57dec21de 100644 (file)
@@ -453,6 +453,7 @@ static void fw_device_shutdown(struct work_struct *work)
        idr_remove(&fw_device_idr, minor);
        up_write(&fw_bus_type.subsys.rwsem);
 
+       fw_device_cdev_remove(device);
        device_remove_file(&device->device, &config_rom_attribute);
        device_for_each_child(&device->device, NULL, shutdown_unit);
        device_unregister(&device->device);
index 1a3655bea3350b4e84562d331d263e4ca6d3f801..ba0e2463e4a7794f1eb47ed7777a16da125a5b94 100644 (file)
@@ -53,11 +53,18 @@ fw_device(struct device *dev)
        return container_of(dev, struct fw_device, device);
 }
 
+static inline int
+fw_device_is_shutdown(struct fw_device *device)
+{
+       return atomic_read(&device->state) == FW_DEVICE_SHUTDOWN;
+}
+
 struct fw_device *fw_device_get(struct fw_device *device);
 void fw_device_put(struct fw_device *device);
 int fw_device_enable_phys_dma(struct fw_device *device);
 
 void fw_device_cdev_update(struct fw_device *device);
+void fw_device_cdev_remove(struct fw_device *device);
 
 struct fw_device *fw_device_from_devt(dev_t devt);
 extern int fw_cdev_major;