diff --git a/sys/xtimer/xtimer.c b/sys/xtimer/xtimer.c index d51e03f5b3..8576228084 100644 --- a/sys/xtimer/xtimer.c +++ b/sys/xtimer/xtimer.c @@ -45,7 +45,15 @@ typedef struct { mutex_t *mutex; thread_t *thread; - volatile int timeout; + /* + * @brief: one means the thread was removed from the mutex thread waiting list + * by _mutex_timeout() + */ + volatile uint8_t dequeued; + /* + * @brief: mutex_lock() should block because _mutex_timeout() did not shoot + */ + volatile uint8_t blocking; } mutex_thread_t; static void _callback_unlock_mutex(void* arg) @@ -200,9 +208,36 @@ void xtimer_now_timex(timex_t *out) out->microseconds = now - (out->seconds * US_PER_SEC); } +/* + * The value pointed to by unlocked will be set to 1 if the thread was removed from the waiting queue otherwise 0. + */ +static void _mutex_remove_thread_from_waiting_queue(mutex_t *mutex, thread_t *thread, volatile uint8_t *unlocked) +{ + unsigned irqstate = irq_disable(); + assert(mutex != NULL && thread != NULL); + + if (mutex->queue.next != MUTEX_LOCKED && mutex->queue.next != NULL) { + list_node_t *node = list_remove(&mutex->queue, (list_node_t *)&thread->rq_entry); + /* if thread was removed from the list */ + if (node != NULL) { + if (mutex->queue.next == NULL) { + mutex->queue.next = MUTEX_LOCKED; + } + *unlocked = 1; + + sched_set_status(thread, STATUS_PENDING); + irq_restore(irqstate); + sched_switch(thread->priority); + return; + } + } + *unlocked = 0; + irq_restore(irqstate); +} + static void _mutex_timeout(void *arg) { - /* interrupts a disabled because xtimer can spin + /* interrupts are disabled because xtimer can spin * if xtimer_set spins the callback is executed * in the thread context * @@ -212,41 +247,27 @@ static void _mutex_timeout(void *arg) unsigned int irqstate = irq_disable(); mutex_thread_t *mt = (mutex_thread_t *)arg; - - if (mt->mutex->queue.next != MUTEX_LOCKED && - mt->mutex->queue.next != NULL) { - mt->timeout = 1; - list_node_t *node = list_remove(&mt->mutex->queue, - (list_node_t *)&mt->thread->rq_entry); - - /* if thread was removed from the list */ - if (node != NULL) { - if (mt->mutex->queue.next == NULL) { - mt->mutex->queue.next = MUTEX_LOCKED; - } - sched_set_status(mt->thread, STATUS_PENDING); - irq_restore(irqstate); - sched_switch(mt->thread->priority); - return; - } - } + mt->blocking = 0; + _mutex_remove_thread_from_waiting_queue(mt->mutex, mt->thread, &mt->dequeued); irq_restore(irqstate); } int xtimer_mutex_lock_timeout(mutex_t *mutex, uint64_t timeout) { xtimer_t t; - mutex_thread_t mt = { mutex, (thread_t *)sched_active_thread, 0 }; + mutex_thread_t mt = { mutex, (thread_t *)sched_active_thread, .dequeued=0, .blocking=1 }; if (timeout != 0) { t.callback = _mutex_timeout; t.arg = (void *)((mutex_thread_t *)&mt); xtimer_set64(&t, timeout); } - - mutex_lock(mutex); + int ret = _mutex_lock(mutex, mt.blocking); + if (ret == 0) { + return -1; + } xtimer_remove(&t); - return -mt.timeout; + return -mt.dequeued; } #ifdef MODULE_CORE_THREAD_FLAGS diff --git a/tests/xtimer_mutex_lock_timeout/main.c b/tests/xtimer_mutex_lock_timeout/main.c index c96f32ccbd..f06cf8e25c 100644 --- a/tests/xtimer_mutex_lock_timeout/main.c +++ b/tests/xtimer_mutex_lock_timeout/main.c @@ -28,6 +28,9 @@ /* timeout at one millisecond (1000 us) to make sure it does not spin. */ #define LONG_MUTEX_TIMEOUT 1000 +/* timeout smaller than XTIMER_BACKOFF to make sure it spins. */ +#define SHORT_MUTEX_TIMEOUT ((1 << XTIMER_SHIFT) + 1) + /* main Thread PID */ static kernel_pid_t main_thread_pid; @@ -40,6 +43,11 @@ static int cmd_test_xtimer_mutex_lock_timeout_long_locked(int argc, char **argv); static int cmd_test_xtimer_mutex_lock_timeout_low_prio_thread(int argc, char **argv); +static int cmd_test_xtimer_mutex_lock_timeout_short_unlocked(int argc, + char **argv); +static int cmd_test_xtimer_mutex_lock_timeout_short_locked(int argc, + char **argv); + /** * @brief List of command for this application. @@ -52,6 +60,10 @@ static const shell_command_t shell_commands[] = { { "mutex_timeout_long_locked_low", "lock low-prio-locked-mutex from high-prio-thread (no-spin timeout)", cmd_test_xtimer_mutex_lock_timeout_low_prio_thread, }, + { "mutex_timeout_short_unlocked", "unlocked mutex (spin timeout)", + cmd_test_xtimer_mutex_lock_timeout_short_unlocked, }, + { "mutex_timeout_short_locked", "locked mutex (spin timeout)", + cmd_test_xtimer_mutex_lock_timeout_short_locked, }, { NULL, NULL, NULL } }; @@ -247,6 +259,86 @@ static int cmd_test_xtimer_mutex_lock_timeout_low_prio_thread(int argc, return 0; } +/** + * @brief shell command to test xtimer_mutex_lock_timeout when spinning + * + * The mutex is locked before the function call and + * the timer short. Meaning the timer will trigger before + * xtimer_mutex_lock_timeout tries to acquire the mutex. + * + * @param[in] argc Number of arguments + * @param[in] argv Array of arguments + * + * @return 0 on success + */ +static int cmd_test_xtimer_mutex_lock_timeout_short_locked(int argc, + char **argv) +{ + (void)argc; + (void)argv; + puts( + "starting test: xtimer mutex lock timeout with short timeout and locked mutex"); + mutex_t test_mutex = MUTEX_INIT; + mutex_lock(&test_mutex); + + if (xtimer_mutex_lock_timeout(&test_mutex, SHORT_MUTEX_TIMEOUT) == 0) { + puts("Error: mutex taken"); + } + else { + /* mutex has to be locked */ + if (mutex_trylock(&test_mutex) == 0) { + puts("OK"); + } + else { + puts("error mutex not locked"); + } + } + /* to make the test easier to read */ + printf("\n"); + + return 0; +} + +/** + * @brief shell command to test xtimer_mutex_lock_timeout when spinning + * + * the mutex is not locked before the function is called and + * the timer is short. Meaning the timer will trigger before + * xtimer_mutex_lock_timeout tries to acquire the mutex. + * + * @param[in] argc Number of arguments + * @param[in] argv Array of arguments + * + * @return 0 on success + */ +static int cmd_test_xtimer_mutex_lock_timeout_short_unlocked(int argc, + char **argv) +{ + (void)argc; + (void)argv; + puts( + "starting test: xtimer mutex lock timeout with short timeout and unlocked mutex"); + mutex_t test_mutex = MUTEX_INIT; + + if (xtimer_mutex_lock_timeout(&test_mutex, SHORT_MUTEX_TIMEOUT) == 0) { + /* mutex has to be locked */ + if (mutex_trylock(&test_mutex) == 0) { + puts("OK"); + } + else { + puts("error mutex not locked"); + } + } + else { + puts("Error: mutex timed out"); + } + /* to make the test easier to read */ + printf("\n"); + + return 0; +} + + /** * @brief main function starting shell * diff --git a/tests/xtimer_mutex_lock_timeout/tests/01-run.py b/tests/xtimer_mutex_lock_timeout/tests/01-run.py index e121d477d0..14170f10dd 100755 --- a/tests/xtimer_mutex_lock_timeout/tests/01-run.py +++ b/tests/xtimer_mutex_lock_timeout/tests/01-run.py @@ -38,6 +38,14 @@ def testfunc(child): child.expect("THREAD low prio: exiting low") child.expect("threads = 2") child.expect_exact("> ") + child.sendline("mutex_timeout_short_locked") + child.expect("starting test: xtimer mutex lock timeout with short timeout and locked mutex") + child.expect("OK") + child.expect_exact("> ") + child.sendline("mutex_timeout_short_unlocked") + child.expect("starting test: xtimer mutex lock timeout with short timeout and unlocked mutex") + child.expect("OK") + child.expect_exact("> ") if __name__ == "__main__":