# HG changeset patch
# User Martin Troxler <ch1010277@ch10pc446>
# Date 1294226793 -3600
# Node ID 7a025a9e192de00b10536a70217463c970173c26
# Parent  2bd8ad8bf41fd74f611ddde2981df4c9b6415c46
Register read/write wait_event() deadlock fixed: use wait_event_interruptible and refcount ec_reg_request_t objects with kref

diff -r 2bd8ad8bf41f -r 7a025a9e192d master/cdev.c
--- a/master/cdev.c	Wed Jan 05 11:33:31 2011 +0100
+++ b/master/cdev.c	Wed Jan 05 12:26:33 2011 +0100
@@ -1154,7 +1154,8 @@
     ec_ioctl_slave_reg_t data;
     ec_slave_t *slave;
     uint8_t *contents;
-    ec_reg_request_t request;
+    ec_reg_request_t* request;
+    int retval;
 
     if (copy_from_user(&data, (void __user *) arg, sizeof(data))) {
         return -EFAULT;
@@ -1169,56 +1170,58 @@
         return -ENOMEM;
     }
 
-    if (ec_mutex_lock_interruptible(&master->master_mutex))
-        return -EINTR;
-
+    request = kmalloc(sizeof(*request), GFP_KERNEL);
+    if (!request)
+        return -ENOMEM;
+    kref_init(&request->refcount);
+
+    // init register request
+    INIT_LIST_HEAD(&request->list);
+    request->dir = EC_DIR_INPUT;
+    request->data = contents;   // now "owned" by request, see ec_master_reg_request_release
+    request->offset = data.offset;
+    request->length = data.length;
+
+    if (ec_mutex_lock_interruptible(&master->master_mutex)) {
+        kref_put(&request->refcount,ec_master_reg_request_release);
+        return -EINTR;
+    }
     if (!(slave = ec_master_find_slave(
                     master, 0, data.slave_position))) {
         ec_mutex_unlock(&master->master_mutex);
         EC_MASTER_ERR(master, "Slave %u does not exist!\n",
                 data.slave_position);
-        return -EINVAL;
-    }
-
-    // init register request
-    INIT_LIST_HEAD(&request.list);
-    request.slave = slave;
-    request.dir = EC_DIR_INPUT;
-    request.data = contents;
-    request.offset = data.offset;
-    request.length = data.length;
-    request.state = EC_INT_REQUEST_QUEUED;
+        kref_put(&request->refcount,ec_master_reg_request_release);
+        return -EINVAL;
+    }
+
+    request->slave = slave;
+    request->state = EC_INT_REQUEST_QUEUED;
 
     // schedule request.
-    list_add_tail(&request.list, &master->reg_requests);
+    list_add_tail(&request->list, &master->reg_requests);
+    kref_get(&request->refcount);
 
     ec_mutex_unlock(&master->master_mutex);
 
     // wait for processing through FSM
     if (wait_event_interruptible(master->reg_queue,
-                request.state != EC_INT_REQUEST_QUEUED)) {
-        // interrupted by signal
-        ec_mutex_lock(&master->master_mutex);
-        if (request.state == EC_INT_REQUEST_QUEUED) {
-            // abort request
-            list_del(&request.list);
-            ec_mutex_unlock(&master->master_mutex);
-            kfree(contents);
-            return -EINTR;
+          ((request->state == EC_INT_REQUEST_SUCCESS) || (request->state == EC_INT_REQUEST_FAILURE)))) {
+           // interrupted by signal
+           kref_put(&request->refcount,ec_master_reg_request_release);
+           return -EINTR;
+    }
+
+    if (request->state == EC_INT_REQUEST_SUCCESS) {
+        if (copy_to_user((void __user *) data.data, request->data, data.length)) {
+            kref_put(&request->refcount,ec_master_reg_request_release);
+            return -EFAULT;
         }
-        ec_mutex_unlock(&master->master_mutex);
-    }
-
-    // wait until master FSM has finished processing
-    wait_event(master->reg_queue, request.state != EC_INT_REQUEST_BUSY);
-
-    if (request.state == EC_INT_REQUEST_SUCCESS) {
-        if (copy_to_user((void __user *) data.data, contents, data.length))
-            return -EFAULT;
-    }
-    kfree(contents);
-
-    return request.state == EC_INT_REQUEST_SUCCESS ? 0 : -EIO;
+    }
+    retval = request->state == EC_INT_REQUEST_SUCCESS ? 0 : -EIO;
+
+    kref_put(&request->refcount,ec_master_reg_request_release);
+    return retval;
 }
 
 /*****************************************************************************/
@@ -1233,7 +1236,8 @@
     ec_ioctl_slave_reg_t data;
     ec_slave_t *slave;
     uint8_t *contents;
-    ec_reg_request_t request;
+    ec_reg_request_t* request;
+    int retval;
 
     if (copy_from_user(&data, (void __user *) arg, sizeof(data))) {
         return -EFAULT;
@@ -1253,53 +1257,52 @@
         return -EFAULT;
     }
 
-    if (ec_mutex_lock_interruptible(&master->master_mutex))
-        return -EINTR;
+    request = kmalloc(sizeof(*request), GFP_KERNEL);
+    if (!request)
+        return -ENOMEM;
+    kref_init(&request->refcount);
+    // init register request
+    INIT_LIST_HEAD(&request->list);
+    request->dir = EC_DIR_OUTPUT;
+    request->data = contents; // now "owned" by request, see ec_master_reg_request_release
+    request->offset = data.offset;
+    request->length = data.length;
+
+    if (ec_mutex_lock_interruptible(&master->master_mutex)) {
+        kref_put(&request->refcount,ec_master_reg_request_release);
+        return -EINTR;
+    }
 
     if (!(slave = ec_master_find_slave(
                     master, 0, data.slave_position))) {
         ec_mutex_unlock(&master->master_mutex);
         EC_MASTER_ERR(master, "Slave %u does not exist!\n",
                 data.slave_position);
-        kfree(contents);
-        return -EINVAL;
-    }
-
-    // init register request
-    INIT_LIST_HEAD(&request.list);
-    request.slave = slave;
-    request.dir = EC_DIR_OUTPUT;
-    request.data = contents;
-    request.offset = data.offset;
-    request.length = data.length;
-    request.state = EC_INT_REQUEST_QUEUED;
+        kref_put(&request->refcount,ec_master_reg_request_release);
+        return -EINVAL;
+    }
+
+    request->slave = slave;
+    request->state = EC_INT_REQUEST_QUEUED;
 
     // schedule request.
-    list_add_tail(&request.list, &master->reg_requests);
+    list_add_tail(&request->list, &master->reg_requests);
+    kref_get(&request->refcount);
 
     ec_mutex_unlock(&master->master_mutex);
 
     // wait for processing through FSM
     if (wait_event_interruptible(master->reg_queue,
-                request.state != EC_INT_REQUEST_QUEUED)) {
-        // interrupted by signal
-        ec_mutex_lock(&master->master_mutex);
-        if (request.state == EC_INT_REQUEST_QUEUED) {
-            // abort request
-            list_del(&request.list);
-            ec_mutex_unlock(&master->master_mutex);
-            kfree(contents);
-            return -EINTR;
-        }
-        ec_mutex_unlock(&master->master_mutex);
-    }
-
-    // wait until master FSM has finished processing
-    wait_event(master->reg_queue, request.state != EC_INT_REQUEST_BUSY);
-
-    kfree(contents);
-
-    return request.state == EC_INT_REQUEST_SUCCESS ? 0 : -EIO;
+          ((request->state == EC_INT_REQUEST_SUCCESS) || (request->state == EC_INT_REQUEST_FAILURE)))) {
+           // interrupted by signal
+           kref_put(&request->refcount,ec_master_reg_request_release);
+           return -EINTR;
+    }
+
+    retval = request->state == EC_INT_REQUEST_SUCCESS ? 0 : -EIO;
+    kref_put(&request->refcount,ec_master_reg_request_release);
+    return retval;
+
 }
 
 /*****************************************************************************/
diff -r 2bd8ad8bf41f -r 7a025a9e192d master/fsm_master.c
--- a/master/fsm_master.c	Wed Jan 05 11:33:31 2011 +0100
+++ b/master/fsm_master.c	Wed Jan 05 12:26:33 2011 +0100
@@ -391,6 +391,7 @@
                     "datagram size (%zu)!\n", request->length,
                     fsm->datagram->mem_size);
             request->state = EC_INT_REQUEST_FAILURE;
+            kref_put(&request->refcount,ec_master_reg_request_release);
             wake_up(&master->reg_queue);
             continue;
         }
@@ -1191,6 +1192,7 @@
                 " request datagram: ");
         ec_datagram_print_state(datagram);
         request->state = EC_INT_REQUEST_FAILURE;
+        kref_put(&request->refcount,ec_master_reg_request_release);
         wake_up(&master->reg_queue);
         ec_fsm_master_restart(fsm);
         return;
@@ -1205,6 +1207,7 @@
                 EC_MASTER_ERR(master, "Failed to allocate %zu bytes"
                         " of memory for register data.\n", request->length);
                 request->state = EC_INT_REQUEST_FAILURE;
+                kref_put(&request->refcount,ec_master_reg_request_release);
                 wake_up(&master->reg_queue);
                 ec_fsm_master_restart(fsm);
                 return;
@@ -1219,6 +1222,7 @@
         EC_MASTER_ERR(master, "Register request failed.\n");
     }
 
+    kref_put(&request->refcount,ec_master_reg_request_release);
     wake_up(&master->reg_queue);
 
     // check for another register request
@@ -1243,6 +1247,20 @@
 
 /*****************************************************************************/
 
+/** called by kref_put if the reg request's refcount becomes zero.
+ *
+ */
+void ec_master_reg_request_release(struct kref *ref)
+{
+    ec_reg_request_t *request = container_of(ref, ec_reg_request_t, refcount);
+    EC_SLAVE_DBG(request->slave, 1, "Releasing reg request %p.\n",request);
+    if (request->data)
+        kfree(request->data);
+    kfree(request);
+}
+
+/*****************************************************************************/
+
 /** called by kref_put if the SDO request's refcount becomes zero.
  *
  */
diff -r 2bd8ad8bf41f -r 7a025a9e192d master/fsm_master.h
--- a/master/fsm_master.h	Wed Jan 05 11:33:31 2011 +0100
+++ b/master/fsm_master.h	Wed Jan 05 12:26:33 2011 +0100
@@ -74,8 +74,10 @@
     size_t length; /**< Number of bytes. */
     uint8_t *data; /**< Data to write / memory for read data. */
     ec_internal_request_state_t state; /**< State of the request. */
+    struct kref refcount;
 } ec_reg_request_t;
 
+void ec_master_reg_request_release(struct kref *);
 
 /*****************************************************************************/
 
diff -r 2bd8ad8bf41f -r 7a025a9e192d master/master.c
--- a/master/master.c	Wed Jan 05 11:33:31 2011 +0100
+++ b/master/master.c	Wed Jan 05 12:26:33 2011 +0100
@@ -408,6 +408,7 @@
         EC_MASTER_WARN(master, "Discarding register request, slave %u"
                 " about to be deleted.\n", request->slave->ring_position);
         request->state = EC_INT_REQUEST_FAILURE;
+        kref_put(&request->refcount,ec_master_reg_request_release);
         wake_up(&master->reg_queue);
     }