# HG changeset patch # User Martin Troxler # Date 1294217435 -3600 # Node ID 5ef6507fc77afe05a2ec3273374058d65b629cd7 # Parent 55854f070c4a1521e9e3403f8c43cfce4a437657 FoE wait_event() deadlock fixed: refcount ec_master_foe_request_t objects with kref diff -r 55854f070c4a -r 5ef6507fc77a master/cdev.c --- a/master/cdev.c Wed Jan 05 08:36:53 2011 +0100 +++ b/master/cdev.c Wed Jan 05 09:50:35 2011 +0100 @@ -3270,74 +3270,70 @@ ) { ec_ioctl_slave_foe_t data; - ec_master_foe_request_t request; + ec_master_foe_request_t* request; int retval; if (copy_from_user(&data, (void __user *) arg, sizeof(data))) { return -EFAULT; } - ec_foe_request_init(&request.req, data.file_name); - ec_foe_request_read(&request.req); - ec_foe_request_alloc(&request.req, 10000); // FIXME - - if (ec_mutex_lock_interruptible(&master->master_mutex)) - return -EINTR; - - if (!(request.slave = ec_master_find_slave( + request = kmalloc(sizeof(*request), GFP_KERNEL); + if (!request) + return -ENOMEM; + kref_init(&request->refcount); + + ec_foe_request_init(&request->req, data.file_name); + ec_foe_request_read(&request->req); + ec_foe_request_alloc(&request->req, 10000); // FIXME + + if (ec_mutex_lock_interruptible(&master->master_mutex)) { + kref_put(&request->refcount,ec_master_foe_request_release); + return -EINTR; + } + if (!(request->slave = ec_master_find_slave( master, 0, data.slave_position))) { ec_mutex_unlock(&master->master_mutex); - ec_foe_request_clear(&request.req); + kref_put(&request->refcount,ec_master_foe_request_release); EC_MASTER_ERR(master, "Slave %u does not exist!\n", data.slave_position); return -EINVAL; } // schedule request. - list_add_tail(&request.list, &request.slave->foe_requests); - - ec_mutex_unlock(&master->master_mutex); - - EC_SLAVE_DBG(request.slave, 1, "Scheduled FoE read request.\n"); + list_add_tail(&request->list, &request->slave->foe_requests); + kref_get(&request->refcount); + + ec_mutex_unlock(&master->master_mutex); + + EC_SLAVE_DBG(request->slave, 1, "Scheduled FoE read request %p.\n",request); // wait for processing through FSM - if (wait_event_interruptible(request.slave->foe_queue, - request.req.state != EC_INT_REQUEST_QUEUED)) { + if (wait_event_interruptible(request->slave->foe_queue, + ((request->req.state == EC_INT_REQUEST_SUCCESS) || (request->req.state == EC_INT_REQUEST_FAILURE)))) { // 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_foe_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->foe_queue, - request.req.state != EC_INT_REQUEST_BUSY); - - data.result = request.req.result; - data.error_code = request.req.error_code; - - EC_SLAVE_DBG(request.slave, 1, "Read %zd bytes via FoE" - " (result = 0x%x).\n", request.req.data_size, request.req.result); - - if (request.req.state != EC_INT_REQUEST_SUCCESS) { + kref_put(&request->refcount,ec_master_foe_request_release); + return -EINTR; + } + + data.result = request->req.result; + data.error_code = request->req.error_code; + + EC_SLAVE_DBG(request->slave, 1, "Read %zd bytes via FoE" + " (result = 0x%x).\n", request->req.data_size, request->req.result); + + if (request->req.state != EC_INT_REQUEST_SUCCESS) { data.data_size = 0; retval = -EIO; } else { - if (request.req.data_size > data.buffer_size) { + if (request->req.data_size > data.buffer_size) { EC_MASTER_ERR(master, "Buffer too small.\n"); - ec_foe_request_clear(&request.req); + kref_put(&request->refcount,ec_master_foe_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.buffer, - request.req.buffer, data.data_size)) { - ec_foe_request_clear(&request.req); + request->req.buffer, data.data_size)) { + kref_put(&request->refcount,ec_master_foe_request_release); return -EFAULT; } retval = 0; @@ -3347,9 +3343,8 @@ retval = -EFAULT; } - EC_SLAVE_DBG(request.slave, 1, "Finished FoE read request.\n"); - - ec_foe_request_clear(&request.req); + EC_SLAVE_DBG(request->slave, 1, "Finished FoE read request %p.\n",request); + kref_put(&request->refcount,ec_master_foe_request_release); return retval; } @@ -3364,79 +3359,73 @@ ) { ec_ioctl_slave_foe_t data; - ec_master_foe_request_t request; + ec_master_foe_request_t* request; int retval; if (copy_from_user(&data, (void __user *) arg, sizeof(data))) { return -EFAULT; } - INIT_LIST_HEAD(&request.list); - - ec_foe_request_init(&request.req, data.file_name); - - if (ec_foe_request_alloc(&request.req, data.buffer_size)) { - ec_foe_request_clear(&request.req); + request = kmalloc(sizeof(*request), GFP_KERNEL); + if (!request) return -ENOMEM; - } - if (copy_from_user(request.req.buffer, + kref_init(&request->refcount); + + INIT_LIST_HEAD(&request->list); + + ec_foe_request_init(&request->req, data.file_name); + + if (ec_foe_request_alloc(&request->req, data.buffer_size)) { + kref_put(&request->refcount,ec_master_foe_request_release); + return -ENOMEM; + } + if (copy_from_user(request->req.buffer, (void __user *) data.buffer, data.buffer_size)) { - ec_foe_request_clear(&request.req); - return -EFAULT; - } - request.req.data_size = data.buffer_size; - ec_foe_request_write(&request.req); - - if (ec_mutex_lock_interruptible(&master->master_mutex)) - return -EINTR; - - if (!(request.slave = ec_master_find_slave( + kref_put(&request->refcount,ec_master_foe_request_release); + return -EFAULT; + } + request->req.data_size = data.buffer_size; + ec_foe_request_write(&request->req); + + if (ec_mutex_lock_interruptible(&master->master_mutex)) + return -EINTR; + + if (!(request->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); - ec_foe_request_clear(&request.req); - return -EINVAL; - } - - EC_SLAVE_DBG(request.slave, 1, "Scheduling FoE write request.\n"); + kref_put(&request->refcount,ec_master_foe_request_release); + return -EINVAL; + } + + EC_SLAVE_DBG(request->slave, 1, "Scheduling FoE write request %p.\n",request); // schedule FoE write request. - list_add_tail(&request.list, &request.slave->foe_requests); + list_add_tail(&request->list, &request->slave->foe_requests); + kref_get(&request->refcount); ec_mutex_unlock(&master->master_mutex); // wait for processing through FSM - if (wait_event_interruptible(request.slave->foe_queue, - request.req.state != EC_INT_REQUEST_QUEUED)) { + if (wait_event_interruptible(request->slave->foe_queue, + ((request->req.state == EC_INT_REQUEST_SUCCESS) || (request->req.state == EC_INT_REQUEST_FAILURE)))) { // 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_foe_request_clear(&request.req); - return -EINTR; - } - ec_mutex_unlock(&master->master_mutex); - } - - // wait until master FSM has finished processing - wait_event(request.slave->foe_queue, - request.req.state != EC_INT_REQUEST_BUSY); - - data.result = request.req.result; - data.error_code = request.req.error_code; - - retval = request.req.state == EC_INT_REQUEST_SUCCESS ? 0 : -EIO; + kref_put(&request->refcount,ec_master_foe_request_release); + return -EINTR; + } + + data.result = request->req.result; + data.error_code = request->req.error_code; + + retval = request->req.state == EC_INT_REQUEST_SUCCESS ? 0 : -EIO; if (__copy_to_user((void __user *) arg, &data, sizeof(data))) { retval = -EFAULT; } - ec_foe_request_clear(&request.req); - - EC_SLAVE_DBG(request.slave, 1, "Finished FoE write request.\n"); + EC_SLAVE_DBG(request->slave, 1, "Finished FoE write request %p.\n",request); + kref_put(&request->refcount,ec_master_foe_request_release); return retval; } diff -r 55854f070c4a -r 5ef6507fc77a master/fsm_master.c --- a/master/fsm_master.c Wed Jan 05 08:36:53 2011 +0100 +++ b/master/fsm_master.c Wed Jan 05 09:50:35 2011 +0100 @@ -1228,13 +1228,26 @@ /*****************************************************************************/ -/** called by kref_put if the request's refcount becomes zero. +/** called by kref_put if the SDO 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_SLAVE_DBG(request->slave, 1, "Releasing SDO request %p.\n",request); ec_sdo_request_clear(&request->req); kfree(request); } + +/*****************************************************************************/ + +/** called by kref_put if the FoE request's refcount becomes zero. + * + */ +void ec_master_foe_request_release(struct kref *ref) +{ + ec_master_foe_request_t *request = container_of(ref, ec_master_foe_request_t, refcount); + EC_SLAVE_DBG(request->slave, 1, "Releasing FoE request %p.\n",request); + ec_foe_request_clear(&request->req); + kfree(request); +} diff -r 55854f070c4a -r 5ef6507fc77a master/fsm_master.h --- a/master/fsm_master.h Wed Jan 05 08:36:53 2011 +0100 +++ b/master/fsm_master.h Wed Jan 05 09:50:35 2011 +0100 @@ -73,7 +73,6 @@ ec_internal_request_state_t state; /**< State of the request. */ } ec_reg_request_t; -void ec_master_sdo_request_release(struct kref *); /*****************************************************************************/ @@ -86,6 +85,8 @@ struct kref refcount; } ec_master_sdo_request_t; +void ec_master_sdo_request_release(struct kref *); + /*****************************************************************************/ /** FoE request. @@ -94,8 +95,11 @@ struct list_head list; /**< List head. */ ec_slave_t *slave; /**< EtherCAT slave. */ ec_foe_request_t req; /**< FoE request. */ + struct kref refcount; } ec_master_foe_request_t; +void ec_master_foe_request_release(struct kref *); + /*****************************************************************************/ /** SoE request. diff -r 55854f070c4a -r 5ef6507fc77a master/fsm_slave.c --- a/master/fsm_slave.c Wed Jan 05 08:36:53 2011 +0100 +++ b/master/fsm_slave.c Wed Jan 05 09:50:35 2011 +0100 @@ -267,9 +267,10 @@ // search the first request to be processed list_for_each_entry_safe(request, next, &slave->foe_requests, list) { if (slave->current_state & EC_SLAVE_STATE_ACK_ERR) { - EC_SLAVE_WARN(slave, "Aborting FOE request," - " slave has error flag set.\n"); + EC_SLAVE_WARN(slave, "Aborting FOE request %p," + " slave has error flag set.\n",request); request->req.state = EC_INT_REQUEST_FAILURE; + kref_put(&request->refcount,ec_master_foe_request_release); wake_up(&slave->foe_queue); fsm->sdo_request = NULL; fsm->state = ec_fsm_slave_state_idle; @@ -278,9 +279,9 @@ list_del_init(&request->list); // dequeue request->req.state = EC_INT_REQUEST_BUSY; - EC_SLAVE_DBG(slave, 1, "Processing FoE request.\n"); - - fsm->foe_request = &request->req; + EC_SLAVE_DBG(slave, 1, "Processing FoE request %p.\n",request); + + fsm->foe_request = request; fsm->state = ec_fsm_slave_state_foe_request; ec_fsm_foe_transfer(&fsm->fsm_foe, slave, &request->req); ec_fsm_foe_exec(&fsm->fsm_foe); @@ -299,7 +300,7 @@ ) { ec_slave_t *slave = fsm->slave; - ec_foe_request_t *request = fsm->foe_request; + ec_master_foe_request_t *request = fsm->foe_request; if (ec_fsm_foe_exec(&fsm->fsm_foe)) { @@ -308,8 +309,9 @@ } if (!ec_fsm_foe_success(&fsm->fsm_foe)) { - EC_SLAVE_ERR(slave, "Failed to handle FoE request.\n"); - request->state = EC_INT_REQUEST_FAILURE; + EC_SLAVE_ERR(slave, "Failed to handle FoE request %p.\n",request); + request->req.state = EC_INT_REQUEST_FAILURE; + kref_put(&request->refcount,ec_master_foe_request_release); wake_up(&slave->foe_queue); fsm->foe_request = NULL; fsm->state = ec_fsm_slave_state_idle; @@ -317,10 +319,11 @@ } // finished transferring FoE - EC_SLAVE_DBG(slave, 1, "Successfully transferred %zu bytes of FoE" - " data.\n", request->data_size); - - request->state = EC_INT_REQUEST_SUCCESS; + EC_SLAVE_DBG(slave, 1, "FoE request %p successfully transferred %zu bytes.\n", + request,request->req.data_size); + + request->req.state = EC_INT_REQUEST_SUCCESS; + kref_put(&request->refcount,ec_master_foe_request_release); wake_up(&slave->foe_queue); fsm->foe_request = NULL; diff -r 55854f070c4a -r 5ef6507fc77a master/fsm_slave.h --- a/master/fsm_slave.h Wed Jan 05 08:36:53 2011 +0100 +++ b/master/fsm_slave.h Wed Jan 05 09:50:35 2011 +0100 @@ -54,7 +54,7 @@ void (*state)(ec_fsm_slave_t *); /**< master state function */ ec_master_sdo_request_t *sdo_request; /**< SDO request to process. */ - ec_foe_request_t *foe_request; /**< FoE 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. */ diff -r 55854f070c4a -r 5ef6507fc77a master/slave.c --- a/master/slave.c Wed Jan 05 08:36:53 2011 +0100 +++ b/master/slave.c Wed Jan 05 09:50:35 2011 +0100 @@ -209,6 +209,7 @@ EC_SLAVE_WARN(slave, "Discarding FoE request," " slave about to be deleted.\n"); request->req.state = EC_INT_REQUEST_FAILURE; + kref_put(&request->refcount,ec_master_foe_request_release); wake_up(&slave->foe_queue); }