From 27cf5623b190edcc9c54676029429bb09a9987b7 Mon Sep 17 00:00:00 2001 From: "Wesley W. Terpstra" <w.terpstra@gsi.de> Date: Mon, 5 Mar 2012 18:02:53 +0000 Subject: [PATCH] Change the logic of who calls callbacks: eb_device_close kills outstanding callbacks, not eb_socket_close. ... to do this, response tracked cycles remember their device. ... and on device close it filters the entire list of awaiting responses. The reasoning behind this change: the user probably has handles to the device in the callback if he closes the device, the callbacks might run later with a bad pointer instead => kill those callbacks as part of closing the device. --- api/etherbone.h | 8 ++- api/format/master.c | 3 + api/glue/cycle.c | 9 ++- api/glue/device.c | 49 ++++++++++++++++- api/glue/readwrite.c | 3 +- api/glue/socket.c | 128 +++++++++++++++++++++++++++++-------------- api/glue/socket.h | 5 +- 7 files changed, 156 insertions(+), 49 deletions(-) diff --git a/api/etherbone.h b/api/etherbone.h index 1f8728b..fd5b719 100644 --- a/api/etherbone.h +++ b/api/etherbone.h @@ -322,10 +322,11 @@ EB_PUBLIC eb_width_t eb_device_width(eb_device_t device); /* Close a remote Etherbone device. + * Any inflight or ready-to-send cycles will receive EB_TIMEOUT. * * Return codes: - * OK - associated memory has been freed - * BUSY - there are outstanding wishbone cycles on this device + * OK - associated memory has been freed + * BUSY - there are unclosed wishbone cycles on this device */ EB_PUBLIC eb_status_t eb_device_close(eb_device_t device); @@ -348,8 +349,9 @@ eb_status_t eb_device_flush(eb_device_t device); * Read/write operations are executed in the order they are queued. * Until the cycle is closed and flushed, the operations are not sent. * If there is insufficient memory to begin a cycle, EB_NULL is returned. + * If the device is being closed, EB_NULL is also returned. * - * Your callback is called from either eb_socket_poll or eb_device_flush. + * Your callback may be called from: eb_socket_poll/eb_device_flush/eb_device_close. * It receives these arguments: (user_data, operations, status) * * If status != OK, the cycle was never sent to the remote bus. diff --git a/api/format/master.c b/api/format/master.c index 88e6ad1..59a3582 100644 --- a/api/format/master.c +++ b/api/format/master.c @@ -136,6 +136,9 @@ eb_status_t eb_device_flush(eb_device_t devicep) { cycle = EB_CYCLE(cyclep); nextp = cycle->un_link.next; + /* Record the device which answers */ + cycle->un_link.device = devicep; + /* Deal with OOM cases */ if (cycle->un_ops.dead == cyclep) { if (cycle->callback) diff --git a/api/glue/cycle.c b/api/glue/cycle.c index d16dc82..2859435 100644 --- a/api/glue/cycle.c +++ b/api/glue/cycle.c @@ -56,7 +56,14 @@ eb_cycle_t eb_cycle_open(eb_device_t devicep, eb_user_data_t user, eb_callback_t cycle->un_link.device = devicep; device = EB_DEVICE(devicep); - ++device->unready; + + /* Is the device closing? */ + if (device->un_link.passive == devicep) { + eb_free_cycle(cyclep); + return EB_OOM; + } else { + ++device->unready; + } return cyclep; } diff --git a/api/glue/device.c b/api/glue/device.c index e2c07e5..1416e65 100644 --- a/api/glue/device.c +++ b/api/glue/device.c @@ -27,6 +27,7 @@ #define ETHERBONE_IMPL +#include "socket.h" #include "device.h" #include "socket.h" #include "widths.h" @@ -206,20 +207,62 @@ eb_status_t eb_device_close(eb_device_t devicep) { struct eb_socket* socket; struct eb_device* device; struct eb_transport* transport; + struct eb_cycle* cycle; struct eb_link* link; struct eb_device* idev; eb_device_t* ptr, i; eb_link_t linkp; + eb_socket_t socketp; + eb_cycle_t cyclep, nextp, prevp, firstp; device = EB_DEVICE(devicep); + socketp = device->socket; - if ((device->un_link.ready != EB_NULL && device->un_link.passive != devicep) || device->unready != 0) + if (device->unready != 0) return EB_BUSY; + + if (device->un_link.passive != devicep) { + /* We will clear these cycles */ + firstp = device->un_link.ready; + + /* Mark the device as inappropriate for cycle_open */ + device->un_link.passive = devicep; + + /* Kill any inflight cycles that belong to us */ + eb_socket_kill_inflight(socketp, devicep); + device = EB_DEVICE(devicep); /* Refresh */ + } else { + firstp = EB_NULL; + } + + /* First, reverse the list so the answer come back 'in order' */ + prevp = EB_NULL; + for (cyclep = firstp; cyclep != EB_NULL; cyclep = nextp) { + cycle = EB_CYCLE(cyclep); + nextp = cycle->un_link.next; + cycle->un_link.next = prevp; + prevp = cyclep; + } + /* Kill the cycles */ + for (cyclep = prevp; cyclep != EB_NULL; cyclep = nextp) { + cycle = EB_CYCLE(cyclep); + nextp = cycle->un_link.next; + + if (cycle->callback) + (*cycle->callback)(cycle->user_data, cycle->un_ops.first, EB_TIMEOUT); + + /* Free it */ + eb_cycle_destroy(cyclep); + eb_free_cycle(cyclep); + } + + /* Refresh pointers */ + device = EB_DEVICE(devicep); + socket = EB_SOCKET(socketp); transport = EB_TRANSPORT(device->transport); - socket = EB_SOCKET(device->socket); - /* Find it in the socket's list */ + /* Now find and remove the device from the socket's list */ for (ptr = &socket->first_device; (i = *ptr) != EB_NULL; ptr = &idev->next) { if (i == devicep) break; idev = EB_DEVICE(i); diff --git a/api/glue/readwrite.c b/api/glue/readwrite.c index c7c331c..c04ff12 100644 --- a/api/glue/readwrite.c +++ b/api/glue/readwrite.c @@ -50,7 +50,8 @@ void eb_socket_write_config(eb_socket_t socketp, eb_width_t widths, eb_address_t responsepp = &socket->first_response; while (1) { if ((responsep = *responsepp) == EB_NULL) { - *responsepp = responsep = eb_socket_flip_last(socket); + *responsepp = responsep = eb_response_flip(socket->last_response); + socket->last_response = EB_NULL; if (responsep == EB_NULL) return; /* No matching response record */ } response = EB_RESPONSE(responsep); diff --git a/api/glue/socket.c b/api/glue/socket.c index 82d94a5..5a4a130 100644 --- a/api/glue/socket.c +++ b/api/glue/socket.c @@ -153,62 +153,38 @@ eb_status_t eb_socket_open(uint16_t abi_code, const char* port, eb_width_t suppo return status; } -eb_response_t eb_socket_flip_last(struct eb_socket* socket) { - struct eb_response* i; - eb_response_t ip, prev, next; - - prev = EB_NULL; - for (ip = socket->last_response; ip != EB_NULL; ip = next) { - i = EB_RESPONSE(ip); - next = i->next; - i->next = prev; - prev = ip; +eb_response_t eb_response_flip(eb_response_t firstp) { + struct eb_response* response; + eb_response_t responsep, prev_responsep, next_responsep; + + prev_responsep = EB_NULL; + for (responsep = firstp; responsep != EB_NULL; responsep = next_responsep) { + response = EB_RESPONSE(responsep); + next_responsep = response->next; + response->next = prev_responsep; + prev_responsep = responsep; } - socket->last_response = EB_NULL; - return prev; + return prev_responsep; } eb_status_t eb_socket_close(eb_socket_t socketp) { struct eb_socket* socket; struct eb_socket_aux* aux; struct eb_handler_address* handler; - struct eb_response* response; - struct eb_cycle* cycle; struct eb_transport* transport; eb_transport_t transportp, next_transportp; - eb_response_t tmp; eb_socket_aux_t auxp; eb_handler_address_t i, next; socket = EB_SOCKET(socketp); + /* If there are open devices, we can't close */ if (socket->first_device != EB_NULL) return EB_BUSY; - /* Cancel all callbacks */ - while ((tmp = socket->first_response) != EB_NULL) { - response = EB_RESPONSE(tmp); - - if (response->next == EB_NULL) { - socket->first_response = eb_socket_flip_last(socket); - } else { - socket->first_response = response->next; - } - - /* Report the cycle callback */ - cycle = EB_CYCLE(response->cycle); - if (cycle->callback) - (*cycle->callback)(cycle->user_data, cycle->un_ops.first, EB_TIMEOUT); /* invalidate: socket response cycle */ - - socket = EB_SOCKET(socketp); - response = EB_RESPONSE(tmp); - - /* Free associated memory */ - eb_cycle_destroy(response->cycle); - eb_free_cycle(response->cycle); - eb_free_response(tmp); - } + /* All responses must be closed if devices are closed */ + /* assert (socket->first_response == EB_NULL); */ /* Flush handlers */ for (i = socket->first_handler; i != EB_NULL; i = next) { @@ -234,6 +210,76 @@ eb_status_t eb_socket_close(eb_socket_t socketp) { return EB_OK; } +static void eb_socket_filter_inflight(eb_response_t* goodp, eb_response_t* badp, eb_device_t devicep, eb_response_t firstp) { + struct eb_response* response; + struct eb_cycle* cycle; + eb_response_t good, bad; + eb_response_t responsep, next_responsep; + eb_cycle_t cyclep; + + good = *goodp; + bad = *badp; + + /* Partiton the list */ + for (responsep = firstp; responsep != EB_NULL; responsep = next_responsep) { + response = EB_RESPONSE(responsep); + next_responsep = response->next; + + cyclep = response->cycle; + cycle = EB_CYCLE(cyclep); + + if (cycle->un_link.device == devicep) { + response->next = bad; + bad = responsep; + } else { + response->next = good; + good = responsep; + } + } + + *goodp = good; + *badp = bad; +} + +void eb_socket_kill_inflight(eb_socket_t socketp, eb_device_t devicep) { + struct eb_socket* socket; + struct eb_response* response; + struct eb_cycle* cycle; + eb_response_t responsep, next_responsep; + eb_response_t good, bad; + eb_cycle_t cyclep; + + /* Split the list into good responses we keep, and bad responses we kill */ + good = EB_NULL; + bad = EB_NULL; + socket = EB_SOCKET(socketp); + eb_socket_filter_inflight(&good, &bad, devicep, socket->last_response); + eb_socket_filter_inflight(&good, &bad, devicep, eb_response_flip(socket->first_response)); + socket->first_response = good; + socket->last_response = EB_NULL; + + /* Now kill all the bad responses */ + for (responsep = bad; responsep != EB_NULL; responsep = next_responsep) { + response = EB_RESPONSE(responsep); + next_responsep = response->next; + + cyclep = response->cycle; + cycle = EB_CYCLE(response->cycle); + + /* Mark the response for clean-up */ + response->cycle = EB_NULL; + + /* Run the callback */ + if (cycle->callback) + (*cycle->callback)(cycle->user_data, cycle->un_ops.first, EB_TIMEOUT); + + /* Free it all */ + eb_cycle_destroy(cyclep); + eb_free_cycle(cyclep); + eb_free_response(responsep); + } +} + void eb_socket_descriptor(eb_socket_t socketp, eb_user_data_t user, eb_descriptor_callback_t cb) { struct eb_socket* socket; struct eb_socket_aux* aux; @@ -298,8 +344,10 @@ uint32_t eb_socket_timeout(eb_socket_t socketp) { aux = EB_SOCKET_AUX(socket->aux); /* Find the first timeout */ - if (socket->first_response == EB_NULL) - socket->first_response = eb_socket_flip_last(socket); + if (socket->first_response == EB_NULL) { + socket->first_response = eb_response_flip(socket->last_response); + socket->last_response = EB_NULL; + } /* Determine how long until deadline expires */ if (socket->first_response != EB_NULL) { diff --git a/api/glue/socket.h b/api/glue/socket.h index b4258cf..f9945b4 100644 --- a/api/glue/socket.h +++ b/api/glue/socket.h @@ -71,7 +71,10 @@ struct eb_socket { }; /* Invert last_response, suitable for attaching to the end of first_response */ -EB_PRIVATE eb_response_t eb_socket_flip_last(struct eb_socket* socket); +EB_PRIVATE eb_response_t eb_response_flip(eb_response_t firstp); + +/* Kill all responses inflight for this device */ +EB_PRIVATE void eb_socket_kill_inflight(eb_socket_t socketp, eb_device_t devicep); /* Process inbound read/write requests */ EB_PRIVATE eb_data_t eb_socket_read (eb_socket_t socket, eb_width_t width, eb_address_t addr, uint64_t* error); -- GitLab