From b3bfc21724694415a3c2cb59a73fcc96f89c6275 Mon Sep 17 00:00:00 2001 From: Martin Troxler Date: Wed, 5 Jan 2011 12:46:12 +0100 Subject: [PATCH] SoE request wait_event() deadlock fixed: use wait_event_interruptible and refcount ec_master_soe_request_t objects with kref --- master/fsm_master.c | 13 +++++ master/fsm_master.h | 3 + master/fsm_slave.c | 26 +++++---- master/fsm_slave.h | 2 +- master/master.c | 136 +++++++++++++++++++++----------------------- master/slave.c | 1 + 6 files changed, 99 insertions(+), 82 deletions(-) diff --git a/master/fsm_master.c b/master/fsm_master.c index af8cdbfa..98245f4f 100644 --- a/master/fsm_master.c +++ b/master/fsm_master.c @@ -1284,3 +1284,16 @@ void ec_master_foe_request_release(struct kref *ref) 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 --git a/master/fsm_master.h b/master/fsm_master.h index e9e1277e..cdc9f4cc 100644 --- a/master/fsm_master.h +++ b/master/fsm_master.h @@ -113,8 +113,11 @@ typedef struct { 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 --git a/master/fsm_slave.c b/master/fsm_slave.c index aed7c624..e1700c73 100644 --- a/master/fsm_slave.c +++ b/master/fsm_slave.c @@ -341,16 +341,17 @@ int ec_fsm_slave_action_process_soe( ) { 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_for_each_entry_safe(request, next, &slave->soe_requests, list) { - list_del_init(&req->list); // dequeue + 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 @@ int ec_fsm_slave_action_process_soe( 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 @@ void ec_fsm_slave_state_soe_request( ) { 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 @@ void ec_fsm_slave_state_soe_request( 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 @@ void ec_fsm_slave_state_soe_request( 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 --git a/master/fsm_slave.h b/master/fsm_slave.h index 2d6b246a..d95fc2ac 100644 --- a/master/fsm_slave.h +++ b/master/fsm_slave.h @@ -56,7 +56,7 @@ struct ec_fsm_slave { 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 --git a/master/master.c b/master/master.c index e7a56b04..577638c6 100644 --- a/master/master.c +++ b/master/master.c @@ -2306,7 +2306,7 @@ int ecrt_master_write_idn(ec_master_t *master, uint16_t slave_position, 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 @@ int ecrt_master_write_idn(ec_master_t *master, uint16_t slave_position, 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); + request = kmalloc(sizeof(*request), GFP_KERNEL); + if (!request) + return -ENOMEM; + kref_init(&request->refcount); - if (ec_soe_request_alloc(&request.req, data_size)) { - ec_soe_request_clear(&request.req); + 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); + 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)) + 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); + 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; } - // wait until master FSM has finished processing - wait_event(request.slave->soe_queue, - request.req.state != EC_INT_REQUEST_BUSY); - if (error_code) { - *error_code = request.req.error_code; + *error_code = request->req.error_code; } - retval = request.req.state == EC_INT_REQUEST_SUCCESS ? 0 : -EIO; - ec_soe_request_clear(&request.req); + 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 @@ int ecrt_master_read_idn(ec_master_t *master, uint16_t slave_position, 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); + request = kmalloc(sizeof(*request), GFP_KERNEL); + if (!request) + return -ENOMEM; + kref_init(&request->refcount); - if (ec_mutex_lock_interruptible(&master->master_mutex)) + 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); + 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; } - // wait until master FSM has finished processing - wait_event(request.slave->soe_queue, - request.req.state != EC_INT_REQUEST_BUSY); - if (error_code) { - *error_code = request.req.error_code; + *error_code = request->req.error_code; } - EC_SLAVE_DBG(request.slave, 1, "Read %zd bytes via SoE.\n", - request.req.data_size); + 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 (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; + *result_size = request->req.data_size; } - memcpy(target, request.req.data, 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 --git a/master/slave.c b/master/slave.c index 6190c1ca..4af0803a 100644 --- a/master/slave.c +++ b/master/slave.c @@ -221,6 +221,7 @@ void ec_slave_clear(ec_slave_t *slave /**< EtherCAT slave */) 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); }