From 42abfc6532d0e79aa31c3ecad9c37f76b66546cd Mon Sep 17 00:00:00 2001 From: Michael Scott Date: Fri, 5 Apr 2019 16:14:50 -0700 Subject: [PATCH] net: lwm2m: dont use system workqueue for services "It's a Trap!" -- Admiral Ackbar When moving to the BSD-socket APIs, the original thread running LwM2M periodic services such as observes and lifetime updates, was replaced with a re-occuring workqueue job. To save the overhead of creating a new thread, I used the system workqueue for these jobs. This was a mistake. If these jobs hit a semaphore or wait for some reason, it cannot be prempted due to the priority of the system work queue. Let's instead add this service handling to the thread that we already use for polling sockets. This also removes a configuration issue where the system workqueue stack size needed to be increased. This can now be adjusted via the LWM2M_ENGINE_STACK_SIZE knob. Directly fixes semaphore usage in the socket-based DNS code. This was introduced as a bugfix for non-responsive DNS server hanging the Zephyr device forever. However, this probably fixes randomly seeming hangs on the device. Signed-off-by: Michael Scott --- subsys/net/lib/lwm2m/lwm2m_engine.c | 36 ++++++++--------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/subsys/net/lib/lwm2m/lwm2m_engine.c b/subsys/net/lib/lwm2m/lwm2m_engine.c index c38fd787ee8..ef945ef52a5 100644 --- a/subsys/net/lib/lwm2m/lwm2m_engine.c +++ b/subsys/net/lib/lwm2m/lwm2m_engine.c @@ -119,7 +119,7 @@ static struct observe_node observe_node_data[CONFIG_LWM2M_ENGINE_MAX_OBSERVER]; struct service_node { sys_snode_t node; - struct k_work service_work; + k_work_handler_t service_work; u32_t min_call_period; u64_t last_timestamp; }; @@ -166,8 +166,6 @@ static const u8_t LWM2M_ATTR_LEN[] = { 4, 4, 2, 2, 2 }; static struct lwm2m_attr write_attr_pool[CONFIG_LWM2M_NUM_ATTR]; -static struct k_delayed_work periodic_work; - static struct lwm2m_engine_obj *get_engine_obj(int obj_id); static struct lwm2m_engine_obj_inst *get_engine_obj_inst(int obj_id, int obj_inst_id); @@ -3745,7 +3743,7 @@ int lwm2m_engine_add_service(k_work_handler_t service, u32_t period_ms) /* find an unused service index node */ for (i = 0; i < MAX_PERIODIC_SERVICE; i++) { - if (!service_node_data[i].service_work.handler) { + if (!service_node_data[i].service_work) { break; } } @@ -3754,7 +3752,7 @@ int lwm2m_engine_add_service(k_work_handler_t service, u32_t period_ms) return -ENOMEM; } - k_work_init(&service_node_data[i].service_work, service); + service_node_data[i].service_work = service; service_node_data[i].min_call_period = period_ms; service_node_data[i].last_timestamp = 0U; @@ -3764,13 +3762,11 @@ int lwm2m_engine_add_service(k_work_handler_t service, u32_t period_ms) return 0; } -static void lwm2m_engine_service(struct k_work *work) +static int lwm2m_engine_service(void) { struct observe_node *obs; struct service_node *srv; s64_t timestamp, service_due_timestamp; - s32_t sleep_ms; - int ret; /* * 1. scan the observer list @@ -3810,16 +3806,12 @@ static void lwm2m_engine_service(struct k_work *work) /* service is due */ if (timestamp > service_due_timestamp) { srv->last_timestamp = k_uptime_get(); - k_work_submit(&srv->service_work); + srv->service_work(NULL); } } /* calculate how long to sleep till the next service */ - sleep_ms = engine_next_service_timeout_ms(ENGINE_UPDATE_INTERVAL); - ret = k_delayed_work_submit(&periodic_work, sleep_ms); - if (ret < 0) { - LOG_ERR("Work submit error:%d", ret); - } + return engine_next_service_timeout_ms(ENGINE_UPDATE_INTERVAL); } int lwm2m_engine_context_close(struct lwm2m_ctx *client_ctx) @@ -3905,7 +3897,7 @@ static void socket_receive_loop(void) while (1) { /* wait for sockets */ if (sock_nfds < 1) { - k_sleep(ENGINE_UPDATE_INTERVAL); + k_sleep(lwm2m_engine_service()); continue; } @@ -3913,7 +3905,7 @@ static void socket_receive_loop(void) * FIXME: Currently we timeout and restart poll in case fds * were modified. */ - if (poll(sock_fds, sock_nfds, ENGINE_UPDATE_INTERVAL) < 0) { + if (poll(sock_fds, sock_nfds, lwm2m_engine_service()) < 0) { LOG_ERR("Error in poll:%d", errno); errno = 0; k_sleep(ENGINE_UPDATE_INTERVAL); @@ -4187,7 +4179,7 @@ int lwm2m_engine_start(struct lwm2m_ctx *client_ctx) static int lwm2m_engine_init(struct device *dev) { - int ret; + int ret = 0; (void)memset(block1_contexts, 0, sizeof(struct block_context) * NUM_BLOCK1_CONTEXT); @@ -4204,15 +4196,7 @@ static int lwm2m_engine_init(struct device *dev) k_thread_name_set(&engine_thread_data, "lwm2m-sock-recv"); LOG_DBG("LWM2M engine socket receive thread started"); - k_delayed_work_init(&periodic_work, lwm2m_engine_service); - ret = k_delayed_work_submit(&periodic_work, K_MSEC(2000)); - if (ret < 0) { - LOG_ERR("Error starting periodic work: %d", ret); - return ret; - } - - LOG_DBG("LWM2M engine periodic work started"); - return 0; + return ret; } SYS_INIT(lwm2m_engine_init, APPLICATION, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT);