# HG changeset patch
# User Martin Troxler <ch1010277@ch10pc446>
# Date 1293094136 -3600
# Node ID ac35f4d38a310fa5335684217b75bb3770d869cd
# Parent  68c1c31522a22eabb89f78badec05584e03a3c75
SDO wait_event() deadlock fixed: refcount ec_master_sdo_request_t objects with kref

diff -r 68c1c31522a2 -r ac35f4d38a31 master/cdev.c
--- a/master/cdev.c	Thu Dec 23 09:28:39 2010 +0100
+++ b/master/cdev.c	Thu Dec 23 09:48:56 2010 +0100
@@ -840,78 +840,74 @@
         )
 {
     ec_ioctl_slave_sdo_upload_t data;
-    ec_master_sdo_request_t request;
+    ec_master_sdo_request_t* request;
     int retval;
 
     if (copy_from_user(&data, (void __user *) arg, sizeof(data))) {
         return -EFAULT;
     }
 
-    ec_sdo_request_init(&request.req);
-    ec_sdo_request_address(&request.req,
-            data.sdo_index, data.sdo_entry_subindex);
-    ecrt_sdo_request_read(&request.req);
-
-    if (down_interruptible(&master->master_sem))
-        return -EINTR;
-
-    if (!(request.slave = ec_master_find_slave(
+    request = kmalloc(sizeof(*request), GFP_KERNEL);
+    if (!request)
+        return -ENOMEM;
+    kref_init(&request->refcount);
+
+    ec_sdo_request_init(&request->req);
+    ec_sdo_request_address(&request->req,
+        data.sdo_index, data.sdo_entry_subindex);
+    ecrt_sdo_request_read(&request->req);
+
+    if (down_interruptible(&master->master_sem))  {
+        kref_put(&request->refcount,ec_master_sdo_request_release);
+        return -EINTR;
+    }
+    if (!(request->slave = ec_master_find_slave(
                     master, 0, data.slave_position))) {
         up(&master->master_sem);
-        ec_sdo_request_clear(&request.req);
+        kref_put(&request->refcount,ec_master_sdo_request_release);
         EC_MASTER_ERR(master, "Slave %u does not exist!\n",
                 data.slave_position);
         return -EINVAL;
     }
 
-    EC_SLAVE_DBG(request.slave, 1, "Schedule SDO upload request.\n");
+    EC_SLAVE_DBG(request->slave, 1, "Schedule SDO upload request %p.\n",request);
 
     // schedule request.
-    list_add_tail(&request.list, &request.slave->slave_sdo_requests);
+    kref_get(&request->refcount);
+    list_add_tail(&request->list, &request->slave->slave_sdo_requests);
 
     up(&master->master_sem);
 
     // wait for processing through FSM
-    if (wait_event_interruptible(request.slave->sdo_queue,
-                request.req.state != EC_INT_REQUEST_QUEUED)) {
+    if (wait_event_interruptible(request->slave->sdo_queue,
+          ((request->req.state == EC_INT_REQUEST_SUCCESS) || (request->req.state == EC_INT_REQUEST_FAILURE)))) {
         // interrupted by signal
-        down(&master->master_sem);
-        if (request.req.state == EC_INT_REQUEST_QUEUED) {
-            list_del(&request.list);
-            up(&master->master_sem);
-            ec_sdo_request_clear(&request.req);
-            return -EINTR;
-        }
-        // request already processing: interrupt not possible.
-        up(&master->master_sem);
-    }
-
-    // wait until master FSM has finished processing
-    wait_event(request.slave->sdo_queue,
-            request.req.state != EC_INT_REQUEST_BUSY);
-
-    EC_SLAVE_DBG(request.slave, 1, "Finished SDO upload request.\n");
-
-    data.abort_code = request.req.abort_code;
-
-    if (request.req.state != EC_INT_REQUEST_SUCCESS) {
+        kref_put(&request->refcount,ec_master_sdo_request_release);
+        return -EINTR;
+    }
+
+    EC_SLAVE_DBG(request->slave, 1, "Finished SDO upload request %p.\n",request);
+
+    data.abort_code = request->req.abort_code;
+
+    if (request->req.state != EC_INT_REQUEST_SUCCESS) {
         data.data_size = 0;
-        if (request.req.errno) {
-            retval = -request.req.errno;
+        if (request->req.errno) {
+            retval = -request->req.errno;
         } else {
             retval = -EIO;
         }
     } else {
-        if (request.req.data_size > data.target_size) {
+        if (request->req.data_size > data.target_size) {
             EC_MASTER_ERR(master, "Buffer too small.\n");
-            ec_sdo_request_clear(&request.req);
+            kref_put(&request->refcount,ec_master_sdo_request_release);
             return -EOVERFLOW;
         }
-        data.data_size = request.req.data_size;
+        data.data_size = request->req.data_size;
 
         if (copy_to_user((void __user *) data.target,
-                    request.req.data, data.data_size)) {
-            ec_sdo_request_clear(&request.req);
+                    request->req.data, data.data_size)) {
+            kref_put(&request->refcount,ec_master_sdo_request_release);
             return -EFAULT;
         }
         retval = 0;
@@ -921,7 +917,7 @@
         retval = -EFAULT;
     }
 
-    ec_sdo_request_clear(&request.req);
+    kref_put(&request->refcount,ec_master_sdo_request_release);
     return retval;
 }
 
@@ -935,7 +931,7 @@
         )
 {
     ec_ioctl_slave_sdo_download_t data;
-    ec_master_sdo_request_t request;
+    ec_master_sdo_request_t* request;
     int retval;
 
     if (copy_from_user(&data, (void __user *) arg, sizeof(data))) {
@@ -948,67 +944,63 @@
         return -EINVAL;
     }
 
-    ec_sdo_request_init(&request.req);
-    ec_sdo_request_address(&request.req,
+    request = kmalloc(sizeof(*request), GFP_KERNEL);
+    if (!request)
+        return -ENOMEM;
+    kref_init(&request->refcount);
+
+    ec_sdo_request_init(&request->req);
+    ec_sdo_request_address(&request->req,
             data.sdo_index, data.sdo_entry_subindex);
-    if (ec_sdo_request_alloc(&request.req, data.data_size)) {
-        ec_sdo_request_clear(&request.req);
+    if (ec_sdo_request_alloc(&request->req, data.data_size)) {
+        kref_put(&request->refcount,ec_master_sdo_request_release);
         return -ENOMEM;
     }
-    if (copy_from_user(request.req.data,
+    if (copy_from_user(request->req.data,
                 (void __user *) data.data, data.data_size)) {
-        ec_sdo_request_clear(&request.req);
-        return -EFAULT;
-    }
-    request.req.data_size = data.data_size;
-    ecrt_sdo_request_write(&request.req);
-
-    if (down_interruptible(&master->master_sem))
-        return -EINTR;
-
-    if (!(request.slave = ec_master_find_slave(
+        kref_put(&request->refcount,ec_master_sdo_request_release);
+        return -EFAULT;
+    }
+    request->req.data_size = data.data_size;
+    ecrt_sdo_request_write(&request->req);
+
+    if (down_interruptible(&master->master_sem)) {
+        kref_put(&request->refcount,ec_master_sdo_request_release);
+        return -EINTR;
+    }
+    if (!(request->slave = ec_master_find_slave(
                     master, 0, data.slave_position))) {
         up(&master->master_sem);
         EC_MASTER_ERR(master, "Slave %u does not exist!\n",
                 data.slave_position);
-        ec_sdo_request_clear(&request.req);
+        kref_put(&request->refcount,ec_master_sdo_request_release);
         return -EINVAL;
     }
     
-    EC_SLAVE_DBG(request.slave, 1, "Schedule SDO download request.\n");
+    EC_SLAVE_DBG(request->slave, 1, "Schedule SDO download request %p.\n",request);
 
     // schedule request.
-    list_add_tail(&request.list, &request.slave->slave_sdo_requests);
+    kref_get(&request->refcount);
+    list_add_tail(&request->list, &request->slave->slave_sdo_requests);
 
     up(&master->master_sem);
 
     // wait for processing through FSM
-    if (wait_event_interruptible(request.slave->sdo_queue,
-                request.req.state != EC_INT_REQUEST_QUEUED)) {
+    if (wait_event_interruptible(request->slave->sdo_queue,
+       ((request->req.state == EC_INT_REQUEST_SUCCESS) || (request->req.state == EC_INT_REQUEST_FAILURE)))) {
         // interrupted by signal
-        down(&master->master_sem);
-        if (request.req.state == EC_INT_REQUEST_QUEUED) {
-            list_del(&request.list);
-            up(&master->master_sem);
-            ec_sdo_request_clear(&request.req);
-            return -EINTR;
-        }
-        // request already processing: interrupt not possible.
-        up(&master->master_sem);
-    }
-
-    // wait until master FSM has finished processing
-    wait_event(request.slave->sdo_queue,
-            request.req.state != EC_INT_REQUEST_BUSY);
-
-    EC_SLAVE_DBG(request.slave, 1, "Finished SDO download request.\n");
-
-    data.abort_code = request.req.abort_code;
-
-    if (request.req.state == EC_INT_REQUEST_SUCCESS) {
+        kref_put(&request->refcount,ec_master_sdo_request_release);
+        return -EINTR;
+    }
+
+    EC_SLAVE_DBG(request->slave, 1, "Finished SDO download request %p.\n",request);
+
+    data.abort_code = request->req.abort_code;
+
+    if (request->req.state == EC_INT_REQUEST_SUCCESS) {
         retval = 0;
-    } else if (request.req.errno) {
-        retval = -request.req.errno;
+    } else if (request->req.errno) {
+        retval = -request->req.errno;
     } else {
         retval = -EIO;
     }
@@ -1017,7 +1009,7 @@
         retval = -EFAULT;
     }
 
-    ec_sdo_request_clear(&request.req);
+    kref_put(&request->refcount,ec_master_sdo_request_release);
     return retval;
 }
 
diff -r 68c1c31522a2 -r ac35f4d38a31 master/fsm_master.c
--- a/master/fsm_master.c	Thu Dec 23 09:28:39 2010 +0100
+++ b/master/fsm_master.c	Thu Dec 23 09:48:56 2010 +0100
@@ -1227,3 +1227,14 @@
 }
 
 /*****************************************************************************/
+
+/** called by kref_put if the request's refcount becomes zero.
+ *
+ */
+void ec_master_sdo_request_release(struct kref *ref)
+{
+    ec_master_sdo_request_t *request = container_of(ref, ec_master_sdo_request_t, refcount);
+    EC_SLAVE_DBG(request->slave, 1, "Releasing request %p.\n",request);
+    ec_sdo_request_clear(&request->req);
+    kfree(request);
+}
diff -r 68c1c31522a2 -r ac35f4d38a31 master/fsm_master.h
--- a/master/fsm_master.h	Thu Dec 23 09:28:39 2010 +0100
+++ b/master/fsm_master.h	Thu Dec 23 09:48:56 2010 +0100
@@ -73,6 +73,8 @@
     ec_internal_request_state_t state; /**< State of the request. */
 } ec_reg_request_t;
 
+void ec_master_sdo_request_release(struct kref *);
+
 /*****************************************************************************/
 
 /** Slave/SDO request record for master's SDO request list.
@@ -81,6 +83,7 @@
     struct list_head list; /**< List element. */
     ec_slave_t *slave; /**< Slave. */
     ec_sdo_request_t req; /**< SDO request. */
+    struct kref refcount;
 } ec_master_sdo_request_t;
 
 /*****************************************************************************/
diff -r 68c1c31522a2 -r ac35f4d38a31 master/fsm_slave.c
--- a/master/fsm_slave.c	Thu Dec 23 09:28:39 2010 +0100
+++ b/master/fsm_slave.c	Thu Dec 23 09:48:56 2010 +0100
@@ -178,9 +178,10 @@
 
         list_del_init(&request->list); // dequeue
         if (slave->current_state & EC_SLAVE_STATE_ACK_ERR) {
-            EC_SLAVE_WARN(slave, "Aborting SDO request,"
-                    " slave has error flag set.\n");
+            EC_SLAVE_WARN(slave, "Aborting SDO request %p,"
+                    " slave has error flag set.\n",request);
             request->req.state = EC_INT_REQUEST_FAILURE;
+            kref_put(&request->refcount,ec_master_sdo_request_release);
             wake_up(&slave->sdo_queue);
             fsm->sdo_request = NULL;
             fsm->state = ec_fsm_slave_state_idle;
@@ -188,8 +189,9 @@
         }
 
         if (slave->current_state == EC_SLAVE_STATE_INIT) {
-            EC_SLAVE_WARN(slave, "Aborting SDO request, slave is in INIT.\n");
+            EC_SLAVE_WARN(slave, "Aborting SDO request %p, slave is in INIT.\n",request);
             request->req.state = EC_INT_REQUEST_FAILURE;
+            kref_put(&request->refcount,ec_master_sdo_request_release);
             wake_up(&slave->sdo_queue);
             fsm->sdo_request = NULL;
             fsm->state = ec_fsm_slave_state_idle;
@@ -199,10 +201,10 @@
         request->req.state = EC_INT_REQUEST_BUSY;
 
         // Found pending SDO request. Execute it!
-        EC_SLAVE_DBG(slave, 1, "Processing SDO request...\n");
+        EC_SLAVE_DBG(slave, 1, "Processing SDO request %p...\n",request);
 
         // Start SDO transfer
-        fsm->sdo_request = &request->req;
+        fsm->sdo_request = request;
         fsm->state = ec_fsm_slave_state_sdo_request;
         ec_fsm_coe_transfer(&fsm->fsm_coe, slave, &request->req);
         ec_fsm_coe_exec(&fsm->fsm_coe); // execute immediately
@@ -221,7 +223,7 @@
         )
 {
     ec_slave_t *slave = fsm->slave;
-    ec_sdo_request_t *request = fsm->sdo_request;
+    ec_master_sdo_request_t *request = fsm->sdo_request;
 
     if (ec_fsm_coe_exec(&fsm->fsm_coe))
     {
@@ -229,18 +231,20 @@
         return;
     }
     if (!ec_fsm_coe_success(&fsm->fsm_coe)) {
-        EC_SLAVE_ERR(slave, "Failed to process SDO request.\n");
-        request->state = EC_INT_REQUEST_FAILURE;
+        EC_SLAVE_ERR(slave, "Failed to process SDO request %p.\n",request);
+        request->req.state = EC_INT_REQUEST_FAILURE;
+        kref_put(&request->refcount,ec_master_sdo_request_release);
         wake_up(&slave->sdo_queue);
         fsm->sdo_request = NULL;
         fsm->state = ec_fsm_slave_state_idle;
         return;
     }
 
-    EC_SLAVE_DBG(slave, 1, "Finished SDO request.\n");
+    EC_SLAVE_DBG(slave, 1, "Finished SDO request %p.\n",request);
 
     // SDO request finished
-    request->state = EC_INT_REQUEST_SUCCESS;
+    request->req.state = EC_INT_REQUEST_SUCCESS;
+    kref_put(&request->refcount,ec_master_sdo_request_release);
     wake_up(&slave->sdo_queue);
 
     fsm->sdo_request = NULL;
diff -r 68c1c31522a2 -r ac35f4d38a31 master/fsm_slave.h
--- a/master/fsm_slave.h	Thu Dec 23 09:28:39 2010 +0100
+++ b/master/fsm_slave.h	Thu Dec 23 09:48:56 2010 +0100
@@ -42,6 +42,7 @@
 #include "fsm_coe.h"
 #include "fsm_foe.h"
 #include "fsm_soe.h"
+#include "fsm_master.h"
 
 typedef struct ec_fsm_slave ec_fsm_slave_t; /**< \see ec_fsm_slave */
 
@@ -52,7 +53,7 @@
     ec_datagram_t *datagram; /**< datagram used in the state machine */
 
     void (*state)(ec_fsm_slave_t *); /**< master state function */
-    ec_sdo_request_t *sdo_request; /**< SDO request to process. */
+    ec_master_sdo_request_t *sdo_request; /**< SDO request to process. */
     ec_foe_request_t *foe_request; /**< FoE request to process. */
     off_t foe_index; /**< index to FoE write request data */
     ec_soe_request_t *soe_request; /**< SoE request to process. */
diff -r 68c1c31522a2 -r ac35f4d38a31 master/slave.c
--- a/master/slave.c	Thu Dec 23 09:28:39 2010 +0100
+++ b/master/slave.c	Thu Dec 23 09:48:56 2010 +0100
@@ -194,9 +194,10 @@
             list_entry(slave->slave_sdo_requests.next,
                 ec_master_sdo_request_t, list);
         list_del_init(&request->list); // dequeue
-        EC_SLAVE_WARN(slave, "Discarding SDO request,"
-                " slave about to be deleted.\n");
+        EC_SLAVE_WARN(slave, "Discarding SDO request %p,"
+                " slave about to be deleted.\n",request);
         request->req.state = EC_INT_REQUEST_FAILURE;
+        kref_put(&request->refcount,ec_master_sdo_request_release);
         wake_up(&slave->sdo_queue);
     }