SDO wait_event() deadlock fixed: refcount ec_master_sdo_request_t objects with kref

This commit is contained in:
Martin Troxler 2010-12-23 09:48:56 +01:00
parent f78a45f668
commit f2131608f1
6 changed files with 98 additions and 86 deletions

View File

@ -840,78 +840,74 @@ int ec_cdev_ioctl_slave_sdo_upload(
)
{
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);
request = kmalloc(sizeof(*request), GFP_KERNEL);
if (!request)
return -ENOMEM;
kref_init(&request->refcount);
if (down_interruptible(&master->master_sem))
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(
}
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);
kref_put(&request->refcount,ec_master_sdo_request_release);
return -EINTR;
}
// 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 %p.\n",request);
EC_SLAVE_DBG(request.slave, 1, "Finished SDO upload request.\n");
data.abort_code = request->req.abort_code;
data.abort_code = request.req.abort_code;
if (request.req.state != EC_INT_REQUEST_SUCCESS) {
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 @@ int ec_cdev_ioctl_slave_sdo_upload(
retval = -EFAULT;
}
ec_sdo_request_clear(&request.req);
kref_put(&request->refcount,ec_master_sdo_request_release);
return retval;
}
@ -935,7 +931,7 @@ int ec_cdev_ioctl_slave_sdo_download(
)
{
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 @@ int ec_cdev_ioctl_slave_sdo_download(
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);
kref_put(&request->refcount,ec_master_sdo_request_release);
return -EFAULT;
}
request.req.data_size = data.data_size;
ecrt_sdo_request_write(&request.req);
request->req.data_size = data.data_size;
ecrt_sdo_request_write(&request->req);
if (down_interruptible(&master->master_sem))
if (down_interruptible(&master->master_sem)) {
kref_put(&request->refcount,ec_master_sdo_request_release);
return -EINTR;
if (!(request.slave = ec_master_find_slave(
}
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);
kref_put(&request->refcount,ec_master_sdo_request_release);
return -EINTR;
}
// 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 %p.\n",request);
EC_SLAVE_DBG(request.slave, 1, "Finished SDO download request.\n");
data.abort_code = request->req.abort_code;
data.abort_code = request.req.abort_code;
if (request.req.state == EC_INT_REQUEST_SUCCESS) {
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 @@ int ec_cdev_ioctl_slave_sdo_download(
retval = -EFAULT;
}
ec_sdo_request_clear(&request.req);
kref_put(&request->refcount,ec_master_sdo_request_release);
return retval;
}

View File

@ -1227,3 +1227,14 @@ void ec_fsm_master_state_reg_request(
}
/*****************************************************************************/
/** 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);
}

View File

@ -73,6 +73,8 @@ typedef struct {
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 @@ typedef struct {
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;
/*****************************************************************************/

View File

@ -178,9 +178,10 @@ int ec_fsm_slave_action_process_sdo(
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 @@ int ec_fsm_slave_action_process_sdo(
}
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 @@ int ec_fsm_slave_action_process_sdo(
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 @@ void ec_fsm_slave_state_sdo_request(
)
{
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 @@ void ec_fsm_slave_state_sdo_request(
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;

View File

@ -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 @@ struct ec_fsm_slave {
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. */

View File

@ -194,9 +194,10 @@ void ec_slave_clear(ec_slave_t *slave /**< EtherCAT slave */)
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);
}