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 <mike@foundries.io>
This commit is contained in:
Michael Scott 2019-04-05 16:14:50 -07:00 committed by Anas Nashif
commit 42abfc6532

View file

@ -119,7 +119,7 @@ static struct observe_node observe_node_data[CONFIG_LWM2M_ENGINE_MAX_OBSERVER];
struct service_node { struct service_node {
sys_snode_t node; sys_snode_t node;
struct k_work service_work; k_work_handler_t service_work;
u32_t min_call_period; u32_t min_call_period;
u64_t last_timestamp; 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 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 *get_engine_obj(int obj_id);
static struct lwm2m_engine_obj_inst *get_engine_obj_inst(int obj_id, static struct lwm2m_engine_obj_inst *get_engine_obj_inst(int obj_id,
int obj_inst_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 */ /* find an unused service index node */
for (i = 0; i < MAX_PERIODIC_SERVICE; i++) { for (i = 0; i < MAX_PERIODIC_SERVICE; i++) {
if (!service_node_data[i].service_work.handler) { if (!service_node_data[i].service_work) {
break; break;
} }
} }
@ -3754,7 +3752,7 @@ int lwm2m_engine_add_service(k_work_handler_t service, u32_t period_ms)
return -ENOMEM; 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].min_call_period = period_ms;
service_node_data[i].last_timestamp = 0U; 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; return 0;
} }
static void lwm2m_engine_service(struct k_work *work) static int lwm2m_engine_service(void)
{ {
struct observe_node *obs; struct observe_node *obs;
struct service_node *srv; struct service_node *srv;
s64_t timestamp, service_due_timestamp; s64_t timestamp, service_due_timestamp;
s32_t sleep_ms;
int ret;
/* /*
* 1. scan the observer list * 1. scan the observer list
@ -3810,16 +3806,12 @@ static void lwm2m_engine_service(struct k_work *work)
/* service is due */ /* service is due */
if (timestamp > service_due_timestamp) { if (timestamp > service_due_timestamp) {
srv->last_timestamp = k_uptime_get(); 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 */ /* calculate how long to sleep till the next service */
sleep_ms = engine_next_service_timeout_ms(ENGINE_UPDATE_INTERVAL); return 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);
}
} }
int lwm2m_engine_context_close(struct lwm2m_ctx *client_ctx) int lwm2m_engine_context_close(struct lwm2m_ctx *client_ctx)
@ -3905,7 +3897,7 @@ static void socket_receive_loop(void)
while (1) { while (1) {
/* wait for sockets */ /* wait for sockets */
if (sock_nfds < 1) { if (sock_nfds < 1) {
k_sleep(ENGINE_UPDATE_INTERVAL); k_sleep(lwm2m_engine_service());
continue; continue;
} }
@ -3913,7 +3905,7 @@ static void socket_receive_loop(void)
* FIXME: Currently we timeout and restart poll in case fds * FIXME: Currently we timeout and restart poll in case fds
* were modified. * 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); LOG_ERR("Error in poll:%d", errno);
errno = 0; errno = 0;
k_sleep(ENGINE_UPDATE_INTERVAL); 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) static int lwm2m_engine_init(struct device *dev)
{ {
int ret; int ret = 0;
(void)memset(block1_contexts, 0, (void)memset(block1_contexts, 0,
sizeof(struct block_context) * NUM_BLOCK1_CONTEXT); 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"); k_thread_name_set(&engine_thread_data, "lwm2m-sock-recv");
LOG_DBG("LWM2M engine socket receive thread started"); LOG_DBG("LWM2M engine socket receive thread started");
k_delayed_work_init(&periodic_work, lwm2m_engine_service); return ret;
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;
} }
SYS_INIT(lwm2m_engine_init, APPLICATION, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT); SYS_INIT(lwm2m_engine_init, APPLICATION, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT);