SoE request wait_event() deadlock fixed: use wait_event_interruptible and refcount ec_master_soe_request_t objects with kref
authorMartin Troxler <ch1010277@ch10pc446>
Wed, 05 Jan 2011 12:46:12 +0100
changeset 2032 57c618557912
parent 2031 7a025a9e192d
child 2033 b67eb5c26716
SoE request wait_event() deadlock fixed: use wait_event_interruptible and refcount ec_master_soe_request_t objects with kref
master/fsm_master.c
master/fsm_master.h
master/fsm_slave.c
master/fsm_slave.h
master/master.c
master/slave.c
--- a/master/fsm_master.c	Wed Jan 05 12:26:33 2011 +0100
+++ b/master/fsm_master.c	Wed Jan 05 12:46:12 2011 +0100
@@ -1284,3 +1284,16 @@
     ec_foe_request_clear(&request->req);
     kfree(request);
 }
+
+/*****************************************************************************/
+
+/** called by kref_put if the SoE request's refcount becomes zero.
+ *
+ */
+void ec_master_soe_request_release(struct kref *ref)
+{
+    ec_master_soe_request_t *request = container_of(ref, ec_master_soe_request_t, refcount);
+    EC_SLAVE_DBG(request->slave, 1, "Releasing SoE request %p.\n",request);
+    ec_soe_request_clear(&request->req);
+    kfree(request);
+}
--- a/master/fsm_master.h	Wed Jan 05 12:26:33 2011 +0100
+++ b/master/fsm_master.h	Wed Jan 05 12:46:12 2011 +0100
@@ -113,8 +113,11 @@
     struct list_head list; /**< List head. */
     ec_slave_t *slave; /**< EtherCAT slave. */
     ec_soe_request_t req; /**< SoE request. */
+    struct kref refcount;
 } ec_master_soe_request_t;
 
+void ec_master_soe_request_release(struct kref *);
+
 /*****************************************************************************/
 
 typedef struct ec_fsm_master ec_fsm_master_t; /**< \see ec_fsm_master */
--- a/master/fsm_slave.c	Wed Jan 05 12:26:33 2011 +0100
+++ b/master/fsm_slave.c	Wed Jan 05 12:46:12 2011 +0100
@@ -341,16 +341,17 @@
         )
 {
     ec_slave_t *slave = fsm->slave;
-    ec_master_soe_request_t *req, *next;
+    ec_master_soe_request_t *request, *next;
 
     // search the first request to be processed
-    list_for_each_entry_safe(req, next, &slave->soe_requests, list) {
-
-        list_del_init(&req->list); // dequeue
+    list_for_each_entry_safe(request, next, &slave->soe_requests, list) {
+
+        list_del_init(&request->list); // dequeue
         if (slave->current_state & EC_SLAVE_STATE_ACK_ERR) {
             EC_SLAVE_WARN(slave, "Aborting SoE request,"
                     " slave has error flag set.\n");
-            req->req.state = EC_INT_REQUEST_FAILURE;
+            request->req.state = EC_INT_REQUEST_FAILURE;
+            kref_put(&request->refcount,ec_master_soe_request_release);
             wake_up(&slave->soe_queue);
             fsm->state = ec_fsm_slave_state_idle;
             return 0;
@@ -358,21 +359,22 @@
 
         if (slave->current_state == EC_SLAVE_STATE_INIT) {
             EC_SLAVE_WARN(slave, "Aborting SoE request, slave is in INIT.\n");
-            req->req.state = EC_INT_REQUEST_FAILURE;
+            request->req.state = EC_INT_REQUEST_FAILURE;
+            kref_put(&request->refcount,ec_master_soe_request_release);
             wake_up(&slave->soe_queue);
             fsm->state = ec_fsm_slave_state_idle;
             return 0;
         }
 
-        req->req.state = EC_INT_REQUEST_BUSY;
+        request->req.state = EC_INT_REQUEST_BUSY;
 
         // Found pending request. Execute it!
         EC_SLAVE_DBG(slave, 1, "Processing SoE request...\n");
 
         // Start SoE transfer
-        fsm->soe_request = &req->req;
+        fsm->soe_request = request;
         fsm->state = ec_fsm_slave_state_soe_request;
-        ec_fsm_soe_transfer(&fsm->fsm_soe, slave, &req->req);
+        ec_fsm_soe_transfer(&fsm->fsm_soe, slave, &request->req);
         ec_fsm_soe_exec(&fsm->fsm_soe); // execute immediately
         ec_master_queue_request_fsm_datagram(fsm->slave->master, fsm->datagram);
         return 1;
@@ -389,7 +391,7 @@
         )
 {
     ec_slave_t *slave = fsm->slave;
-    ec_soe_request_t *request = fsm->soe_request;
+    ec_master_soe_request_t *request = fsm->soe_request;
 
     if (ec_fsm_soe_exec(&fsm->fsm_soe)) {
         ec_master_queue_request_fsm_datagram(fsm->slave->master, fsm->datagram);
@@ -398,7 +400,8 @@
 
     if (!ec_fsm_soe_success(&fsm->fsm_soe)) {
         EC_SLAVE_ERR(slave, "Failed to process SoE request.\n");
-        request->state = EC_INT_REQUEST_FAILURE;
+        request->req.state = EC_INT_REQUEST_FAILURE;
+        kref_put(&request->refcount,ec_master_soe_request_release);
         wake_up(&slave->soe_queue);
         fsm->soe_request = NULL;
         fsm->state = ec_fsm_slave_state_idle;
@@ -408,7 +411,8 @@
     EC_SLAVE_DBG(slave, 1, "Finished SoE request.\n");
 
     // SoE request finished
-    request->state = EC_INT_REQUEST_SUCCESS;
+    request->req.state = EC_INT_REQUEST_SUCCESS;
+    kref_put(&request->refcount,ec_master_soe_request_release);
     wake_up(&slave->soe_queue);
 
     fsm->soe_request = NULL;
--- a/master/fsm_slave.h	Wed Jan 05 12:26:33 2011 +0100
+++ b/master/fsm_slave.h	Wed Jan 05 12:46:12 2011 +0100
@@ -56,7 +56,7 @@
     ec_master_sdo_request_t *sdo_request; /**< SDO request to process. */
     ec_master_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. */
+    ec_master_soe_request_t *soe_request; /**< SoE request to process. */
 
     ec_fsm_coe_t fsm_coe; /**< CoE state machine */
     ec_fsm_foe_t fsm_foe; /**< FoE state machine */
--- a/master/master.c	Wed Jan 05 12:26:33 2011 +0100
+++ b/master/master.c	Wed Jan 05 12:46:12 2011 +0100
@@ -2306,7 +2306,7 @@
         uint8_t drive_no, uint16_t idn, uint8_t *data, size_t data_size,
         uint16_t *error_code)
 {
-    ec_master_soe_request_t request;
+    ec_master_soe_request_t* request;
     int retval;
 
     if (drive_no > 7) {
@@ -2314,63 +2314,61 @@
         return -EINVAL;
     }
 
-    INIT_LIST_HEAD(&request.list);
-    ec_soe_request_init(&request.req);
-    ec_soe_request_set_drive_no(&request.req, drive_no);
-    ec_soe_request_set_idn(&request.req, idn);
-
-    if (ec_soe_request_alloc(&request.req, data_size)) {
-        ec_soe_request_clear(&request.req);
+    request = kmalloc(sizeof(*request), GFP_KERNEL);
+    if (!request)
         return -ENOMEM;
-    }
-
-    memcpy(request.req.data, data, data_size);
-    request.req.data_size = data_size;
-    ec_soe_request_write(&request.req);
-
-    if (ec_mutex_lock_interruptible(&master->master_mutex))
+    kref_init(&request->refcount);
+
+    INIT_LIST_HEAD(&request->list);
+    ec_soe_request_init(&request->req);
+    ec_soe_request_set_drive_no(&request->req, drive_no);
+    ec_soe_request_set_idn(&request->req, idn);
+
+    if (ec_soe_request_alloc(&request->req, data_size)) {
+        ec_soe_request_clear(&request->req);
+        kref_put(&request->refcount,ec_master_soe_request_release);
+        return -ENOMEM;
+    }
+
+    memcpy(request->req.data, data, data_size);
+    request->req.data_size = data_size;
+    ec_soe_request_write(&request->req);
+
+    if (ec_mutex_lock_interruptible(&master->master_mutex)) {
+        kref_put(&request->refcount,ec_master_soe_request_release);
         return -EINTR;
-
-    if (!(request.slave = ec_master_find_slave(
+    }
+
+    if (!(request->slave = ec_master_find_slave(
                     master, 0, slave_position))) {
         ec_mutex_unlock(&master->master_mutex);
         EC_MASTER_ERR(master, "Slave %u does not exist!\n",
                 slave_position);
-        ec_soe_request_clear(&request.req);
+        kref_put(&request->refcount,ec_master_soe_request_release);
         return -EINVAL;
     }
 
-    EC_SLAVE_DBG(request.slave, 1, "Scheduling SoE write request.\n");
+    EC_SLAVE_DBG(request->slave, 1, "Scheduled SoE write request %p.\n",request);
 
     // schedule SoE write request.
-    list_add_tail(&request.list, &request.slave->soe_requests);
+    list_add_tail(&request->list, &request->slave->soe_requests);
+    kref_get(&request->refcount);
 
     ec_mutex_unlock(&master->master_mutex);
 
     // wait for processing through FSM
-    if (wait_event_interruptible(request.slave->soe_queue,
-                request.req.state != EC_INT_REQUEST_QUEUED)) {
-        // interrupted by signal
-        ec_mutex_lock(&master->master_mutex);
-        if (request.req.state == EC_INT_REQUEST_QUEUED) {
-            // abort request
-            list_del(&request.list);
-            ec_mutex_unlock(&master->master_mutex);
-            ec_soe_request_clear(&request.req);
-            return -EINTR;
-        }
-        ec_mutex_unlock(&master->master_mutex);
-    }
-
-    // wait until master FSM has finished processing
-    wait_event(request.slave->soe_queue,
-            request.req.state != EC_INT_REQUEST_BUSY);
+    if (wait_event_interruptible(request->slave->soe_queue,
+          ((request->req.state == EC_INT_REQUEST_SUCCESS) || (request->req.state == EC_INT_REQUEST_FAILURE)))) {
+           // interrupted by signal
+           kref_put(&request->refcount,ec_master_soe_request_release);
+           return -EINTR;
+    }
 
     if (error_code) {
-        *error_code = request.req.error_code;
-    }
-    retval = request.req.state == EC_INT_REQUEST_SUCCESS ? 0 : -EIO;
-    ec_soe_request_clear(&request.req);
+        *error_code = request->req.error_code;
+    }
+    retval = request->req.state == EC_INT_REQUEST_SUCCESS ? 0 : -EIO;
+    kref_put(&request->refcount,ec_master_soe_request_release);
 
     return retval;
 }
@@ -2381,78 +2379,76 @@
         uint8_t drive_no, uint16_t idn, uint8_t *target, size_t target_size,
         size_t *result_size, uint16_t *error_code)
 {
-    ec_master_soe_request_t request;
+    ec_master_soe_request_t* request;
 
     if (drive_no > 7) {
         EC_MASTER_ERR(master, "Invalid drive number!\n");
         return -EINVAL;
     }
 
-    INIT_LIST_HEAD(&request.list);
-    ec_soe_request_init(&request.req);
-    ec_soe_request_set_drive_no(&request.req, drive_no);
-    ec_soe_request_set_idn(&request.req, idn);
-    ec_soe_request_read(&request.req);
-
-    if (ec_mutex_lock_interruptible(&master->master_mutex))
+    request = kmalloc(sizeof(*request), GFP_KERNEL);
+    if (!request)
+        return -ENOMEM;
+    kref_init(&request->refcount);
+
+    INIT_LIST_HEAD(&request->list);
+    ec_soe_request_init(&request->req);
+    ec_soe_request_set_drive_no(&request->req, drive_no);
+    ec_soe_request_set_idn(&request->req, idn);
+    ec_soe_request_read(&request->req);
+
+    if (ec_mutex_lock_interruptible(&master->master_mutex)) {
+        kref_put(&request->refcount,ec_master_soe_request_release);
         return -EINTR;
-
-    if (!(request.slave = ec_master_find_slave(master, 0, slave_position))) {
+    }
+
+    if (!(request->slave = ec_master_find_slave(master, 0, slave_position))) {
         ec_mutex_unlock(&master->master_mutex);
-        ec_soe_request_clear(&request.req);
+        kref_put(&request->refcount,ec_master_soe_request_release);
         EC_MASTER_ERR(master, "Slave %u does not exist!\n", slave_position);
         return -EINVAL;
     }
 
     // schedule request.
-    list_add_tail(&request.list, &request.slave->soe_requests);
+    list_add_tail(&request->list, &request->slave->soe_requests);
+    kref_get(&request->refcount);
 
     ec_mutex_unlock(&master->master_mutex);
 
-    EC_SLAVE_DBG(request.slave, 1, "Scheduled SoE read request.\n");
+    EC_SLAVE_DBG(request->slave, 1, "Scheduled SoE read request %p.\n",request);
 
     // wait for processing through FSM
-    if (wait_event_interruptible(request.slave->soe_queue,
-                request.req.state != EC_INT_REQUEST_QUEUED)) {
-        // interrupted by signal
-        ec_mutex_lock(&master->master_mutex);
-        if (request.req.state == EC_INT_REQUEST_QUEUED) {
-            list_del(&request.list);
-            ec_mutex_unlock(&master->master_mutex);
-            ec_soe_request_clear(&request.req);
-            return -EINTR;
-        }
-        // request already processing: interrupt not possible.
-        ec_mutex_unlock(&master->master_mutex);
-    }
-
-    // wait until master FSM has finished processing
-    wait_event(request.slave->soe_queue,
-            request.req.state != EC_INT_REQUEST_BUSY);
+    if (wait_event_interruptible(request->slave->soe_queue,
+          ((request->req.state == EC_INT_REQUEST_SUCCESS) || (request->req.state == EC_INT_REQUEST_FAILURE)))) {
+           // interrupted by signal
+           kref_put(&request->refcount,ec_master_soe_request_release);
+           return -EINTR;
+    }
 
     if (error_code) {
-        *error_code = request.req.error_code;
-    }
-
-    EC_SLAVE_DBG(request.slave, 1, "Read %zd bytes via SoE.\n",
-            request.req.data_size);
-
-    if (request.req.state != EC_INT_REQUEST_SUCCESS) {
+        *error_code = request->req.error_code;
+    }
+
+    EC_SLAVE_DBG(request->slave, 1, "SoE request %p read %zd bytes via SoE.\n",
+            request,request->req.data_size);
+
+    if (request->req.state != EC_INT_REQUEST_SUCCESS) {
         if (result_size) {
             *result_size = 0;
         }
-        ec_soe_request_clear(&request.req);
+        kref_put(&request->refcount,ec_master_soe_request_release);
         return -EIO;
     } else {
-        if (request.req.data_size > target_size) {
+        if (request->req.data_size > target_size) {
             EC_MASTER_ERR(master, "Buffer too small.\n");
-            ec_soe_request_clear(&request.req);
+            kref_put(&request->refcount,ec_master_soe_request_release);
             return -EOVERFLOW;
         }
         if (result_size) {
-            *result_size = request.req.data_size;
-        }
-        memcpy(target, request.req.data, request.req.data_size);
+            *result_size = request->req.data_size;
+        }
+        memcpy(target, request->req.data, request->req.data_size);
+        kref_put(&request->refcount,ec_master_soe_request_release);
         return 0;
     }
 }
--- a/master/slave.c	Wed Jan 05 12:26:33 2011 +0100
+++ b/master/slave.c	Wed Jan 05 12:46:12 2011 +0100
@@ -221,6 +221,7 @@
         EC_SLAVE_WARN(slave, "Discarding SoE request,"
                 " slave about to be deleted.\n");
         request->req.state = EC_INT_REQUEST_FAILURE;
+        kref_put(&request->refcount,ec_master_soe_request_release);
         wake_up(&slave->soe_queue);
     }