From eae664ea8fdae9e514dc7970c6cf21a0b9767ddd Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Mon, 12 Mar 2018 14:38:46 +0100 Subject: mutex: Use panic() after ISR lock release Using panic() with interrupts disabled could lead to an additional error (INTERNAL_ERROR_BAD_THREAD_DISPATCH_ENVIRONMENT) due to a potentially blocking output. --- rtemsbsd/include/machine/rtems-bsd-muteximpl.h | 21 +++++- rtemsbsd/rtems/rtems-kernel-muteximpl.c | 10 ++- testsuite/mutex01/test_main.c | 100 ++++++++++++++++++++++++- 3 files changed, 124 insertions(+), 7 deletions(-) diff --git a/rtemsbsd/include/machine/rtems-bsd-muteximpl.h b/rtemsbsd/include/machine/rtems-bsd-muteximpl.h index 4706201b..c947e2ba 100644 --- a/rtemsbsd/include/machine/rtems-bsd-muteximpl.h +++ b/rtemsbsd/include/machine/rtems-bsd-muteximpl.h @@ -8,7 +8,7 @@ */ /* - * Copyright (c) 2014, 2017 embedded brains GmbH. All rights reserved. + * Copyright (c) 2014, 2018 embedded brains GmbH. All rights reserved. * * embedded brains GmbH * Dornierstr. 4 @@ -46,6 +46,9 @@ #include #include +#include + +#include #include #include @@ -160,7 +163,12 @@ rtems_bsd_mutex_trylock(struct lock_object *lock, rtems_bsd_mutex *m) _Thread_Resource_count_increment(executing); success = 1; } else if (owner == executing) { - BSD_ASSERT(lock->lo_flags & LO_RECURSABLE); + if ((lock->lo_flags & LO_RECURSABLE) == 0) { + rtems_bsd_mutex_release(m, isr_level, &queue_context); + panic("mutex trylock: %s: not LO_RECURSABLE\n", + m->queue.Queue.name); + } + ++m->nest_level; success = 1; } else { @@ -178,6 +186,7 @@ rtems_bsd_mutex_unlock(rtems_bsd_mutex *m) ISR_Level isr_level; Thread_queue_Context queue_context; Thread_Control *owner; + Thread_Control *executing; int nest_level; _Thread_queue_Context_initialize(&queue_context); @@ -186,8 +195,14 @@ rtems_bsd_mutex_unlock(rtems_bsd_mutex *m) nest_level = m->nest_level; owner = m->queue.Queue.owner; + executing = _Thread_Executing; - BSD_ASSERT(owner == _Thread_Executing); + if (__predict_false(owner != executing)) { + rtems_bsd_mutex_release(m, isr_level, &queue_context); + panic("mutex unlock: %s: owner 0x%08" PRIx32 + " != executing 0x%08" PRIx32 "\n", m->queue.Queue.name, + owner->Object.id, executing->Object.id); + } if (__predict_true(nest_level == 0)) { Thread_queue_Heads *heads; diff --git a/rtemsbsd/rtems/rtems-kernel-muteximpl.c b/rtemsbsd/rtems/rtems-kernel-muteximpl.c index d2400282..8a832b4e 100644 --- a/rtemsbsd/rtems/rtems-kernel-muteximpl.c +++ b/rtemsbsd/rtems/rtems-kernel-muteximpl.c @@ -7,7 +7,7 @@ */ /* - * Copyright (c) 2014, 2016 embedded brains GmbH. All rights reserved. + * Copyright (c) 2014, 2018 embedded brains GmbH. All rights reserved. * * embedded brains GmbH * Dornierstr. 4 @@ -48,9 +48,13 @@ rtems_bsd_mutex_lock_more(struct lock_object *lock, rtems_bsd_mutex *m, Thread_queue_Context *queue_context) { if (owner == executing) { - BSD_ASSERT(lock->lo_flags & LO_RECURSABLE); - ++m->nest_level; + if ((lock->lo_flags & LO_RECURSABLE) == 0) { + _Thread_queue_Release(&m->queue, queue_context); + panic("mutex lock: %s: not LO_RECURSABLE\n", + m->queue.Queue.name); + } + ++m->nest_level; _Thread_queue_Release(&m->queue, queue_context); } else { _Thread_queue_Context_set_thread_state(queue_context, diff --git a/testsuite/mutex01/test_main.c b/testsuite/mutex01/test_main.c index caaf1185..c3e193d9 100644 --- a/testsuite/mutex01/test_main.c +++ b/testsuite/mutex01/test_main.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014 embedded brains GmbH. All rights reserved. + * Copyright (c) 2014, 2018 embedded brains GmbH. All rights reserved. * * embedded brains GmbH * Dornierstr. 4 @@ -66,6 +66,7 @@ typedef struct { int timo; rtems_id worker_task[WORKER_COUNT]; bool done[WORKER_COUNT]; + rtems_id panic_task; } test_context; static test_context test_instance; @@ -478,6 +479,102 @@ test_mtx_sleep_timeout(test_context *ctx) assert(!mtx_initialized(mtx)); } +#define PANIC_DO_LOCK 0 + +#define PANIC_DO_TRYLOCK 1 + +#define PANIC_DO_UNLOCK 2 + +static void +panic_task(rtems_task_argument arg) +{ + test_context *ctx = &test_instance; + struct mtx *mtx = &ctx->mtx; + + switch (arg) { + case PANIC_DO_LOCK: + mtx_lock(mtx); + mtx_lock(mtx); + break; + case PANIC_DO_TRYLOCK: + mtx_lock(mtx); + mtx_trylock(mtx); + break; + case PANIC_DO_UNLOCK: + mtx_unlock(mtx); + break; + default: + assert(0); + break; + } + + rtems_task_suspend(RTEMS_SELF); +} + +static void +test_mtx_panic(test_context *ctx) +{ + struct mtx *mtx = &ctx->mtx; + rtems_status_code sc; + + puts("test mtx panic"); + + sc = rtems_task_create(rtems_build_name('P', 'A', 'N', 'I'), + get_self_prio(), RTEMS_MINIMUM_STACK_SIZE, RTEMS_DEFAULT_MODES, + RTEMS_FLOATING_POINT, &ctx->panic_task); + assert(sc == RTEMS_SUCCESSFUL); + + assert(!mtx_initialized(mtx)); + mtx_init(mtx, "test", NULL, MTX_DEF); + assert(mtx_initialized(mtx)); + + /* Lock recursive panic */ + + sc = rtems_task_start(ctx->panic_task, panic_task, PANIC_DO_LOCK); + assert(sc == RTEMS_SUCCESSFUL); + + sc = rtems_task_wake_after(RTEMS_YIELD_PROCESSOR); + assert(sc == RTEMS_SUCCESSFUL); + + sc = rtems_task_restart(ctx->panic_task, PANIC_DO_UNLOCK); + assert(sc == RTEMS_SUCCESSFUL); + + sc = rtems_task_wake_after(RTEMS_YIELD_PROCESSOR); + assert(sc == RTEMS_SUCCESSFUL); + + /* Try lock recursive panic */ + + sc = rtems_task_restart(ctx->panic_task, PANIC_DO_TRYLOCK); + assert(sc == RTEMS_SUCCESSFUL); + + sc = rtems_task_wake_after(RTEMS_YIELD_PROCESSOR); + assert(sc == RTEMS_SUCCESSFUL); + + sc = rtems_task_restart(ctx->panic_task, PANIC_DO_UNLOCK); + assert(sc == RTEMS_SUCCESSFUL); + + sc = rtems_task_wake_after(RTEMS_YIELD_PROCESSOR); + assert(sc == RTEMS_SUCCESSFUL); + + /* Unlock not owner panic */ + + mtx_lock(mtx); + + sc = rtems_task_restart(ctx->panic_task, PANIC_DO_UNLOCK); + assert(sc == RTEMS_SUCCESSFUL); + + sc = rtems_task_wake_after(RTEMS_YIELD_PROCESSOR); + assert(sc == RTEMS_SUCCESSFUL); + + mtx_unlock(mtx); + + mtx_destroy(mtx); + assert(!mtx_initialized(mtx)); + + sc = rtems_task_delete(ctx->panic_task); + assert(sc == RTEMS_SUCCESSFUL); +} + static void alloc_basic_resources(void) { @@ -504,6 +601,7 @@ test_main(void) test_mtx_recursive(ctx); test_mtx_trylock(ctx); test_mtx_lock(ctx); + test_mtx_panic(ctx); assert(rtems_resource_snapshot_check(&snapshot_1)); -- cgit v1.2.3