# HG changeset patch # User Martin Troxler # Date 1294227972 -3600 # Node ID 57c61855791265d0657b2abaefce2ea0259e94d7 # Parent 7a025a9e192de00b10536a70217463c970173c26 SoE request wait_event() deadlock fixed: use wait_event_interruptible and refcount ec_master_soe_request_t objects with kref diff -r 7a025a9e192d -r 57c618557912 master/fsm_master.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); +} diff -r 7a025a9e192d -r 57c618557912 master/fsm_master.h --- 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 */ diff -r 7a025a9e192d -r 57c618557912 master/fsm_slave.c --- 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; diff -r 7a025a9e192d -r 57c618557912 master/fsm_slave.h --- 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 */ diff -r 7a025a9e192d -r 57c618557912 master/master.c --- 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; } } diff -r 7a025a9e192d -r 57c618557912 master/slave.c --- 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); }