firewire: cdev: reference-count client instances
[cascardo/linux.git] / drivers / firewire / fw-cdev.c
index 094aee5..81362c1 100644 (file)
@@ -20,6 +20,7 @@
 
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/kref.h>
 #include <linux/wait.h>
 #include <linux/errno.h>
 #include <linux/device.h>
@@ -94,16 +95,33 @@ struct client {
        unsigned long vm_start;
 
        struct list_head link;
+       struct kref kref;
 };
 
-static inline void __user *
-u64_to_uptr(__u64 value)
+static inline void client_get(struct client *client)
+{
+       kref_get(&client->kref);
+}
+
+static void client_release(struct kref *kref)
+{
+       struct client *client = container_of(kref, struct client, kref);
+
+       fw_device_put(client->device);
+       kfree(client);
+}
+
+static void client_put(struct client *client)
+{
+       kref_put(&client->kref, client_release);
+}
+
+static inline void __user *u64_to_uptr(__u64 value)
 {
        return (void __user *)(unsigned long)value;
 }
 
-static inline __u64
-uptr_to_u64(void __user *ptr)
+static inline __u64 uptr_to_u64(void __user *ptr)
 {
        return (__u64)(unsigned long)ptr;
 }
@@ -133,6 +151,7 @@ static int fw_device_op_open(struct inode *inode, struct file *file)
        idr_init(&client->resource_idr);
        INIT_LIST_HEAD(&client->event_list);
        init_waitqueue_head(&client->wait);
+       kref_init(&client->kref);
 
        file->private_data = client;
 
@@ -163,26 +182,26 @@ static void queue_event(struct client *client, struct event *event,
        wake_up_interruptible(&client->wait);
 }
 
-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;
+       int i, ret;
 
-       retval = wait_event_interruptible(client->wait,
-                                         !list_empty(&client->event_list) ||
-                                         fw_device_is_shutdown(client->device));
-       if (retval < 0)
-               return retval;
+       ret = wait_event_interruptible(client->wait,
+                       !list_empty(&client->event_list) ||
+                       fw_device_is_shutdown(client->device));
+       if (ret < 0)
+               return ret;
 
        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);
+       event = list_first_entry(&client->event_list, struct event, link);
        list_del(&event->link);
        spin_unlock_irqrestore(&client->lock, flags);
 
@@ -190,31 +209,29 @@ dequeue_event(struct client *client, char __user *buffer, size_t count)
        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)) {
-                       retval = -EFAULT;
+                       ret = -EFAULT;
                        goto out;
                }
                total += size;
        }
-       retval = total;
+       ret = total;
 
  out:
        kfree(event);
 
-       return retval;
+       return ret;
 }
 
-static ssize_t
-fw_device_op_read(struct file *file,
-                 char __user *buffer, size_t count, loff_t *offset)
+static ssize_t fw_device_op_read(struct file *file, char __user *buffer,
+                                size_t count, loff_t *offset)
 {
        struct client *client = file->private_data;
 
        return dequeue_event(client, buffer, count);
 }
 
-static void
-fill_bus_reset_event(struct fw_cdev_event_bus_reset *event,
-                    struct client *client)
+static void fill_bus_reset_event(struct fw_cdev_event_bus_reset *event,
+                                struct client *client)
 {
        struct fw_card *card = client->device->card;
        unsigned long flags;
@@ -233,9 +250,8 @@ fill_bus_reset_event(struct fw_cdev_event_bus_reset *event,
        spin_unlock_irqrestore(&card->lock, flags);
 }
 
-static void
-for_each_client(struct fw_device *device,
-               void (*callback)(struct client *client))
+static void for_each_client(struct fw_device *device,
+                           void (*callback)(struct client *client))
 {
        struct client *c;
 
@@ -245,8 +261,7 @@ for_each_client(struct fw_device *device,
        mutex_unlock(&device->client_list_mutex);
 }
 
-static void
-queue_bus_reset_event(struct client *client)
+static void queue_bus_reset_event(struct client *client)
 {
        struct bus_reset *bus_reset;
 
@@ -316,9 +331,8 @@ static int ioctl_get_info(struct client *client, void *buffer)
        return 0;
 }
 
-static int
-add_client_resource(struct client *client, struct client_resource *resource,
-                   gfp_t gfp_mask)
+static int add_client_resource(struct client *client,
+                              struct client_resource *resource, gfp_t gfp_mask)
 {
        unsigned long flags;
        int ret;
@@ -333,6 +347,8 @@ add_client_resource(struct client *client, struct client_resource *resource,
        else
                ret = idr_get_new(&client->resource_idr, resource,
                                  &resource->handle);
+       if (ret >= 0)
+               client_get(client);
        spin_unlock_irqrestore(&client->lock, flags);
 
        if (ret == -EAGAIN)
@@ -341,10 +357,9 @@ add_client_resource(struct client *client, struct client_resource *resource,
        return ret < 0 ? ret : 0;
 }
 
-static int
-release_client_resource(struct client *client, u32 handle,
-                       client_resource_release_fn_t release,
-                       struct client_resource **resource)
+static int release_client_resource(struct client *client, u32 handle,
+                                  client_resource_release_fn_t release,
+                                  struct client_resource **resource)
 {
        struct client_resource *r;
        unsigned long flags;
@@ -366,11 +381,13 @@ release_client_resource(struct client *client, u32 handle,
        else
                r->release(client, r);
 
+       client_put(client);
+
        return 0;
 }
 
-static void
-release_transaction(struct client *client, struct client_resource *resource)
+static void release_transaction(struct client *client,
+                               struct client_resource *resource)
 {
        struct response *response =
                container_of(resource, struct response, resource);
@@ -378,9 +395,8 @@ release_transaction(struct client *client, struct client_resource *resource)
        fw_cancel_transaction(client->device->card, &response->transaction);
 }
 
-static void
-complete_transaction(struct fw_card *card, int rcode,
-                    void *payload, size_t length, void *data)
+static void complete_transaction(struct fw_card *card, int rcode,
+                                void *payload, size_t length, void *data)
 {
        struct response *response = data;
        struct client *client = response->client;
@@ -394,11 +410,21 @@ complete_transaction(struct fw_card *card, int rcode,
 
        spin_lock_irqsave(&client->lock, flags);
        /*
-        * If called while in shutdown, the idr tree must be left untouched.
-        * The idr handle will be removed later.
+        * 1. If called while in shutdown, the idr tree must be left untouched.
+        *    The idr handle will be removed and the client reference will be
+        *    dropped later.
+        * 2. If the call chain was release_client_resource ->
+        *    release_transaction -> complete_transaction (instead of a normal
+        *    conclusion of the transaction), i.e. if this resource was already
+        *    unregistered from the idr, the client reference will be dropped
+        *    by release_client_resource and we must not drop it here.
         */
-       if (!client->in_shutdown)
+       if (!client->in_shutdown &&
+           idr_find(&client->resource_idr, response->resource.handle)) {
                idr_remove(&client->resource_idr, response->resource.handle);
+               /* Drop the idr's reference */
+               client_put(client);
+       }
        spin_unlock_irqrestore(&client->lock, flags);
 
        r->type   = FW_CDEV_EVENT_RESPONSE;
@@ -417,6 +443,9 @@ complete_transaction(struct fw_card *card, int rcode,
        else
                queue_event(client, &response->event, r, sizeof(*r) + r->length,
                            NULL, 0);
+
+       /* Drop the transaction callback's reference */
+       client_put(client);
 }
 
 static int ioctl_send_request(struct client *client, void *buffer)
@@ -468,6 +497,9 @@ static int ioctl_send_request(struct client *client, void *buffer)
        if (ret < 0)
                goto failed;
 
+       /* Get a reference for the transaction callback */
+       client_get(client);
+
        fw_send_request(device->card, &response->transaction,
                        request->tcode & 0x1f,
                        device->node->node_id,
@@ -506,8 +538,8 @@ struct request_event {
        struct fw_cdev_event_request request;
 };
 
-static void
-release_request(struct client *client, struct client_resource *resource)
+static void release_request(struct client *client,
+                           struct client_resource *resource)
 {
        struct request *request =
                container_of(resource, struct request, resource);
@@ -517,12 +549,11 @@ release_request(struct client *client, struct client_resource *resource)
        kfree(request);
 }
 
-static void
-handle_request(struct fw_card *card, struct fw_request *r,
-              int tcode, int destination, int source,
-              int generation, int speed,
-              unsigned long long offset,
-              void *payload, size_t length, void *callback_data)
+static void handle_request(struct fw_card *card, struct fw_request *r,
+                          int tcode, int destination, int source,
+                          int generation, int speed,
+                          unsigned long long offset,
+                          void *payload, size_t length, void *callback_data)
 {
        struct address_handler *handler = callback_data;
        struct request *request;
@@ -561,9 +592,8 @@ handle_request(struct fw_card *card, struct fw_request *r,
        fw_send_response(card, r, RCODE_CONFLICT_ERROR);
 }
 
-static void
-release_address_handler(struct client *client,
-                       struct client_resource *resource)
+static void release_address_handler(struct client *client,
+                                   struct client_resource *resource)
 {
        struct address_handler *handler =
                container_of(resource, struct address_handler, resource);
@@ -591,9 +621,10 @@ static int ioctl_allocate(struct client *client, void *buffer)
        handler->closure = request->closure;
        handler->client = client;
 
-       if (fw_core_add_address_handler(&handler->handler, &region) < 0) {
+       ret = fw_core_add_address_handler(&handler->handler, &region);
+       if (ret < 0) {
                kfree(handler);
-               return -EBUSY;
+               return ret;
        }
 
        handler->resource.release = release_address_handler;
@@ -715,9 +746,8 @@ static int ioctl_remove_descriptor(struct client *client, void *buffer)
                                       release_descriptor, NULL);
 }
 
-static void
-iso_callback(struct fw_iso_context *context, u32 cycle,
-            size_t header_length, void *header, void *data)
+static void iso_callback(struct fw_iso_context *context, u32 cycle,
+                        size_t header_length, void *header, void *data)
 {
        struct client *client = data;
        struct iso_interrupt *irq;
@@ -953,11 +983,11 @@ static int (* const ioctl_handlers[])(struct client *client, void *buffer) = {
        ioctl_get_cycle_timer,
 };
 
-static int
-dispatch_ioctl(struct client *client, unsigned int cmd, void __user *arg)
+static int dispatch_ioctl(struct client *client,
+                         unsigned int cmd, void __user *arg)
 {
        char buffer[256];
-       int retval;
+       int ret;
 
        if (_IOC_TYPE(cmd) != '#' ||
            _IOC_NR(cmd) >= ARRAY_SIZE(ioctl_handlers))
@@ -969,9 +999,9 @@ dispatch_ioctl(struct client *client, unsigned int cmd, void __user *arg)
                        return -EFAULT;
        }
 
-       retval = ioctl_handlers[_IOC_NR(cmd)](client, buffer);
-       if (retval < 0)
-               return retval;
+       ret = ioctl_handlers[_IOC_NR(cmd)](client, buffer);
+       if (ret < 0)
+               return ret;
 
        if (_IOC_DIR(cmd) & _IOC_READ) {
                if (_IOC_SIZE(cmd) > sizeof(buffer) ||
@@ -979,12 +1009,11 @@ dispatch_ioctl(struct client *client, unsigned int cmd, void __user *arg)
                        return -EFAULT;
        }
 
-       return retval;
+       return ret;
 }
 
-static long
-fw_device_op_ioctl(struct file *file,
-                  unsigned int cmd, unsigned long arg)
+static long fw_device_op_ioctl(struct file *file,
+                              unsigned int cmd, unsigned long arg)
 {
        struct client *client = file->private_data;
 
@@ -995,9 +1024,8 @@ fw_device_op_ioctl(struct file *file,
 }
 
 #ifdef CONFIG_COMPAT
-static long
-fw_device_op_compat_ioctl(struct file *file,
-                         unsigned int cmd, unsigned long arg)
+static long fw_device_op_compat_ioctl(struct file *file,
+                                     unsigned int cmd, unsigned long arg)
 {
        struct client *client = file->private_data;
 
@@ -1013,7 +1041,7 @@ static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma)
        struct client *client = file->private_data;
        enum dma_data_direction direction;
        unsigned long size;
-       int page_count, retval;
+       int page_count, ret;
 
        if (fw_device_is_shutdown(client->device))
                return -ENODEV;
@@ -1039,16 +1067,16 @@ static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma)
        else
                direction = DMA_FROM_DEVICE;
 
-       retval = fw_iso_buffer_init(&client->buffer, client->device->card,
-                                   page_count, direction);
-       if (retval < 0)
-               return retval;
+       ret = fw_iso_buffer_init(&client->buffer, client->device->card,
+                                page_count, direction);
+       if (ret < 0)
+               return ret;
 
-       retval = fw_iso_buffer_map(&client->buffer, vma);
-       if (retval < 0)
+       ret = fw_iso_buffer_map(&client->buffer, vma);
+       if (ret < 0)
                fw_iso_buffer_destroy(&client->buffer, client->device->card);
 
-       return retval;
+       return ret;
 }
 
 static int shutdown_resource(int id, void *p, void *data)
@@ -1057,6 +1085,7 @@ static int shutdown_resource(int id, void *p, void *data)
        struct client *client = data;
 
        r->release(client, r);
+       client_put(client);
 
        return 0;
 }
@@ -1089,12 +1118,7 @@ static int fw_device_op_release(struct inode *inode, struct file *file)
        list_for_each_entry_safe(e, next_e, &client->event_list, link)
                kfree(e);
 
-       /*
-        * FIXME: client should be reference-counted.  It's extremely unlikely
-        * but there may still be transactions being completed at this point.
-        */
-       fw_device_put(client->device);
-       kfree(client);
+       client_put(client);
 
        return 0;
 }