From 7d3d2b62e89ed5eac1570e7a309eabe1a8dffbad Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Mon, 16 Dec 2019 05:39:53 -0700 Subject: [PATCH] lib/thread: account for busy work for unregistered timers When a timed poller unregisters itself during execution, we were continuing the loop without updating the timer_rc. This would result in spdk_thread_poll() indicating that the poll execution was idle rather than busy. Note that the DEBUG print would have still been OK where it was, since the poller variable itself was valid, even though it had been freed. But it looked a bit awkward there, so I moved it right after we capture timer_rc. Signed-off-by: Jim Harris Change-Id: I0673261ecea0e49db97b008a83b60a35f995f83d Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/478120 Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: Paul Luse Reviewed-by: Tomasz Zawadzki Reviewed-by: Ben Walker --- lib/thread/thread.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/thread/thread.c b/lib/thread/thread.c index b22b9c309..a7ae97255 100644 --- a/lib/thread/thread.c +++ b/lib/thread/thread.c @@ -521,25 +521,23 @@ spdk_thread_poll(struct spdk_thread *thread, uint32_t max_msgs, uint64_t now) poller->state = SPDK_POLLER_STATE_RUNNING; timer_rc = poller->fn(poller->arg); - if (poller->state == SPDK_POLLER_STATE_UNREGISTERED) { - TAILQ_REMOVE(&thread->timer_pollers, poller, tailq); - free(poller); - continue; - } - - poller->state = SPDK_POLLER_STATE_WAITING; - TAILQ_REMOVE(&thread->timer_pollers, poller, tailq); - _spdk_poller_insert_timer(thread, poller, now); - #ifdef DEBUG if (timer_rc == -1) { SPDK_DEBUGLOG(SPDK_LOG_THREAD, "Timed poller %p returned -1\n", poller); } #endif + if (poller->state == SPDK_POLLER_STATE_UNREGISTERED) { + TAILQ_REMOVE(&thread->timer_pollers, poller, tailq); + free(poller); + } else { + poller->state = SPDK_POLLER_STATE_WAITING; + TAILQ_REMOVE(&thread->timer_pollers, poller, tailq); + _spdk_poller_insert_timer(thread, poller, now); + } + if (timer_rc > rc) { rc = timer_rc; - } }