Register read/write wait_event() deadlock fixed: use wait_event_interruptible and refcount ec_reg_request_t objects with kref
--- 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;
+
}
/*****************************************************************************/
--- 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.
*
*/
--- 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 *);
/*****************************************************************************/
--- 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);
}